Closed Bug 1235299 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

()

Details

Attachments

(1 file, 3 obsolete files)

We will get assertion error when running the test.
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
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
Attachment #8702445 - Flags: review?(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?
Attachment #8702445 - Flags: review?(jgilbert) → review-
Apply jgilbert's comment. Keep the assertion for ConvertSurfaceTexture and ConvertGralloc.
Attachment #8702445 - Attachment is obsolete: true
Attachment #8702455 - Flags: review?(jgilbert)
Attachment #8702455 - Attachment is obsolete: true
Attachment #8702455 - Flags: review?(jgilbert)
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
Attachment #8702640 - Flags: review?(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.
Attachment #8702640 - Flags: review?(jgilbert)
Attachment #8702640 - Flags: review+
Attachment #8702640 - Flags: feedback?(dglastonbury)
Attachment #8702640 - Attachment is obsolete: true
Attachment #8702640 - Flags: feedback?(dglastonbury)
Comment on attachment 8705458 [details] [diff] [review]
Fix shader compile error and assertion problem. (carry r+: jgilbert)

Keep f? for comment 6
Attachment #8705458 - Flags: feedback?(dglastonbury)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d538bcfa4e69
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
(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.
Attachment #8705458 - Flags: feedback?(dglastonbury)
Depends on: 1373702
You need to log in before you can comment on or make changes to this bug.