Bug 219759

Created attachment 415937 [details] Patch

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

<rdar://problem/72200222>

Created attachment 415958 [details] Patch

Created attachment 416012 [details] Patch

Created attachment 416046 [details] Patch

This is amazing work! A couple of comments: - Does this really enable the Metal backend for ANGLE in WebKit by default? If so, could I suggest doing that in a separate patch? That way if it needs to be turned off due to any regressions, this large patch won't be reverted. - While the ANGLE update instructions in WebKit say to update changes.diff, at this point that file is automatically regenerated when ANGLE is rolled into WebKit via the Tools/Scripts/update-angle script. Including the changes in changes.diff doubles the size of this already-large patch, and makes it harder to review, so would you consider skipping that step? (Assuming doing so is OK with the other teammates) - Could you document in this bug description and in the ChangeLog more precisely what this patch implements? It looks like you've implemented support for transform feedback (fantastic!) - it would be really great to document that. - Would you consider uploading this patch upstream to the ANGLE project per https://github.com/google/angle/blob/master/doc/ContributingCode.md ? The ANGLE developers would love to work with you to get this work integrated upstream. Will add more comments on the patch itself. Thanks!

We won't enable this by default yet. I've already added an Internal setting for this, which means developers can toggle it (but not people using Safari Technology Preview).

We'll also post this to ANGLE as soon as possible, which is where the official review will happen. Landing it here is our way of testing it on more systems as it is officially integrated into ANGLE, and then copied back to WebKit.

Thanks Dean - all sounds great!

Seems like the patch is too big for the review tool. Here are some comments: - don't include changes.diff (Ken mentioned this) - We can probably leave the complete list of files out of the ChangeLog - The GCC_PREPROCESSOR_DEFINITIONS should go in the .xcconfig files - I don't think we should change ANGLE's ASSERT macro - src/common/platform.h checks for iOS < 13, but I don't believe we support that any more in WebKit - src/common/utilities.cpp should probably keep the clang #pragma I'll fix some of these.

This patch broke depth textures on iOS because it removed a bit of code that was intentionally not checking for some required depth formats (because iOS OpenGL doesn't support them). I expect this backend does support them, so we'll need to make this a runtime decision based on the backend. However, it should be ok to continue NOT checking :)

Comment on attachment 416046 [details] Patch Before landing I: - Fixed the depth texture and other failure - removed changes.diff - Put the ANGLE_ENABLE_METAL preprocessor definition in the configuration files - Reverted the changes to ANGLE's ASSERT - Reverted the change to the #pragma clang warning

Committed r270733: <https://trac.webkit.org/changeset/270733>