Fix #118509: Grease Pencil frames no longer disappearing while being transformed in the timeline #119337

Amélie Fondevilla · 2024-03-11T18:14:26+01:00

2024-03-11 18:14:26 +01:00

While selecting and transforming multiple grease pencil frames in the timeline, frames would sometimes disappear.
This happened when the transformation overlapped, meaning when a frame replaced another moving one in the timeline.
The frames transformation was happening in place and in series, and thus leaded to the initial position
of the frame to be removed, even if it was occupied by a freshly transformed framed.

This commit fixes the issue by storing two separate maps in the transform data structure instead of one,
one map to store the static data, meaning the frames that are not affected by the transform,
and another one for the moving data, meaning the frames that are affected by the transform.

Some changes were made to account for the potential duplication of the frames (move + duplicate) :

the map of the duplicated frames was renamed to a more explicit name,
when a duplication occurs, the original frame is stored in the static frames map,
and the duplicate is stored in the moving frames map,
thus the check for the duplicate flag of the operator is now done at the
init step of the transform, instead of the update step, so that the maps are initialized accordingly.

While selecting and transforming multiple grease pencil frames in the timeline, frames would sometimes disappear. This happened when the transformation overlapped, meaning when a frame replaced another moving one in the timeline. The frames transformation was happening in place and in series, and thus leaded to the initial position of the frame to be removed, even if it was occupied by a freshly transformed framed. This commit fixes the issue by storing two separate maps in the transform data structure instead of one, one map to store the static data, meaning the frames that are not affected by the transform, and another one for the moving data, meaning the frames that are affected by the transform. Some changes were made to account for the potential duplication of the frames (move + duplicate) : * the map of the duplicated frames was renamed to a more explicit name, * when a duplication occurs, the original frame is stored in the static frames map, and the duplicate is stored in the moving frames map, * thus the check for the duplicate flag of the operator is now done at the init step of the transform, instead of the update step, so that the maps are initialized accordingly.

Amélie Fondevilla added 3 commits 2024-03-11 18:14:38 +01:00

6d06713129 Fix disappearing frames when moving multiple frames

02375a513d Minor change: Vector to Span in function call

1665d6e569 Use the operator flag for duplicate in the init function instead of the update function

Amélie Fondevilla added this to the Grease Pencil project 2024-03-11 18:14:43 +01:00

Amélie Fondevilla requested review from Lukas Tönne 2024-03-11 18:15:01 +01:00

Amélie Fondevilla requested review from Falk David 2024-03-11 18:15:01 +01:00

Hans Goudey changed title from ~~Fix #118509 : Grease Pencil frames not longer disappearing while being transformed in the timeline~~ to Fix #118509: Grease Pencil frames not longer disappearing while being transformed in the timeline

2024-03-11 20:17:42 +01:00

Amélie Fondevilla changed title from ~~Fix #118509: Grease Pencil frames not longer disappearing while being transformed in the timeline~~ to Fix #118509: Grease Pencil frames no longer disappearing while being transformed in the timeline

2024-03-12 09:03:07 +01:00

Falk David approved these changes 2024-03-13 17:52:39 +01:00

Falk David left a comment

Thanks! Code looks ok to me. Also gave this a quick test and it works well 👍

Lukas Tönne commented

2024-03-14 11:42:17 +01:00

I managed to make it crash, looks like the issue with invalidated drawing_index is still around.
Steps:

make a bunch of keyframes
scale keyframes so they collapse onto the same frames
translate keyframes until they overlap the current frame

I managed to make it crash, looks like the issue with invalidated `drawing_index` is still around. Steps: 1. make a bunch of keyframes 2. __scale__ keyframes so they collapse onto the same frames 3. __translate__ keyframes until they overlap the current frame

2024-03-14 11-40-59.mp4

259 KiB

gp_keyframe_undo_crash4.blend

930 KiB

Falk David reviewed 2024-03-20 10:51:19 +01:00

Falk David left a comment

While talking with @LukasTonne about this we agree that it might be better to switch out the runtime Map representation for one that just uses two Arrays (an array for the sorted keys and one for the values/frames). That would likely make it possible to deal with duplicate keys on the same frame gracefully. I'll look at this again when I have more time to come up with a design.

While talking with @LukasTonne about this we agree that it might be better to switch out the runtime `Map` representation for one that just uses two `Array`s (an array for the sorted keys and one for the values/frames). That would likely make it possible to deal with duplicate keys on the same frame gracefully. I'll look at this again when I have more time to come up with a design.

👍 1

Lukas Tönne commented

2024-04-09 11:37:48 +02:00

@amelief I'll continue working on this, hope you don't mind!

Amélie Fondevilla commented

2024-04-09 11:45:42 +02:00

Thank you @LukasTonne , I don't mind at all.

Lukas Tönne referenced this pull request

2024-04-09 16:10:42 +02:00

Fix #118509: GPv3: fix broken drawing array compression function #120433

Lukas Tönne commented

2024-04-09 16:15:49 +02:00

Ok, so it turned out there are actually two independent issues here, one them fixed by this PR and the other by #120433:

The disappearing keyframes during transform are fixed here. This is not, however, causing a crash by itself.
The remove_drawings_with_no_users function was broken when removing more than one drawing (e.g. when scaling down a range of keyframes). This is what causes the invalid drawing indices and is fixed by #120433.

Thanks for working on this! It should be fine as is. I will update the branch with current main, do a test run and finish it.

Ok, so it turned out there are actually two independent issues here, one them fixed by this PR and the other by #120433: - The disappearing keyframes during transform are fixed here. This is not, however, causing a crash by itself. - The `remove_drawings_with_no_users` function was broken when removing more than one drawing (e.g. when scaling down a range of keyframes). This is what causes the invalid drawing indices and is fixed by #120433. Thanks for working on this! It should be fine as is. I will update the branch with current main, do a test run and finish it.

Lukas Tönne added 2 commits 2024-04-09 16:17:58 +02:00

b52833d88f Merge branch 'main' into gpv3-fix-disappearing-frames

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

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

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

7375d13b80 Merge branch 'main' into gpv3-fix-disappearing-frames

Lukas Tönne approved these changes 2024-04-09 16:18:22 +02:00

Lukas Tönne commented

2024-04-09 16:18:35 +02:00