1150310 - MOZ_Assert: Assertion failure: false (Bad cached value.), at ../../../dom/canvas/WebGLContextUtils.cpp:1004

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Gregor Wagner [:gwagner]]

On B2G, current trunk when trying to run https://videos.cdn.mozilla.net/uploads/mozhacks/flight-of-the-navigator/ with debug gecko

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: gdb

Comment 2

[Gregor Wagner [:gwagner]]

04-01 18:01:24.371  3862  3862 I Gecko   : Failed 0x0b98 shadow: Cached 0xffffffff/4294967295, should be 0xff/255.

Comment 3

[Gregor Wagner [:gwagner]]

Any updates here?
Thats still happening on B2G.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

It may take a bit to get to this - Jeff, should we disable this assert until this bug gets closed?  What are the next steps, I should be able to get somebody to help if you can lay them out.

Comment 5

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I will try to fix this.

Comment 6

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: clamp stencil mask value. v1

Currently, the maximum bit of stencil buffer is 8.
We might set a big mask value. If we don't clamp it, we will get assert at WebGLContext::AssertCachedState().
I just hard code the maximum value. If we get it dynamically from gl context, we might have performance issue.

e.g.
context->StencilMask(0xffffffff);  <== mStencilWriteMaskFront becomes 0xffffffff.

Comment 8

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Jeff, do you think we will have 8bit more stencil buffer? I'm worried about Bug 1129213.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8616657 [details] [diff] [review]
clamp stencil mask value. v1

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

This change doesn't match the spec. I believe the code we have is accurate (perhaps with exception of the initial state), but that there are relatively benign driver bugs which we ought not to assert on if we handle them transparently for WebGL content.

::: dom/canvas/WebGLContextFramebufferOperations.cpp
@@ +216,5 @@
> +{
> +  // GLES 3.0.4, $4.1.4, p177:
> +  //   [...] the front and back stencil mask are both set to the value `2^s - 1`, where
> +  //   `s` is greater than or equal to the number of bits in the deepest stencil buffer
> +  //   supported by the GL implementation.

Good use of a reference, but this is only for the initial value set at context creation time. (also `s` can be greater, so there's really no reason to clamp)

IIRC, the only restrictions we have are for `ref`, not `mask`:
GLES 3.0.4, p177:
Stencil comparison operations and queries of `ref` clamp its value to the range `[0,2^s]`, where `s` is the number of bits in the stencil buffer attached to the draw framebuffer.

Comment 10

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: only check stencil LSB 8bit value. v1

Comment 11

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: return context cached value for stencil* queries. v1

from irc:
jgilbert> if we change the assert for STENCIL_WRITEMASK, then we also need to change GetParameter's handling of STENCIL_WRITEMASK, since we ask the driver, and the driver might be wrong

Comment 12

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: only check stencil LSB 8bit value. v2

Comment 13

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8617187 [details] [diff] [review]
only check stencil LSB 8bit value. v2

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

We have assert at [1] and [2]. This patch update the assert at [1].
We don't need to handle the case at [2], since we get the initial value from context as [3].

[1]
https://hg.mozilla.org/mozilla-central/annotate/e10e2e8d8bf2/dom/canvas/WebGLContextUtils.cpp#l1205
[2]
https://hg.mozilla.org/mozilla-central/annotate/e10e2e8d8bf2/dom/canvas/WebGLContextValidate.cpp#l1740
[3]
https://hg.mozilla.org/mozilla-central/annotate/e10e2e8d8bf2/dom/canvas/WebGLContextValidate.cpp#l1735

Comment 14

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8617187 [details] [diff] [review]
only check stencil LSB 8bit value. v2

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

WFM

Comment 15

[Jerry Shih[:jerry] (UTC+8) (inactive)]

try link:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1f86cf4222e

Comment 16

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Please land attachment 8617187 [details] [diff] [review] and attachment 8617184 [details] [diff] [review] to m-c.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bdea0412f27
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6ae1bacd8d

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3bdea0412f27
https://hg.mozilla.org/mozilla-central/rev/ee6ae1bacd8d

Comment 19

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Hi Gregor,
Do you still hit the assert as comment 0?

Comment 20

[No-Jun Park [:njpark]]

I tested on the latest debug build of gecko, and did not see the failure in the gecko log.

Version Tested:
Build ID               20150610223012
Gaia Revision          ea88dde7648d2c6c0de354682e17590fca50e4e2
Gaia Date              2015-06-11 14:52:10
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/bfd82015df48
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150611.015455
Firmware Date          Thu Jun 11 01:55:06 EDT 2015
Bootloader             L1TC000118D0

Comment 21

[Gregor Wagner [:gwagner]]

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #19)
> Hi Gregor,
> Do you still hit the assert as comment 0?

Works fine now. Thanks Jerry!