GLContext.cpp:1008:20: warning: variable ‘depthType’ set but not used [-Wunused-but-set-variable]

RESOLVED FIXED in mozilla8

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug)

Trunk
mozilla8
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(1 attachment, 1 obsolete attachment)

GCC 4.6 build warning:
{
gfx/thebes/GLContext.cpp: In member function ‘PRBool mozilla::gl::GLContext::ResizeOffscreenFBO(const gfxIntSize&)’:
gfx/thebes/GLContext.cpp:1008:20: warning: variable ‘depthType’ set but not used [-Wunused-but-set-variable]
}

Variable appears to have been unused in the patch that created it:
  http://hg.mozilla.org/mozilla-central/rev/d879bb244890#l4.142

I'm guessing it's just stale code from a previous version of that patch that wasn't needed in the final version.
Created attachment 544635 [details] [diff] [review]
fix v1: remove variable
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #544635 - Flags: review?(bjacob)
Comment on attachment 544635 [details] [diff] [review]
fix v1: remove variable

This strawman patch just removes the variable.

Alternately, maybe we want to be passing "depthType" into the fRenderbufferStorage() call lower down in the patch context, rather than passing
> mIsGLES2 ? LOCAL_GL_DEPTH_COMPONENT16
> : LOCAL_GL_DEPTH_COMPONENT24,

bjacob, do you know which we want here?  (perhaps the existing "mIsGLES2" ternary check there is sufficient?)
Attachment #544635 - Attachment description: fix → fix v1: remove variable
OS: Linux → All
Hardware: x86_64 → All
No time to check this carefully now and leaving for 2 weeks, so you will have to find another reviewer if you want the best fix quickly. For WebGL, we do want to get more than 16 bits of depth if possible, ideally 24 bits, but we ask for that explicitly in WebGLContext::SetDimensions by setting up the ContextFormat accordingly, and I don't remember well enough the code to remember why we dont seem to be using that now. jrmuizel, joe, mwoodrow could be good reviewers.
Created attachment 545293 [details] [diff] [review]
fix v2: use variable

per chat with vlad on IRC, we should probably be using this variable after all.
Attachment #544635 - Attachment is obsolete: true
Attachment #545293 - Flags: review?
Attachment #544635 - Flags: review?(bjacob)
Attachment #545293 - Flags: review? → review?(vladimir)
Comment on attachment 545293 [details] [diff] [review]
fix v2: use variable

Looks good, thanks!
Attachment #545293 - Flags: review?(vladimir) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/9fb5b756e44c
Whiteboard: [build_warning] → [build_warning][inbound]
Merged:
http://hg.mozilla.org/mozilla-central/rev/9fb5b756e44c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [build_warning][inbound] → [build_warning]
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.