[webkit-reviews] review granted: [Bug 227449] ANGLE Metal primitive restart range computation could index with size_t : [Attachment 432394] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 28 14:46:33 PDT 2021

Kenneth Russell <kbr at google.com> has granted Kimmo Kinnunen
<kkinnunen at apple.com>'s request for review:
Bug 227449: ANGLE Metal primitive restart range computation could index with

Attachment 432394: Patch


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

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

This is certainly easier to understand. r+ with a couple of small questions.

I defer to kpiddington's review if there's any further feedback.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:421
> +    for (size_t i = 0; i < numIndices; i++)

Would it be better to use "++i" here for consistency with the inner loop?

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:430
> +	   } while (i < numIndices && bufferData[i] == restartMarker);

Out of curiosity do you want to leave the restart indices in these ranges or
elide them?

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:445
> +		   mRestartIndices = calculateRestartRanges<uint8_t>(ctx,

How hot is this routine? If it's called very often then maybe there is an
advantage to continuing to pass mRestartIndices as an argument to reduce
copying (or is return value optimization expected to handle this)?

More information about the webkit-reviews mailing list