GPv3: Keyframes deleted on move and then crash #118509

New Issue

Lukas Tönne · 2024-02-20T14:35:13+01:00

Lukas Tönne commented

2024-02-20 14:35:13 +01:00

System Information
Operating system:
Graphics card:

Blender Version
Broken: 4.2

Short description of error
Moving keyframes for Grease Pencil 3 objects deletes some of them and crashes.

Exact steps for others to reproduce the error

Add some GP3 keyframes (only visible in dope sheet atm)
Select all keyframes and move them around. Some keyframes get deleted this way.
Repeat moving keyframes until it crashes (somewhat random)

All keyframes:

After moving some get deleted:

**System Information** Operating system: Graphics card: **Blender Version** Broken: 4.2 **Short description of error** Moving keyframes for Grease Pencil 3 objects deletes some of them and crashes. **Exact steps for others to reproduce the error** 1. Add some GP3 keyframes (only visible in dope sheet atm) 2. Select all keyframes and move them around. Some keyframes get deleted this way. 3. Repeat moving keyframes until it crashes (somewhat random) All keyframes: ![image](/attachments/227882dd-f87c-48b8-89e3-2ff92a967846) After moving some get deleted: ![image](/attachments/2ab60377-2b3b-4c74-a3b4-959366719bb4)

gp_keyframe_undo_crash.blend

928 KiB

image.png

3.6 KiB

image.png

4.9 KiB

Lukas Tönne added the

labels 2024-02-20 14:35:14 +01:00

Lukas Tönne added this to the Grease Pencil project 2024-02-20 14:35:21 +01:00

Pratik Borhade commented

2024-02-21 05:50:14 +01:00

Yes, can confirm the crash. cc @filedescriptor

Pratik Borhade added

and removed

labels 2024-02-21 05:50:28 +01:00

Pratik Borhade commented

2024-02-21 10:10:44 +01:00

#118334 looks related from function calls

Lukas Tönne commented

2024-02-21 16:10:33 +01:00

Not quite the same issue as #118334 i think. In this case the root cause seems to be the GreasePencil::move_duplicate_frames function.

Not quite the same issue as #118334 i think. In this case the root cause seems to be the `GreasePencil::move_duplicate_frames` function.

Lukas Tönne commented

2024-02-21 17:16:15 +01:00

There seem to be two distinct issues here:

Transform code for grease pencil keyframes handles selected keys one-by-one and does an implicit de-duplication after each keyframe. Example: list of keys [1,2,3,4] gets moved 1 to the right, should be [-,1,2,3]. But each moved key overwrites the previous value before it can be moved in turn: [1,2,3,4] -> [-,1,3,4] -> [-,-,1,4] -> [-,-,-,1]. This needs general fixes to the transform code, it shouldn't throw away information about keys before everything is transformed.
After moving keys the layer needs to be tagged with tag_frames_map_keys_changed but is only getting tag_frames_map_changed currently. This means the frames map gets updated, but the sorted_keys list is not. After transforming the sorted_keys list is longer than the actual frames map, which crashes soon.

There seem to be two distinct issues here: 1. Transform code for grease pencil keyframes handles selected keys one-by-one and does an implicit de-duplication after each keyframe. Example: list of keys `[1,2,3,4]` gets moved 1 to the right, should be `[-,1,2,3]`. But each moved key overwrites the previous value before it can be moved in turn: `[1,2,3,4]` -> `[-,1,3,4]` -> `[-,-,1,4]` -> `[-,-,-,1]`. This needs general fixes to the transform code, it shouldn't throw away information about keys before everything is transformed. 2. After moving keys the layer needs to be tagged with `tag_frames_map_keys_changed` but is only getting `tag_frames_map_changed` currently. This means the frames map gets updated, but the `sorted_keys` list is not. After transforming the `sorted_keys` list is longer than the actual frames map, which crashes soon.

Lukas Tönne commented

2024-02-22 12:10:24 +01:00

The root cause here is the layer API for adding/removing frames. This must be considered an "internal" API because any calls here require manually fixing the user count of drawings. Such user count fixes need access to the GreasePencil object data, it cannot be done using the GreasePencilLayer alone. Transform code for Grease Pencil only stores the layer, but not the GreasePencil ID itself, so it can't ensure valid drawing user counts during the transform.

The root cause here is the layer API for adding/removing frames. This must be considered an "internal" API because any calls here require manually fixing the user count of drawings. Such user count fixes need access to the `GreasePencil` object data, it cannot be done using the `GreasePencilLayer` alone. Transform code for Grease Pencil only stores the layer, but not the `GreasePencil` ID itself, so it can't ensure valid drawing user counts during the transform.

Lukas Tönne referenced a pull request that will close this issue

2024-03-08 14:29:57 +01:00

Fix #118509: Make sure to use valid indices in frame maps #119207

Lukas Tönne referenced this issue from a commit

2024-03-11 12:50:30 +01:00

Fix #118509: Make sure to use valid indices in frame maps

Lukas Tönne closed this issue

2024-03-11 12:50:31 +01:00

Blender Bot added

Status

Resolved

and removed