[Bf-blender-cvs] [6dde185dc47] master: Metal: Fix edge-case with point primitive restart index removal where all indices are restarts.

Jason Fielder noreply at git.blender.org
Mon Jan 30 11:57:06 CET 2023


Commit: 6dde185dc470498d3eb2676a96728b5aef5e60ed
Author: Jason Fielder
Date:   Mon Jan 30 11:25:53 2023 +0100
Branches: master
https://developer.blender.org/rB6dde185dc470498d3eb2676a96728b5aef5e60ed

Metal: Fix edge-case with point primitive restart index removal where all indices are restarts.

Metal backend does not support primtiive restart for point primtiives. Hence strip_restart_indices removes restart indices by swapping them to the end of the index buffer and reducing the length.
An edge-case existed where all indices within the index buffer were restarts and no valid swap-index would be found, resulting in a buffer underflow.

Authored by Apple: Michael Parkin-White

Ref T96261

Reviewed By: fclem

Maniphest Tasks: T96261

Differential Revision: https://developer.blender.org/D17088

===================================================================

M	source/blender/gpu/metal/mtl_index_buffer.mm

===================================================================

diff --git a/source/blender/gpu/metal/mtl_index_buffer.mm b/source/blender/gpu/metal/mtl_index_buffer.mm
index c32f9299d02..cf921e43a4b 100644
--- a/source/blender/gpu/metal/mtl_index_buffer.mm
+++ b/source/blender/gpu/metal/mtl_index_buffer.mm
@@ -475,7 +475,7 @@ void MTLIndexBuf::strip_restart_indices()
 
       /* Find swap index at end of index buffer. */
       int swap_index = -1;
-      for (uint j = index_len_ - 1; j >= i; j--) {
+      for (uint j = index_len_ - 1; j >= i && index_len_ > 0; j--) {
         /* If end index is restart, just reduce length. */
         if (uint_idx[j] == 0xFFFFFFFFu) {
           index_len_--;
@@ -486,22 +486,26 @@ void MTLIndexBuf::strip_restart_indices()
         break;
       }
 
-      /* If swap index is not valid, then there were no valid non-restart indices
-       * to swap with. However, the above loop will have removed these indices by
-       * reducing the length of indices. Debug assertions verify that the restart
-       * index is no longer included. */
-      if (swap_index == -1) {
-        BLI_assert(index_len_ <= i);
-      }
-      else {
-        /* If we have found an index we can swap with, flip the values.
-         * We also reduce the length. As per above loop, swap_index should
-         * now be outside the index length range. */
-        uint32_t swap_index_value = uint_idx[swap_index];
-        uint_idx[i] = swap_index_value;
-        uint_idx[swap_index] = 0xFFFFFFFFu;
-        index_len_--;
-        BLI_assert(index_len_ <= swap_index);
+      /* If index_len_ == 0, this means all indices were flagged as hidden, with restart index
+       * values. Hence we will entirely skip the draw. */
+      if (index_len_ > 0) {
+        /* If swap index is not valid, then there were no valid non-restart indices
+         * to swap with. However, the above loop will have removed these indices by
+         * reducing the length of indices. Debug assertions verify that the restart
+         * index is no longer included. */
+        if (swap_index == -1) {
+          BLI_assert(index_len_ <= i);
+        }
+        else {
+          /* If we have found an index we can swap with, flip the values.
+           * We also reduce the length. As per above loop, swap_index should
+           * now be outside the index length range. */
+          uint32_t swap_index_value = uint_idx[swap_index];
+          uint_idx[i] = swap_index_value;
+          uint_idx[swap_index] = 0xFFFFFFFFu;
+          index_len_--;
+          BLI_assert(index_len_ <= swap_index);
+        }
       }
     }
   }



More information about the Bf-blender-cvs mailing list