In order to enable the ANGLE Metal backend, we need to not regress any LayoutTests. At least the following are failing (just 1.0.3): [160/580] webgl/1.0.3/conformance/extensions/ext-frag-depth.html failed unexpectedly (text diff) [209/580] webgl/1.0.3/conformance/glsl/constructors/glsl-construct-bvec2.html failed unexpectedly (text diff) [222/580] webgl/1.0.3/conformance/context/context-hidden-alpha.html failed unexpectedly (text diff) [287/580] webgl/1.0.3/conformance/textures/copy-tex-image-2d-formats.html failed unexpectedly (text diff) [314/580] webgl/1.0.3/conformance/renderbuffers/framebuffer-object-attachment.html failed unexpectedly (text diff) [344/580] webgl/1.0.3/conformance/programs/program-test.html passed unexpectedly [360/580] webgl/1.0.3/conformance/rendering/line-loop-tri-fan.html failed unexpectedly (text diff) [371/580] webgl/1.0.3/conformance/glsl/constructors/glsl-construct-ivec2.html failed unexpectedly (text diff) [403/580] webgl/1.0.3/conformance/uniforms/out-of-bounds-uniform-array-access.html failed unexpectedly (text diff) [445/580] webgl/1.0.3/conformance/glsl/constructors/glsl-construct-vec2.html failed unexpectedly (text diff) [450/580] webgl/1.0.3/conformance/glsl/constructors/glsl-construct-vec-mat-corner-cases.html failed unexpectedly (text diff) [521/580] webgl/1.0.3/conformance/ogles/GL/mat/mat_041_to_046.html failed unexpectedly (test timed out, text diff) [521/580] webgl/1.0.3/conformance/ogles/GL/functions/functions_113_to_120.html failed unexpectedly (test timed out, text diff) [540/580] webgl/1.0.3/conformance/glsl/misc/shaders-with-invariance.html failed unexpectedly (text diff) [547/580] webgl/1.0.3/conformance/glsl/misc/shaders-with-uniform-structs.html failed unexpectedly (text diff) [568/580] webgl/1.0.3/conformance/glsl/misc/struct-specifiers-in-uniforms.html failed unexpectedly (text diff) [580/580] webgl/1.0.3/conformance/ogles/GL/functions/functions_121_to_126.html failed unexpectedly (test timed out, text diff)
<rdar://problem/73539537>
Created attachment 419865 [details] Patch
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment on attachment 419865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419865&action=review Nice work Kyle. Looks good to me overall. A few minor comments. r+ > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect.cpp:611 > + // Replace gl_FragData() with our globally defined fragdata gl_FragData is a variable rather than a function, so suggest removing the (). > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect.cpp:1139 > + else if ( outputVarying.name == "gl_FragDepthEXT") Remove inconsistent space after (. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect.cpp:1420 > + Strange whitespace. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/EmitMetal.cpp:1896 > + putAngle("texture2DRect"); Hoping these additions don't accidentally add support for rectangular textures in WebGL shaders. I think there are enough negative tests to prevent that. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/FixTypeConstructors.cpp:18 > +class FixTypeTraverser : public TIntermTraverser Could you please add a per-class comment about what this pass does? > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/FixTypeConstructors.cpp:23 > + bool visitAggregate(Visit visit, TIntermAggregate *aggregateNode) override Strange indentation. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/FixTypeConstructors.cpp:29 > + if(aggregateNode->isConstructor()) Space before "(". > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/FixTypeConstructors.cpp:34 > + Please leave a comment as to why nothing needs to be done. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/FixTypeConstructors.cpp:36 > + else if(retType.isVector()) Space before "(". Here again, and throughout. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/FixTypeConstructors.cpp:70 > + assert(!"Should not be reached in case of 0, or 4"); Why not? Please add comment. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/ProgramPrelude.cpp:648 > +ANGLE_ALWAYS_INLINE int ANGLE_int_clamp(int value, int minValue, int maxValue) Note: it might not be guaranteed that these ANGLE_ declarations won't conflict with something the user declares. See WebGL's reserved identifiers in https://www.khronos.org/registry/webgl/specs/latest/1.0/#4.3 . It's fine for this patch, just FYI. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/ReduceInterfaceBlocks.cpp:24 > class Reducer : public TIntermRebuild Possible to document what this (preexisting) class does? > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/ReduceInterfaceBlocks.cpp:29 > + IdGen & mIdGen; No space after "&" per ANGLE style. Here and below. > Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/ReduceInterfaceBlocks.cpp:51 > + //Create instance variable Could you document a little bit more what this instance variable is being created for? > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm:438 > + mIOSurfaceTexture->setColorWritableMask(MTLColorWriteMaskAll & (~MTLColorWriteMaskAlpha)); It's great that disabling alpha writes into these IOSurfaces works with Metal. When attempting to do this with OpenGL there were some cases that were broken - for example BlitFramebuffer into the texture would destroy the alpha channel. There are workarounds at higher levels (in Chromium, at least) that should be removed once the Metal backend's turned on.
Committed r273188 (234374@main): <https://commits.webkit.org/234374@main>
Re-opened since this is blocked by bug 222218
Reverted r273188 for reason: Broke the build: Is missing a cpp file Committed r273189 (234375@main): <https://commits.webkit.org/234375@main>
Committed r273200 (234386@main): <https://commits.webkit.org/234386@main>