blender
/
blender
136
406
Fork 589
Code Issues 6.2k Pull Requests 751 Projects 19 Wiki Activity

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
This commit is contained in:
Jason Fielder 2023-01-30 11:25:53 +01:00 committed by Clément Foucault
parent 57552f52b2
commit 6dde185dc4
Notes: blender-bot 2023-02-13 16:03:55 +01:00 
Referenced by issue #96261, Metal Viewport
1 changed files with 21 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
source/blender/gpu/metal/mtl_index_buffer.mm
View File

@ -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);
}
}
}
}