931335 - "Assertion failure: face < mFacesCount (wrong face index, must be 0 for TEXTURE_2D and at most 5 for cube maps)"

Status: VERIFIED FIXED

(Core :: Canvas: WebGL, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: face < mFacesCount (wrong face index, must be 0 for TEXTURE_2D and at most 5 for cube maps), at content/canvas/src/WebGLTexture.h:143

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

With constants values replaced by symbolic constant names, the testcase is:

  gl.bindTexture(TEXTURE_2D, texture);
  gl.bindFramebuffer(FRAMEBUFFER, frameBuffer);
  gl.copyTexImage2D(TEXTURE_2D, 16777217, 6407, 0.4539905885360182, 2, 1024, 1, 0);
  gl.framebufferTexture2D(FRAMEBUFFER, DEPTH_ATTACHMENT, TEXTURE_CUBE_MAP_NEGATIVE_X, texture, 0);
  gl.checkFramebufferStatus(FRAMEBUFFER);

The framebufferTexture2D call here should have been rejected, as it tries to attach a cube map face... from a 2d texture. But it isn't rejected as it should be, putting us in a corrupted state, which this assertion complains about.

Patch coming.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Handle mismatched texture target in framebufferTexture2D

Comment 4

[Jeff Gilbert [:jgilbert]]

Comment on attachment 823295 [details] [diff] [review]
Handle mismatched texture target in framebufferTexture2D

Review of attachment 823295 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/src/WebGLFramebuffer.cpp
@@ +264,5 @@
> +    if (wtex) {
> +        bool isTexture2D = wtex->Target() == LOCAL_GL_TEXTURE_2D;
> +        bool isTexTarget2D = textarget == LOCAL_GL_TEXTURE_2D;
> +        if (isTexture2D != isTexTarget2D)
> +        {

`{` should be on same line as conditional. (for one-line conditionals)

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/229f179bd8d7

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 823295 [details] [diff] [review]
Handle mismatched texture target in framebufferTexture2D

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Don't know
User impact if declined: Crash on certain WebGL content; possibly security sensitive.
Testing completed (on m-c, etc.): m-i
Risk to taking this patch (and alternatives if risky): very low risk, just adding a simple check in one WebGL method
String or IDL/UUID changes made by this patch: none

Comment 7

[bhavana bajaj [:bajaj]]

(In reply to Benoit Jacob [:bjacob] from comment #6)
> Comment on attachment 823295 [details] [diff] [review]
> Handle mismatched texture target in framebufferTexture2D
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): Don't know
Can we find out where this fallout is from to help understand the criticality on uplift(especially beta) here ?
> User impact if declined: Crash on certain WebGL content; possibly security
> sensitive.
> Testing completed (on m-c, etc.): m-i
> Risk to taking this patch (and alternatives if risky): very low risk, just
> adding a simple check in one WebGL method
> String or IDL/UUID changes made by this patch: none/

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to bhavana bajaj [:bajaj] from comment #7)
> (In reply to Benoit Jacob [:bjacob] from comment #6)
> > Comment on attachment 823295 [details] [diff] [review]
> > Handle mismatched texture target in framebufferTexture2D
> > 
> > [Approval Request Comment]
> > Bug caused by (feature/regressing bug #): Don't know
> Can we find out where this fallout is from to help understand the
> criticality on uplift(especially beta) here ?

I meant that a priori I don't have a reason to believe that it is a regression. As such, I would understand if you didn't consider this beta-worthy.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/229f179bd8d7

Comment 10

[bhavana bajaj [:bajaj]]

Comment on attachment 823295 [details] [diff] [review]
Handle mismatched texture target in framebufferTexture2D

Based on recent bug comments lets uplift this to aurora only.

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1a88cb161053

Comment 12

[Paul Silaghi, QA [:pauly]]

Reproduced on Nightly 2013-10-26-mozilla-central-debug.
Verified fixed Nightly 2013-12-08-mozilla-central-debug, Win 7 x64.

Comment 13

[Ioana (away)]

Reproduced on the 10/26 Nightly, verified as fixed on the 01/24 Beta Debug. Both Firefox 27, both on the same Mac OS X 10.8.5.