[webkit-reviews] review granted: [Bug 234317] [ANGLE] Roll ANGLE to 12-14-2021 (36fcf80b1f2) : [Attachment 447164] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Kyle Piddington
<kpiddington at apple.com>'s request for review:
Bug 234317: [ANGLE] Roll ANGLE to 12-14-2021 (36fcf80b1f2)
https://bugs.webkit.org/show_bug.cgi?id=234317

Attachment 447164: Patch

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




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

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

Reviewed all of changes.diff and the Metal backend changes.

A few comments, but nothing that should block integration. Let's move forward
to unblock John Cunningham's work, and others. r+

> Source/ThirdParty/ANGLE/changes.diff:549
> + 

Can this whitespace change be removed? It looks like it would eliminate a diff
block.

> Source/ThirdParty/ANGLE/changes.diff:571
> +-

Can this whitespace difference be removed?

> Source/ThirdParty/ANGLE/changes.diff:-1081
> --	if (ANGLE_APPLE_AVAILABLE_XI(10.15, 13.0))

We should roll forward to include your
https://chromium-review.googlesource.com/c/angle/angle/+/3338624 ASAP -
otherwise we'll lose ETC/ASTC support on M1 Macs in Safari, too.

> Source/ThirdParty/ANGLE/changes.diff:885
> + def gen_image_map_switch_string(image_table, angle_to_gl):

What causes this large diff in the format table generator? Can this be
investigated and resolved?

> Source/ThirdParty/ANGLE/changes.diff:1171
> +@@ -161,7 +159,7 @@

Additionally - what causes all the diffs in this generated file compared to
upstream?

> Source/ThirdParty/ANGLE/changes.diff:-3976
> -+	    

Can this whitespace diff be removed?

> Source/ThirdParty/ANGLE/changes.diff:2404
> ++	    variable_name, output_file))

Can these formatting changes be reconciled?

> Source/ThirdParty/ANGLE/changes.diff:2411
> ++

Can this whitespace change be removed?

> Source/ThirdParty/ANGLE/changes.diff:2419
> ++	    'echo "// Compiled binary for Metal default shaders.\n\n" >> 
\"{0}\"'.format(output_file))

Can this formatting difference be reconciled?

> Source/ThirdParty/ANGLE/changes.diff:2423
> ++

Can this whitespace change be reconciled?