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

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Daniel Holbert [:dholbert]]

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]]

Attachment: fix v1: remove variable

Comment 2

[Daniel Holbert [:dholbert]]

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)]

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]]

Attachment: 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] (needinfo me, slow to respond)]

Comment on attachment 545293 [details] [diff] [review]
fix v2: use variable

Looks good, thanks!

Comment 6

[Daniel Holbert [:dholbert]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/9fb5b756e44c

Comment 7

[Mounir Lamouri (:mounir)]

Merged:
http://hg.mozilla.org/mozilla-central/rev/9fb5b756e44c