Closed Bug 1137494 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Canvas: WebGL, defect)

39 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: lukebenes, Assigned: kyle_fung)

Details

(Keywords: foxfood, Whiteboard: webgl-conformance gfx-noted)

Attachments

(1 file, 1 obsolete file)

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
Blocks: webgl-1.0.2
Component: Untriaged → Canvas: WebGL
Product: Firefox → Core
Whiteboard: webgl-conformance
Whiteboard: webgl-conformance → webgl-conformance gfx-noted
No longer blocks: webgl-1.0.2
Summary: WebGL conformance error: conformance/extensions/oes-texture-half-float-with-video.html → WebGL 1.0.3 conformance error: conformance/extensions/oes-texture-half-float-with-video.html
Assignee: nobody → kfung
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()
Attached patch webgl-oes-hf.patch (obsolete) — Splinter Review
Sort of a hack that seems to make the tests pass and also not crash. Is this going to affect any other conformance tests?
Attachment #8624335 - Flags: feedback?(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.
Attachment #8624335 - Flags: feedback?(jgilbert)
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.
(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.
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.
I put the type change to inside the validation check function.
Attachment #8624335 - Attachment is obsolete: true
Attachment #8625591 - Flags: review?(jgilbert)
Attachment #8625591 - Flags: review?(jgilbert) → review+
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?
Keywords: foxfood
Keywords: checkin-needed
(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 ?
Flags: needinfo?(kfung)
Keywords: checkin-needed
I don't think the failures are related to this patch. It should only relate to things with half_float in WebGL.
Flags: needinfo?(kfung)
Keywords: checkin-needed
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.
Flags: needinfo?(jujjyl)
https://hg.mozilla.org/mozilla-central/rev/22a6dcbf4b03
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Tested this on the HASWELL system, and it no longer crashes. Great work!
Flags: needinfo?(jujjyl)
You need to log in before you can comment on or make changes to this bug.