Bug 47697 – [chromium] context-attributes-etc conformance test fails in DRT

• Component: WebGL
• Status: RESOLVED FIXED

[Adrienne Walker]

I updated context-attributes-alpha-depth-stencil-antialias.html in the Khronos repository and was hoping to push that updated test into WebKit.

This test runs correctly in Chromium on Linux, but fails the antialias=true test when running in DRT on the same machine.  I tried several variations on the existing test, but could not get Mesa to produce anything other than a pixel that was fully obscured or unobscured.  Mesa reports that it is using multisampling, which is why the test fails.

One possibility to fix this might be to have Mesa not use multisampling at all for WebGL.  We already do this for the canvas case.

[Kenneth Russell]

(In reply to comment #0)
> One possibility to fix this might be to have Mesa not use multisampling at all for WebGL.  We already do this for the canvas case.

This should be pretty easy. By including app/gfx/gl_implementation.h you can query whether Mesa is in use after creating the context.

[Adrienne Walker]

Created attachment 70898 [details]
Patch

[Kenneth Russell]

Comment on attachment 70898 [details]
Patch

Generally looks good, but have you checked in the modified context-attributes-alpha-depth-stencil-antialias.html upstream to the Khronos repository?

[Adrienne Walker]

(In reply to comment #3)
> (From update of attachment 70898 [details])
> Generally looks good, but have you checked in the modified context-attributes-alpha-depth-stencil-antialias.html upstream to the Khronos repository?

I did that first.  It was when I tried to push it into WebKit that I ran into this issue with Mesa.

[Kenneth Russell]

Comment on attachment 70898 [details]
Patch

OK. One issue. Using shouldBeNonNull in the updated tests is not really correct. There should probably be a new predicate "shouldNotBeUndefined" in js-test-pre.js. Adding this will require updating several layout tests:

fast/js/script-tests/global-constructors.js
fast/dom/script-tests/constructed-objects-prototypes.js
fast/dom/script-tests/prototype-inheritance-2.js
fast/dom/script-tests/prototype-inheritance.js
fast/dom/Window/script-tests/window-property-descriptors.js
fast/dom/Window/window-properties.html

I'll r+/cq+ this but please file a bug to add this primitive and update the test to use it.

[Adrienne Walker]

(In reply to comment #5)
> (From update of attachment 70898 [details])
> OK. One issue. Using shouldBeNonNull in the updated tests is not really correct. There should probably be a new predicate "shouldNotBeUndefined" in js-test-pre.js. Adding this will require updating several layout tests:
> 
> fast/js/script-tests/global-constructors.js
> fast/dom/script-tests/constructed-objects-prototypes.js
> fast/dom/script-tests/prototype-inheritance-2.js
> fast/dom/script-tests/prototype-inheritance.js
> fast/dom/Window/script-tests/window-property-descriptors.js
> fast/dom/Window/window-properties.html
> 
> I'll r+/cq+ this but please file a bug to add this primitive and update the test to use it.

Ah, you're quite right that I should be testing against undefined instead of null.

Why does adding this function to js-test-pre.js require touching all those other tests?

[WebKit Commit Bot]

Comment on attachment 70898 [details]
Patch

Clearing flags on attachment: 70898

Committed r69996: <http://trac.webkit.org/changeset/69996>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adrienne Walker]

> I'll r+/cq+ this but please file a bug to add this primitive and update the test to use it.

Filed here: https://bugs.webkit.org/show_bug.cgi?id=47853

[Kenneth Russell]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 70898 [details] [details])
> > OK. One issue. Using shouldBeNonNull in the updated tests is not really correct. There should probably be a new predicate "shouldNotBeUndefined" in js-test-pre.js. Adding this will require updating several layout tests:
> > 
> > fast/js/script-tests/global-constructors.js
> > fast/dom/script-tests/constructed-objects-prototypes.js
> > fast/dom/script-tests/prototype-inheritance-2.js
> > fast/dom/script-tests/prototype-inheritance.js
> > fast/dom/Window/script-tests/window-property-descriptors.js
> > fast/dom/Window/window-properties.html
> > 
> > I'll r+/cq+ this but please file a bug to add this primitive and update the test to use it.
> 
> Ah, you're quite right that I should be testing against undefined instead of null.
> 
> Why does adding this function to js-test-pre.js require touching all those other tests?

Those tests verify that the identifiers attached to the window object are all known. They prevent accidental introduction of new functions at the global scope.