Last Comment Bug 670025 - GLContext.cpp:1008:20: warning: variable ‘depthType’ set but not used [-Wunused-but-set-variable]
: GLContext.cpp:1008:20: warning: variable ‘depthType’ set but not used [-Wunus...
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Daniel Holbert [:dholbert]
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: buildwarning 583838
  Show dependency treegraph
 
Reported: 2011-07-07 15:15 PDT by Daniel Holbert [:dholbert]
Modified: 2011-07-12 04:00 PDT (History)
1 user (show)
mounir: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v1: remove variable (1.60 KB, patch)
2011-07-07 15:20 PDT, Daniel Holbert [:dholbert]
no flags Details | Diff | Splinter Review
fix v2: use variable (1.87 KB, patch)
2011-07-11 17:41 PDT, Daniel Holbert [:dholbert]
vladimir: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2011-07-07 15:15:53 PDT
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.
Comment 1 Daniel Holbert [:dholbert] 2011-07-07 15:20:20 PDT
Created attachment 544635 [details] [diff] [review]
fix v1: remove variable
Comment 2 Daniel Holbert [:dholbert] 2011-07-07 15:23:08 PDT
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?)
Comment 3 Benoit Jacob [:bjacob] (mostly away) 2011-07-07 15:34:02 PDT
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.
Comment 4 Daniel Holbert [:dholbert] 2011-07-11 17:41:02 PDT
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.
Comment 5 Vladimir Vukicevic [:vlad] [:vladv] 2011-07-11 18:59:51 PDT
Comment on attachment 545293 [details] [diff] [review]
fix v2: use variable

Looks good, thanks!
Comment 6 Daniel Holbert [:dholbert] 2011-07-12 00:14:16 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/9fb5b756e44c
Comment 7 Mounir Lamouri (:mounir) 2011-07-12 04:00:17 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/9fb5b756e44c

Note You need to log in before you can comment on or make changes to this bug.