Fix #113740: Crash adding layer into empty group #113852

Falk David · 2023-10-17T17:55:10+02:00

Falk David commented

2023-10-17 17:55:10 +02:00

This crash was cause by some mismatch in the indices.
We need to make sure to clamp the insertion index to the size of the
span of the layers.

This crash was cause by some mismatch in the indices. We need to make sure to clamp the insertion index to the size of the span of the layers.

Falk David added 1 commit 2023-10-17 17:55:23 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

e67e20590c Fix #113740 : Crash adding layer into empty group

This crash was cause by some mismatch in the indices.
We need to make sure to clamp the insertion index to the size of the
span of the layers.

Falk David commented

2023-10-17 17:58:46 +02:00

@blender-bot build

Falk David merged commit 78cb2776bb into main

2023-10-17 18:34:50 +02:00

Falk David deleted branch fix-113740

2023-10-17 18:34:52 +02:00

Falk David referenced this issue from a commit

2023-10-17 18:34:53 +02:00

Fix #113740: Crash adding layer into empty group

Douglas Paul commented

2023-10-17 19:35:09 +02:00

Although this avoids the crash by clamping to_index to a valid value, I believe it still ends up choosing the wrong value for to_index, which I think means it incorrectly re-orders the custom data.

For example, starting with this (with no prior re-ordering):

If I move GP_Layer into GP_Group, then the correct value of to_index should be 1 (because GP_Layer.002 should stay last, at index 2). But the logic in find_layer_insertion_index() chooses a value of 2. (Prior to this patch it would have chosen a value of 3, which was also wrong, but that problem was masked by the resulting crash due to the out-of-bounds index.)

So, I will file a new issue for this, but wanted to document it here first for posterity, and so I could link to it in chat instead of pasting this as a wall of text there.

Although this avoids the crash by clamping `to_index` to a valid value, I believe it still ends up choosing the wrong value for `to_index`, which I think means it incorrectly re-orders the custom data. For example, starting with this (with no prior re-ordering): ![Screenshot 2023-10-17 at 12.52.26 PM.png](/attachments/dd640166-c376-480b-8d61-8ac3658d7457) If I move GP_Layer into GP_Group, then the correct value of `to_index` should be `1` (because GP_Layer.002 should stay last, at index 2). But the logic in `find_layer_insertion_index()` chooses a value of `2`. (Prior to this patch it would have chosen a value of `3`, which was also wrong, but that problem was masked by the resulting crash due to the out-of-bounds index.) So, I will file a new issue for this, but wanted to document it here first for posterity, and so I could link to it in chat instead of pasting this as a wall of text there.

Screenshot 2023-10-17 at 12.52.26 PM.png

38 KiB

Douglas Paul referenced this pull request

2023-10-18 07:52:39 +02:00

GPv3: Fix bugs in find_layer_insertion_index() and some of its usages #113874

Sign in to join this conversation.

No reviewers