Bug 394842 - Animation timeline's "Remove Frames and Pull" works inconsistently when the frame after the currently selected frame is null/empty/hold.
Summary: Animation timeline's "Remove Frames and Pull" works inconsistently when the f...
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Animation (show other bugs)
Version: git master (please specify the git hash!)
Platform: Compiled Sources Linux
: NOR minor
Target Milestone: ---
Assignee: Emmet O'Neill
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-29 22:29 UTC by Emmet O'Neill
Modified: 2018-06-26 15:10 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Krita file for testing for this bug. (200.25 KB, application/x-krita)
2018-06-05 21:37 UTC, Emmet O'Neill
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Emmet O'Neill 2018-05-29 22:29:52 UTC
Animation timeline's "Remove Frames and Pull" works inconsistently when the frame after the currently selected frame is null/empty/hold.

This is a minor bug with the "Remove Frames and Pull" action from the Animation Timeline's right-click context menu. 

This feature seems to work as expected when the frame *after* the selected frame contains a keyframe (including blank keyframes) - the currently selected frame is removed and all of the following frames are shifted back/left.

However, when the frame immediately *after* the selected frame is null/empty/hold, there is a bug where the selected frame is not removed, even though all subsequent frames are still shifted back. As such, the last/rightmost frame in the timeline can never be removed with "Remove Frames and Pull".

---

Here's how to test this:

You can test this by creating two adjacent frames. Draw an "A" on the first one and a "B" on the second. If you select the first (A) and try "Remove Frames and Pull" you'll see the expected behavior - the frame with the "A" will be deleted and the frame with the "B" will shift back to take its place. However, if you select the second frame (B) and try "Remove Frames and Pull" you'll see that neither frame is deleted, although the subsequent frames will still be shifted back.

---

This is probably a small error with indices or early-returning in either KisTimeBasedItemModel::removeFramesAndOffset or KisTimeBasedItemModel::createOffsetFramesCommand functions.
Comment 1 Emmet O'Neill 2018-06-05 21:37:43 UTC
Created attachment 113105 [details]
Krita file for testing for this bug.

Here's a krita file for testing for this bug. Frames contain letters from A to G.

Select the frame containing "D" and activate"Remove Frames and Pull"; it works correctly, removing the frame containing "D" and pulling/shifting all subsequent frames back.

Then select the frame containing "E" and activate "Remove Frames and Pull"; this does not work correctly, the "E" frame is not actually removed, even though subsequent frames are still shifted back.
Comment 2 Emmet O'Neill 2018-06-06 18:59:49 UTC
Git commit b2ae65a7456245caac86c37103e1fe8d7d5273b9 by Emmet O'Neill.
Committed on 06/06/2018 at 18:51.
Pushed by emmetoneill into branch 'master'.

Fixed Timeline's "Remove Frames/Columns and Pull" Actions.

Previously these actions failed to remove the last frame of the
selection if the next following frame was empty. Fixed now!

Revied by Dmitry. Help from Eoin. Thanks as always!

M  +1    -2    plugins/dockers/animation/kis_animation_curves_model.cpp
M  +37   -33   plugins/dockers/animation/kis_animation_utils.cpp
M  +4    -4    plugins/dockers/animation/kis_animation_utils.h
M  +12   -14   plugins/dockers/animation/kis_time_based_item_model.cpp
M  +4    -2    plugins/dockers/animation/kis_time_based_item_model.h
M  +4    -4    plugins/dockers/animation/tests/kis_animation_utils_test.cpp
M  +3    -3    plugins/dockers/animation/timeline_frames_model.cpp
M  +11   -10   plugins/dockers/animation/timeline_frames_view.cpp
M  +1    -1    plugins/dockers/animation/timeline_frames_view.h

https://commits.kde.org/krita/b2ae65a7456245caac86c37103e1fe8d7d5273b9
Comment 3 Andrey 2018-06-26 15:10:28 UTC
Git commit 2a7e1f2dc01c5db16c837172fbb6b927916251fa by Andrey Kamakin, on behalf of Emmet O'Neill.
Committed on 26/06/2018 at 14:18.
Pushed by akamakin into branch 'akamakin/T8628-multithreading-optimization'.

Fixed Timeline's "Remove Frames/Columns and Pull" Actions.

Previously these actions failed to remove the last frame of the
selection if the next following frame was empty. Fixed now!

Revied by Dmitry. Help from Eoin. Thanks as always!

M  +1    -2    plugins/dockers/animation/kis_animation_curves_model.cpp
M  +37   -33   plugins/dockers/animation/kis_animation_utils.cpp
M  +4    -4    plugins/dockers/animation/kis_animation_utils.h
M  +12   -14   plugins/dockers/animation/kis_time_based_item_model.cpp
M  +4    -2    plugins/dockers/animation/kis_time_based_item_model.h
M  +4    -4    plugins/dockers/animation/tests/kis_animation_utils_test.cpp
M  +3    -3    plugins/dockers/animation/timeline_frames_model.cpp
M  +11   -10   plugins/dockers/animation/timeline_frames_view.cpp
M  +1    -1    plugins/dockers/animation/timeline_frames_view.h

https://commits.kde.org/krita/2a7e1f2dc01c5db16c837172fbb6b927916251fa