[webkit-reviews] review granted: [Bug 243160] [GraphicsContextGLTextureMapperANGLE][WebGL2] webgl/2.0.0/conformance2/query/occlusion-query.html is failing : [Attachment 461183] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 243160: [GraphicsContextGLTextureMapperANGLE][WebGL2]
webgl/2.0.0/conformance2/query/occlusion-query.html is failing
https://bugs.webkit.org/show_bug.cgi?id=243160

Attachment 461183: Patch

https://bugs.webkit.org/attachment.cgi?id=461183&action=review




--- Comment #5 from Kenneth Russell <kbr at google.com> ---
Comment on attachment 461183
  --> https://bugs.webkit.org/attachment.cgi?id=461183
Patch

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

Looks fine to me. Filed follow-on bug crbug.com/angleproject/7526 to eliminate
this requirement. r+

>>>
Source/WebCore/platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp
:273
>>> +	     requiredExtensions.append("GL_EXT_occlusion_query_boolean"_s);
>> 
>> Occlusion queries are part of the core OpenGL ES 3.0 spec, so it should not
be necessary to require this extension in that mode.
>> 
>> Is this class properly requesting a WebGL 2.0 compatible context from ANGLE
when WebGL 2.0 is enabled?
> 
> Yes. I confirmed it's reaching the code initialing as EGL ver.3 in the test
case.
>
https://github.com/WebKit/WebKit/blob/00e1e0d6e977c7cc204944834d6af3254a1d5e5b/
Source/WebCore/platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp
#L214-L215
> 
> Cocoa port is also setting the extension for WebGL2.
>
https://github.com/WebKit/WebKit/blob/00e1e0d6e977c7cc204944834d6af3254a1d5e5b/
Source/WebCore/platform/graphics/cocoa/GraphicsContextGLCocoa.mm#L366-L369
> 
> "gl.getQueryParameter(query, gl.QUERY_RESULT_AVAILABLE)" is keeping returning
false because the following condition is true.
>
https://github.com/WebKit/WebKit/blob/00e1e0d6e977c7cc204944834d6af3254a1d5e5b/
Source/ThirdParty/ANGLE/src/libANGLE/validationES.cpp#L2625-L2627

Thanks, I see - that's surprising. Filed crbug.com/angleproject/7526 to remove
this requirement. Looks good given that.