1137494 - WebGL 1.0.3 conformance error: conformance/extensions/oes-texture-half-float-with-video.html

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Luke]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150226030225

Steps to reproduce:

Verify texImage2D and texSubImage2D code paths taking video elements (RGBA/HALF_FLOAT_OES)

https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/oes-texture-half-float-with-video.html?webglVersion=1


Actual results:

conformance/extensions/oes-texture-half-float-with-video.html (62 of 99 passed)

    failed: at (4, 4) expected: 0,255,0 was 255,0,0
    failed: at (4, 24) expected: 255,0,0 was 0,255,0
    failed: at (4, 4) expected: 0,255,0 was 255,0,0
...
    failed: at (4, 4) expected: 0,255,0 was 255,0,0
    failed: at (4, 24) expected: 255,0,0 was 0,255,0
    failed: getError expected: NO_ERROR. Was INVALID_VALUE : should be no errors


Expected results:

Chrome passes all the tests. Firefox fails with both the ANGLE and OpenGL backends.

Related oes-texture-half-float-with-image-data.html was fixed by patch for Bug 1130616

Comment 1

[Kyle Fung]

This test now crashes the browser since revision 737853697fe3 (https://hg.mozilla.org/integration/mozilla-inbound/rev/737853697fe3)
BlitImageToTexture() triggers MOZ_CRASH() when called by TexImageFromVideoElement()

Comment 2

[Kyle Fung]

Attachment: webgl-oes-hf.patch

Sort of a hack that seems to make the tests pass and also not crash. Is this going to affect any other conformance tests?

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8624335 [details] [diff] [review]
webgl-oes-hf.patch

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

I need more info on why this works.

Comment 4

[Kyle Fung]

The crash is caused by trying to use BlitImageToTexture() on a texture that failed to upload (Line 1886 of WebGLContext.cpp). This fail occurs at line 1879 when calling fTexImage2D() with 'type' being GL_HALF_FLOAT instead of GL_HALF_FLOAT_OES. It fails because ValidateES2TexImageParameters() in validationES2.cpp will not recognize GL_HALF_FLOAT at the switch statement at line 286, and will log an error and return false.

Using GL_HALF_FLOAT_OES instead, ValidateES2TexImageParameters() will not fail and the upload will succeed and a crash will be avoided.

Comment 5

[Kelsey Gilbert [:jgilbert]]

(In reply to kfung from comment #4)
> The crash is caused by trying to use BlitImageToTexture() on a texture that
> failed to upload (Line 1886 of WebGLContext.cpp). This fail occurs at line
> 1879 when calling fTexImage2D() with 'type' being GL_HALF_FLOAT instead of
> GL_HALF_FLOAT_OES. It fails because ValidateES2TexImageParameters() in
> validationES2.cpp will not recognize GL_HALF_FLOAT at the switch statement
> at line 286, and will log an error and return false.
> 
> Using GL_HALF_FLOAT_OES instead, ValidateES2TexImageParameters() will not
> fail and the upload will succeed and a crash will be avoided.

HALF_FLOAT_OES is not generally valid though. It's only valid if we're using OES_texture_half_float. We should rather add a branch that uses HALF_FLOAT_OES if we only have HALF_FLOAT support through OES_texture_half_float.

Comment 6

[Kyle Fung]

There is a check in WebGLContext::ValidateTexImageType() that sees if the OES_texture_half_float extension is enabled. If this fails, then we will return 'false' near the top of WebGLContext::TexImageFromVideoElement(), and we wouldn't try to upload anything using HALF_FLOAT_OES.

Comment 7

[Kyle Fung]

Attachment: webgl-oes-hf-v2.patch

I put the type change to inside the validation check function.

Comment 8

[Kyle Fung]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b9d9b6cc536

Comment 9

[Jukka Jylänki]

There seem to be two different issues related to this test(?). The following tested devices crash on this test:
 - SPARK, MACBOOK_AIR_WIN, MACBOOK_PRO_WIN, NEXUS-4, NEXUS-5, NEXUS-10, SAMSUNG-GT, SURFACE, WINDBOX, HASWELL, HPOMEN

The following tested devices fail on this test, but without crashing:
 - GIADA, MACBOOK_AIR_OSX, MACBOOK_PRO_OSX, MACMINI, MACPRO

See https://bugzilla.mozilla.org/show_bug.cgi?id=1178601 for hardware configuration details of these systems.

The crash is discussed at https://bugzilla.mozilla.org/show_bug.cgi?id=1175931 , I wonder after that crash is fixed, the test will pass, or if there is something more needed to make it pass?

Comment 10

[Carsten Book [:Tomcat]]

(In reply to kfung from comment #8)
> Try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b9d9b6cc536

Hi, the try run has some assertion failures on w2/w8 etc are this failures related to this change ?

Comment 11

[Kyle Fung]

I don't think the failures are related to this patch. It should only relate to things with half_float in WebGL.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22a6dcbf4b03

Comment 13

[Kyle Fung]

Jukka, can you see if the crash occurs on the build I made on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b9d9b6cc536

I do not have the crash on my Windows 8.1 machine.

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/22a6dcbf4b03

Comment 15

[Jukka Jylänki]

Tested this on the HASWELL system, and it no longer crashes. Great work!