[webkit-reviews] review granted: [Bug 227596] REGRESSION (r279466): [Big Sur] webgl/1.0.3/conformance & webgl/2.0.0/conformance are failing : [Attachment 432836] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jul 4 18:31:59 PDT 2021


Kenneth Russell <kbr at google.com> has granted Kyle Piddington
<kpiddington at apple.com>'s request for review:
Bug 227596: REGRESSION (r279466):  [Big Sur] webgl/1.0.3/conformance &
webgl/2.0.0/conformance are failing
https://bugs.webkit.org/show_bug.cgi?id=227596

Attachment 432836: Patch

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




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

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

Looks good. r+ also based on johncunningham@'s review. One small comment.

Note to save time that you don't need to request review on subsequent patches -
you can just change the ChangeLogs to say "Reviewed by Kenneth Russell." and
cq+.

> Source/ThirdParty/ANGLE/include/platform/FeaturesMtl.h:133
> +    Feature intelDisableFastMathWorkaround = {

This feature doesn't need to end in "Workaround", and this name is confusing to
read - it sounds like it's disabling a workaround related to fast math.
intelDisableFastMath would be clearer in my opinion.

> Source/ThirdParty/ANGLE/include/platform/FeaturesMtl.h:134
> +	       "intel_disable_fast_math_workarounds",
FeatureCategory::MetalWorkarounds,

Please name the same as the feature - workaround instead of workarounds in this
case.


More information about the webkit-reviews mailing list