ANGLE Metal primitive restart range computation could index with size_t It'd be simpler to understand that the index does not overflow. The index limit is size_t.
Created attachment 432393 [details] Patch
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Created attachment 432394 [details] Patch
Comment on attachment 432394 [details] Patch 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, getCurrentBuffer()); 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)?
Created attachment 432457 [details] Patch for landing
Comment on attachment 432394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432394&action=review >> 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? restart indices should be in the ranges. >> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:445 >> + mRestartIndices = calculateRestartRanges<uint8_t>(ctx, getCurrentBuffer()); > > 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)? RVO is expected. It's not hot, it's called during draw, but only once after the buffer is modified.
Committed r279373 (239239@main): <https://commits.webkit.org/239239@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432457 [details].
<rdar://problem/79913301>