Bug 220076 – Enable Metal ANGLE backend for WebGL

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Dean Jackson]

Enable Metal ANGLE backend for WebGL 1

[Radar WebKit Bug Importer]

<rdar://problem/72565020>

[Dean Jackson]

Created attachment 416641 [details]
Patch

[Dean Jackson]

Comment on attachment 416641 [details]
Patch

Not for review. Currently fails a bunch of tests.

e.g. webgl/1.0.3/conformance/attribs/gl-disabled-vertex-attrib.html

[Dean Jackson]

Also has some extra code in the patch that should be a separate patch.

[Dean Jackson]

The tests pass when run in MiniBrowser, but do not pass when run via WebKitTestRunner.

The failures are usually related to readPixels getting fully white output.

It does call into gl::ReadnPixelsRobustANGLE, but Xcode doesn't want to show me the values of local variables after that call.

e.g. webgl/1.0.3/conformance/attribs/gl-disabled-vertex-attrib.html

Loops through all the vertex attributes

Attribute 0
draw a green quad
before readPixels 0 0 0 0
after readPixels 0 255 0 255

OK!

Attribute 1
draw a green quad
before readPixels 0 0 0 0
after readPixels 255 255 255 255

FAIL!

(this doesn't make sense since the canvas is cleared to black, and the shader should draw red if the attribute wasn't correct)

[Dean Jackson]

So it looks like the first readPixels works, but then it fails from then on, which explains why only some tests are failing.

[Dean Jackson]

A simple test calling readPixels twice works though.

[Dean Jackson]

Hmmm.. maybe it is only tests that use bindAttribLocation.

In fact, fast/canvas/webgl/gl-bind-attrib-location-test.html is failing in MiniBrowser.

(Some of my confusion comes from the fact that the EGLDisplay is reused across contexts, so the code in this patch to allow WebGL1 and 2 with different backends is not actually working.)

[Dean Jackson]

It seems EGL_GetPlatformDisplayEXT returns the existing display even if the parameters are different.

[Dean Jackson]

Yep. Display::GetDisplayFromNativeDisplay can only have one display per native display, and only allows the display to be configured once.

This blocks us from using different backends in the same page (process, even).

[Dean Jackson]

The ANGLEPlatformDisplayMap uses the native display as the key.

[Kenneth Russell]

Something seems broken in the Metal backend related to vertex attributes and possibly binding attribute locations. Tests including:

webgl/1.0.3/conformance/attribs/gl-disabled-vertex-attrib.html
webgl/1.0.3/conformance/attribs/gl-vertex-attrib-render.html
webgl/1.0.3/conformance/attribs/gl-vertex-attrib-zero-issues.html

fail when run in WK1 in the MiniBrowser with the Metal backend.

[Kenneth Russell]

Filed Bug 220137, blocking this one, about the webgl/1.0.3/conformance/attribs/ failures. That is core functionality and those bugs must be fixed before turning on the Metal backend by default.

[Kenneth Russell]

Great work on fixing the attribute location conformance failures in Bug 220137!

There are several other more core conformance failures. Let's file sub-bugs about them.

We're also in the process of upstreaming Apple's large contributions to ANGLE's Metal backend. It would be easier to make future progress if we completed that upstreaming and rolled ANGLE into WebKit before making more changes in WebKit.

[Dean Jackson]

Blocked on 220877

[Dean Jackson]

And b220895

[Dean Jackson]

And finally 220896, which is taking the upstream back to WebKit.

[Dean Jackson]

Created attachment 421117 [details]
Patch

[Dean Jackson]

I am waiting because I think this will change some iOS results, but I don't have an SDK that can launch the simulator right now.

[Dean Jackson]

Still to do:

- avoid crashes on Catalina
- update iOS results (I can't currently test locally)


The Catalina crashes appear to be:
source texture pixelFormat (MTLPixelFormatRG8Unorm) not compatible with texture view pixelFormat (MTLPixelFormatBGRA8Unorm)

[Dean Jackson]

Created attachment 422149 [details]
EWS Test

[Kenneth Russell]

Only 4 test failures, looking really good!!!

[Kenneth Russell]

The list of failures in Bug 222239 is still significant, so let's block this on it.

[Dean Jackson]

Created attachment 422488 [details]
Another EWS test

[Dean Jackson]

Created attachment 423795 [details]
EWS Test

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[Dean Jackson]

Created attachment 423947 [details]
EWS Test 5 or so

[Dean Jackson]

Created attachment 424056 [details]
EWS Test 6

[Dean Jackson]

Committed r274927 (235685@main): <https://commits.webkit.org/235685@main>

[Aakash Jain]

(In reply to Dean Jackson from comment #29)
> Committed r274927 (235685@main): <https://commits.webkit.org/235685@main>
This seems to have broken inspector/canvas/updateShader-webgl.html on 

History: https://results.webkit.org/?suite=layout-tests&test=inspector%2Fcanvas%2FupdateShader-webgl.html

[Aakash Jain]

Example: https://build.webkit.org/results/Apple-BigSur-Release-WK2-Tests/r274927%20(1227)/results.html

Crash: https://build.webkit.org/results/Apple-BigSur-Release-WK2-Tests/r274927%20(1227)/inspector/canvas/updateShader-webgl-crash-log.txt

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libobjc.A.dylib               	0x00007fff201d6d1d objc_msgSend + 29
1   com.apple.Metal               	0x00007fff28380555 -[MTLCompileOptionsInternal setPreprocessorMacros:] + 54
2   libANGLE-shared.dylib         	0x000000010ca59cad rx::mtl::CreateShaderLibrary(id<MTLDevice>, char const*, unsigned long, NSDictionary<NSString*, NSObject*>*, rx::mtl::AutoObjCPtr<NSError*>*) + 502
3   libANGLE-shared.dylib         	0x000000010ca59aad rx::mtl::CreateShaderLibrary(id<MTLDevice>, std::__1::basic_string<char, id<MTLDevice>::char_traits<char>, id<MTLDevice>::allocator<char> > const&, NSDictionary<NSString*, NSObject*>*, rx::mtl::AutoObjCPtr<NSError*>*) + 48
4   libANGLE-shared.dylib         	0x000000010ca94244 rx::ProgramMtl::createMslShaderLib(rx::mtl::Context*, gl::ShaderType, gl::InfoLog&, rx::mtl::TranslatedShaderInfo*, NSDictionary<NSString*, NSObject*>*) + 80
5   libANGLE-shared.dylib         	0x000000010ca93d53 rx::ProgramMtl::linkImplDirect(gl::Context const*, gl::ProgramLinkedResources const&, gl::InfoLog&) + 329
6   libANGLE-shared.dylib         	0x000000010ca93bd2 rx::ProgramMtl::link(gl::Context const*, gl::ProgramLinkedResources const&, gl::InfoLog&, std::__1::vector<gl::ProgramVaryingRef, std::__1::allocator<gl::ProgramVaryingRef> > const&) + 82
7   libANGLE-shared.dylib         	0x000000010ca77b13 gl::Program::linkImpl(gl::Context const*) + 1569
8   libANGLE-shared.dylib         	0x000000010ca77482 gl::Program::link(gl::Context const*) + 18
9   libANGLE-shared.dylib         	0x000000010c8fdf7f gl::Context::linkProgram(gl::ShaderProgramID) + 29
10  libANGLE-shared.dylib         	0x000000010c9663f2 gl::LinkProgram(unsigned int) + 111
11  com.apple.WebCore             	0x0000000106ae7306 WebCore::WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations(WebCore::WebGLProgram*) + 102 (WebGLRenderingContextBase.cpp:3987)
12  com.apple.WebCore             	0x0000000106bb9216 operator() + 139 (InspectorShaderProgram.cpp:197) [inlined]
13  com.apple.WebCore             	0x0000000106bb9216 bool WTF::__visitor_table<WTF::Visitor<WebCore::InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderType, WTF::String const&)::$_11, WebCore::InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderType, WTF::String const&)::$_12, WebCore::InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderType, WTF::String const&)::$_13>, std::__1::reference_wrapper<WebCore::WebGLProgram>, std::__1::reference_wrapper<WebCore::WebGPUPipeline>, WTF::Monostate>::__trampoline_func<std::__1::reference_wrapper<WebCore::WebGLProgram> >(WTF::Visitor<WebCore::InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderType, WTF::String const&)::$_11, WebCore::InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderType, WTF::String const&)::$_12, WebCore::InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderType, WTF::String const&)::$_13>&, WTF::Variant<std::__1::reference_wrapper<WebCore::WebGLProgram>, std::__1::reference_wrapper<WebCore::WebGPUPipeline>, WTF::Monostate>&) + 166 (Variant.h:1873)

[Truitt Savell]

Reverted r274927 for reason:

Broke many tests in WebGL

Committed r274948 (235705@main): <https://commits.webkit.org/235705@main>

[Kenneth Russell]

Looks like there were multiple WebGL layout test failures in:
https://build.webkit.org/results/Apple-BigSur-Release-WK2-Tests/r274927%20(1227)/results.html

crashes:
inspector/canvas/updateShader-webgl.html
webgl/2.0.0/conformance2/textures/image_bitmap_from_image_data/tex-2d-rgba4-rgba-unsigned_byte.html

failures:
webgl/1.0.3/conformance/limits/gl-max-texture-dimensions.html
webgl/1.0.3/conformance/textures/texture-size.html
webgl/2.0.0/conformance/limits/gl-max-texture-dimensions.html
webgl/2.0.0/conformance/textures/misc/texture-size.html

[Kenneth Russell]

Were there other failures on other bots that prompted the revert?

[Kyle Piddington]

(In reply to Kenneth Russell from comment #34)
> Were there other failures on other bots that prompted the revert?

We saw a large number of failures on Intel machines, which prompted the revert.

[Dean Jackson]

Committed r275088 (235798@main): <https://commits.webkit.org/235798@main>