1235299 - Pass WebGL2 conformance test copy-tex-image-2d-formats

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Ethan Lin[:ethlin]]

We will get assertion error when running the test.

Comment 1

[Ethan Lin[:ethlin]]

There are two assertions. One is caused by shader compile error[1]. The other one is android assertion[2].

[1] https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLBlitHelper.cpp#327
[2] https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLBlitHelper.cpp#396

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Fix shader compile error and remove unnecessary assertion

There was a gl error "Shader info log (103 bytes): ERROR: 0:1: '' :  #version required and missing." So I add version tag in shader.
For the android assertion, it was added by bug 1215892 to fix warning [1]. I think we can simply remove this assertion.

[1] https://hg.mozilla.org/mozilla-central/rev/a5d54cef3b23

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8702445 [details] [diff] [review]
Fix shader compile error and remove unnecessary assertion

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

::: gfx/gl/GLBlitHelper.cpp
@@ -392,5 @@
>                  GLint texUnitLoc = mGL->fGetUniformLocation(program, "uTexUnit");
>                  MOZ_ASSERT(texUnitLoc != -1, "uniform uTexUnit not found");
>                  mGL->fUniform1i(texUnitLoc, 0);
> -#else
> -                MOZ_ASSERT_UNREACHABLE("gralloc not support on non-android");

This is still true. Why are you removing it?

Comment 4

[Ethan Lin[:ethlin]]

Attachment: Fix shader compile error and remove unnecessary assertion

Apply jgilbert's comment. Keep the assertion for ConvertSurfaceTexture and ConvertGralloc.

Comment 5

[Ethan Lin[:ethlin]]

Attachment: Fix shader compile error and assertion problem

The origin macro scope was wrong. The android platform macro should apply to ConvertSurfaceTexture and ConvertGralloc, like [1]. So I change the macro scope and remove the assertion.

[1] https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLBlitHelper.cpp?case=true&from=GLBlitHelper.cpp#245

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8702640 [details] [diff] [review]
Fix shader compile error and assertion problem

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

Would this interfere with running on Core profiles?
We don't have to block on this here, but I'd like to know.

Comment 7

[Ethan Lin[:ethlin]]

Attachment: Fix shader compile error and assertion problem. (carry r+: jgilbert)

Fix try server fails by adding default for switch.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=06020735aeb2

Comment 8

[Ethan Lin[:ethlin]]

Comment on attachment 8705458 [details] [diff] [review]
Fix shader compile error and assertion problem. (carry r+: jgilbert)

Keep f? for comment 6

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d538bcfa4e69

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d538bcfa4e69

Comment 11

[u480271]

(In reply to Jeff Gilbert [:jgilbert] from comment #6)
> Would this interfere with running on Core profiles?

This depends on the presence of ARB_ES2_compatibility.