Bug 220076 (anglemetal)

<rdar://problem/72565020>

Created attachment 416641 [details]
Patch

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

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

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)

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

A simple test calling readPixels twice works though.

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.)

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

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).

The ANGLEPlatformDisplayMap uses the native display as the key.

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.

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.

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.

Blocked on 220877

And b220895

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

Created attachment 421117 [details]
Patch

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.

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)

Created attachment 422149 [details]
EWS Test

Only 4 test failures, looking really good!!!

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

Created attachment 422488 [details]
Another EWS test

Created attachment 423795 [details]
EWS Test

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

Created attachment 423947 [details]
EWS Test 5 or so

Created attachment 424056 [details]
EWS Test 6

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

(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

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)

Reverted r274927 for reason:

Broke many tests in WebGL

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

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

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

(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.

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