NLA: Swap strips fixes #110074

Guillermo Venegas · 2023-07-13T19:50:14+02:00

Guillermo Venegas commented

2023-07-13 19:50:14 +02:00

Fixes a bug where disallows swap strips in certain cases:

If there are only 2 strips selected, but there is other strip is on the left in the same nla track
If there are only 2 clusters selected, but one is on the left in the nla track
and have 2 strips selected.

Why happens:
Currently to swap strips the code make temp meta strips, it checks if the first strip
in the nla track is a temp meta strip and if it has only 2 strips and if it does, it separates this
temp meta strip back into the strips.
This is one specific case that don't work well or all cases.

The solution:

The operator should allow to swap 2 strips or 2 temp meta strips (clusters)

To make this possible check how many temp meta strips(clusters) are selected,
if they are only 2 try to swap them.

If they are 3 temp meta strips(clusters) the swap should fail.

If they are only one temp meta strips(clusters) break it into separate
strips and check if there are only 2 strips selected to make the swap

Fixes a bug where disallows swap strips in certain cases: 1. If there are only 2 strips selected, but there is other strip is on the left in the same nla track 2. If there are only 2 clusters selected, but one is on the left in the nla track and have 2 strips selected. Why happens: Currently to `swap strips` the code make `temp meta strips`, it checks if the first strip in the `nla track` is a `temp meta strip` and if it has only `2 strips` and if it does, it separates this `temp meta strip` back into the strips. This is one specific case that don't work well or all cases. The solution: The operator should allow to swap 2 strips or 2 `temp meta strips` (clusters) To make this possible check how many `temp meta strips`(clusters) are selected, if they are only 2 try to swap them. If they are 3 `temp meta strips`(clusters) the swap should fail. If they are only one `temp meta strips`(clusters) break it into separate `strips` and check if there are only 2 `strips` selected to make the swap

Guillermo Venegas changed title from ~~NLA: Keep bounds on strips swap~~ to NLA: Keep bounds on strips swap and fixes

2023-07-19 06:02:48 +02:00

Guillermo Venegas commented

2023-07-19 06:16:26 +02:00

Bugs	Patch applied

| Bugs| Patch applied| | ----------- | ----------- | | <video src="/attachments/12c598f1-13e5-4980-9c54-602ab1708a72" title="2023-07-18 22-08-45.mp4" controls></video>| <video src="/attachments/5e94c9ea-cb27-4106-8c12-f8df5ddc7413" title="2023-07-18 22-11-15.mp4" controls></video>|

2023-07-18 22-11-15.mp4

2.2 MiB

2023-07-18 22-08-45.mp4

2.6 MiB

Iliya Katushenock added this to the Animation & Rigging project 2023-07-19 11:47:36 +02:00

Guillermo Venegas force-pushed improve-swap-strips from f87ad6bb6e to 3bc8643076

2023-07-25 16:07:26 +02:00

Compare

Guillermo Venegas changed title from ~~NLA: Keep bounds on strips swap and fixes~~ to NLA: Swap strips fixes

2023-07-25 16:07:45 +02:00

Guillermo Venegas requested review from Sybren A. Stüvel 2023-07-25 16:08:41 +02:00

Nathan Vegdahl commented

2023-07-31 15:17:39 +02:00

@guishe You have both this and #109980 open for fixing #109081. Do you think one or the other is better? And can you elaborate on what the pros/cons of the two different fixes are?

👍 1

Guillermo Venegas commented

2023-08-01 06:16:55 +02:00

Honestly I don't have much to say about #109980, because after using the shortcut and and see that it fail the user has to fix positions to be able to use the shortcut again.

This pr makes swap directly adjust the position of the strip that will be placed to the right if necessary, then the user can make necessary adjustments if needed. The only drawback is that the original position is not recovered after another swap, due to the necessary adjustment that was made in the first iteration (as seen in the video).

Honestly I don't have much to say about https://projects.blender.org/blender/blender/pulls/109980, because after using the shortcut and and see that it fail the user has to fix positions to be able to use the shortcut again. This pr makes swap directly adjust the position of the strip that will be placed to the right if necessary, then the user can make necessary adjustments if needed. The only drawback is that the original position is not recovered after another swap, due to the necessary adjustment that was made in the first iteration (as seen in the video). <video src="/attachments/07b91769-a041-4973-b10a-0d04fd1ec3a9" title="2023-07-31 22-13-13.mp4" controls></video>

2023-07-31 22-13-13.mp4

1.8 MiB

👍 1

Nathan Vegdahl commented

2023-08-01 10:59:13 +02:00

Got it. Thanks for the explanation! I'll add this to the animation & rigging module meeting agenda (this Thursday), to see which solution people prefer.

👍 1

Guillermo Venegas force-pushed improve-swap-strips from 3bc8643076 to e0efc234c8

2023-08-04 20:10:39 +02:00

Compare

Guillermo Venegas added 1 commit 2023-08-04 20:12:42 +02:00

6604474f76 fixes

Guillermo Venegas added 1 commit 2023-08-04 20:14:03 +02:00

640f88956d remove unwanted change

Guillermo Venegas commented

2023-08-04 20:38:50 +02:00

Separate the pr in 2 to better explain the issues separately: (see #110074 (comment))

Separate the pr in 2 to better explain the issues separately: (see https://projects.blender.org/blender/blender/pulls/110074#issue-100084)

Guillermo Venegas commented

2023-08-04 20:39:41 +02:00

The fix that i mention that was a alternative i will move it to another pr #109980

Nathan Vegdahl commented

2023-08-07 12:18:45 +02:00

Thank you for your patience!

In the animation & rigging module meeting, we decided that #109980 is the most appropriate fix. I'll comment there with more details.

Thank you for your patience! In the animation & rigging module meeting, we decided that #109980 is the most appropriate fix. I'll comment there with more details.

Nathan Vegdahl referenced this pull request

2023-08-07 12:26:31 +02:00

Fix #109081: Ensure swap strips do not overlap each other. #109980

Nathan Vegdahl commented

2023-08-14 17:07:51 +02:00

Closing in favor of #109980.

Nathan Vegdahl closed this pull request

2023-08-14 17:07:53 +02:00