Closed Bug 931335 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Canvas: WebGL, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox25 --- wontfix
firefox26 --- wontfix
firefox27 --- verified
firefox28 --- verified

People

(Reporter: jruderman, Assigned: bjacob)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file 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
Attached file stack
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 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)
Attachment #823295 - Flags: review?(jgilbert) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/229f179bd8d7
QA Contact: bjacob
Target Milestone: --- → mozilla28
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
Attachment #823295 - Flags: approval-mozilla-beta?
Attachment #823295 - Flags: approval-mozilla-aurora?
(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/
(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.
https://hg.mozilla.org/mozilla-central/rev/229f179bd8d7
Assignee: nobody → bjacob
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment on attachment 823295 [details] [diff] [review]
Handle mismatched texture target in framebufferTexture2D

Based on recent bug comments lets uplift this to aurora only.
Attachment #823295 - Flags: approval-mozilla-beta?
Attachment #823295 - Flags: approval-mozilla-beta-
Attachment #823295 - Flags: approval-mozilla-aurora?
Attachment #823295 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Reproduced on Nightly 2013-10-26-mozilla-central-debug.
Verified fixed Nightly 2013-12-08-mozilla-central-debug, Win 7 x64.
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.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.