Bug 212594 – [ macOS ] REGRESSION(r262366): webgl/1.0.3/conformance/canvas/buffer-offscreen-test.html & webgl/2.0.0/conformance/canvas/buffer-offscreen-test.html are constant failures

• Component: WebGL
• Status: RESOLVED FIXED

[Jacob Uphoff]

webgl/2.0.0/conformance/canvas/buffer-offscreen-test.html
webgl/1.0.3/conformance/canvas/buffer-offscreen-test.html

These tests became constant failures after commit r262366 for all of macOS.

History:

https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=webgl%2F2.0.0%2Fconformance%2Fcanvas%2Fbuffer-offscreen-test.html&test=webgl%2F1.0.3%2Fconformance%2Fcanvas%2Fbuffer-offscreen-test.html

Diffs:

--- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/webgl/1.0.3/conformance/canvas/buffer-offscreen-test-expected.txt
+++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/webgl/1.0.3/conformance/canvas/buffer-offscreen-test-actual.txt
@@ -1,5 +1,14 @@
 This test runs the WebGL Test listed below in an iframe and reports PASS or FAIL.
 
 Test: ../../resources/webgl_test_files/conformance/canvas/buffer-offscreen-test.html
-[ PASS ] All tests passed
+[ 1: PASS ] gl1 != null is true
+[ 2: PASS ] gl2 != null is true
+[ 3: PASS ] gl1.getContextAttributes().preserveDrawingBuffer == false is true
+[ 4: PASS ] gl2.getContextAttributes().preserveDrawingBuffer == false is true
+[ 5: PASS ] cleared corner should be blue, stencil should be preserved
+[ 6: PASS ] remainder of buffer should be cleared
+[ 7: PASS ] cleared corner should be blue, stencil should be preserved
+[ 8: FAIL ] at (0, 0) expected: 255,0,0,255 was 0,0,0,0
+[ 9: PASS ] successfullyParsed is true
+[ FAIL ] 1 failures reported
 

--- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/webgl/2.0.0/conformance/canvas/buffer-offscreen-test-expected.txt
+++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/webgl/2.0.0/conformance/canvas/buffer-offscreen-test-actual.txt
@@ -1,5 +1,14 @@
 This test runs the WebGL Test listed below in an iframe and reports PASS or FAIL.
 
 Test: ../../resources/webgl_test_files/conformance/canvas/buffer-offscreen-test.html
-[ PASS ] All tests passed
+[ 1: PASS ] gl1 != null is true
+[ 2: PASS ] gl2 != null is true
+[ 3: PASS ] gl1.getContextAttributes().preserveDrawingBuffer == false is true
+[ 4: PASS ] gl2.getContextAttributes().preserveDrawingBuffer == false is true
+[ 5: PASS ] cleared corner should be blue, stencil should be preserved
+[ 6: PASS ] remainder of buffer should be cleared
+[ 7: PASS ] cleared corner should be blue, stencil should be preserved
+[ 8: FAIL ] remainder of buffer should be un-cleared red at (0, 0) expected: 255,0,0,255 was 0,0,0,0
+[ 9: PASS ] successfullyParsed is true
+[ FAIL ] 1 failures reported

[Radar WebKit Bug Importer]

<rdar://problem/63828783>

[Ryan Haddad]

Marked tests as failing in r262400

[Dean Jackson]

It's quite confusing as to how I could have broken this. Maybe another case where we are setting the GL state temporarily for compositing and not restoring it?

[Dean Jackson]

Our visual result is correct. It's the content read from the in-progress draw that's wrong.

[Dean Jackson]

Hmmm.. I see an extra call to clear(COLOR_BUFFER_BIT | DEPTH_BUFFER_BIT)

[Dean Jackson]

Oh. I see.

We shouldn't prepare the canvas if it isn't going to be displayed. We're effectively swapping the drawing buffer on a canvas that is never drawn, so when it is read back out it is cleared.

[Dean Jackson]

Now that I think of it, this whole area could do with some cleanup. We really only need a WebGLLayer when the owning canvas is put in the page. It's a bit weird that the GraphicsContextGL lets the WebGLLayer manage the IOSurface backbuffers. Instead it should just hand the current framebuffer surface to the layer.

I don't want to make such a big change right now though.

[Dean Jackson]

This is slightly tricky since we indicate we need preparation as we touch GL, but we only know if we need to actually prepare at the end of the run loop.

So I guess the Document::prepareCanvasesForDisplayIfNeeded has an even more correct name now. It should check if the canvas element is actually in its body before preparing.

[Dean Jackson]

Created attachment 400880 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 400880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400880&action=review

> Source/WebCore/ChangeLog:3
> +        [ macOS ] REGRESSION(r262366): webgl/1.0.3/conformance/canvas/buffer-offscreen-test.html & webgl/2.0.0/conformance/canvas/buffer-offscreen-test.html are constant failures

Could you please unmark the tests in a single commit with the fix?

[Dean Jackson]

Comment on attachment 400880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400880&action=review

>> Source/WebCore/ChangeLog:3
>> +        [ macOS ] REGRESSION(r262366): webgl/1.0.3/conformance/canvas/buffer-offscreen-test.html & webgl/2.0.0/conformance/canvas/buffer-offscreen-test.html are constant failures
> 
> Could you please unmark the tests in a single commit with the fix?

Yes, I should have thought of that.

[Dean Jackson]

Committed r262498: <https://trac.webkit.org/changeset/262498>