Differential D13376

Bugfix: Animation Timeline's "Remove Frames and Pull" now removes expect frames consistently.
ClosedPublic
Actions

Authored by emmetoneill on Jun 6 2018, 7:10 AM.

Details

Reviewers

dkazakov

Group Reviewers

Krita

Summary

Bugfix for https://bugs.kde.org/show_bug.cgi?id=394842.

Prior to this patch, "Remove Frames and Pull" would fail to remove the final frame of a given selection if the next frame was empty. This patch should fix that!

(Also, credit to Eoin for helping me nail it down! Thanks!)

Test Plan

Test that the "Remove Frames and Pull" action removes all selected keyframes as expected in any context. Also test "Remove Columns and Pull"!

Should work even with non-contiguous selections.

I have a simple test Krita file that's attached to the bug report linked above.

Diff Detail

Repository

R37 Krita

Lint

Lint Skipped

Unit

Unit Tests Skipped

emmetoneill created this revision.Jun 6 2018, 7:10 AM

Restricted Application added a reviewer: Krita. · View Herald TranscriptJun 6 2018, 7:10 AM

emmetoneill requested review of this revision.Jun 6 2018, 7:10 AM

emmetoneill edited the summary of this revision. (Show Details)Jun 6 2018, 7:13 AM

Hi, @emmetoneill!

The patch looks and works perfectly fine, the bug is fixed! Please push! :)

dkazakov accepted this revision.Jun 6 2018, 8:48 AM

This revision is now accepted and ready to land.Jun 6 2018, 8:48 AM

Ah! I've forgotten to add that you should add this patch to fix compilation. It looks like you don't compile unittests :)

And please also check if KisAnimationUtilsTest still passes fine after your patch. Don't do any review requests for that, just push directly :)

P233 Masterwork From Distant Lands

1	diff --git a/plugins/dockers/animation/tests/kis_animation_utils_test.cpp b/plugins/dockers/animation/tests/kis_animation_utils_test.cpp
2	index a846306..3143788 100644
3	--- a/plugins/dockers/animation/tests/kis_animation_utils_test.cpp
4	+++ b/plugins/dockers/animation/tests/kis_animation_utils_test.cpp
5	@@ -125,7 +125,7 @@ void KisAnimationUtilsTest::test()
6	frameMoves << std::make_pair(FrameItem(layer1, "content", 10), FrameItem(layer1, "content", 20));
7	frameMoves << std::make_pair(FrameItem(layer1, "content", 20), FrameItem(layer1, "content", 0));
8
9	- QScopedPointer<KUndo2Command> cmd(createMoveKeyframesCommand(frameMoves, false));
10	+ QScopedPointer<KUndo2Command> cmd(createMoveKeyframesCommand(frameMoves, false, false));
11	cmd->redo();
12
13	QVector<std::tuple<int, QRect, QRect>> referenceRects;
14	@@ -150,7 +150,7 @@ void KisAnimationUtilsTest::test()
15	frameMoves << std::make_pair(FrameItem(layer2, "content", 10), FrameItem(layer1, "content", 10));
16	frameMoves << std::make_pair(FrameItem(layer1, "content", 20), FrameItem(layer2, "content", 20));
17
18	- cmd.reset(createMoveKeyframesCommand(frameMoves, false));
19	+ cmd.reset(createMoveKeyframesCommand(frameMoves, false, false));
20	cmd->redo();
21
22	referenceRects << std::make_tuple( 0, QRect() , QRect( 0, 10, 10, 10));
23	@@ -176,7 +176,7 @@ void KisAnimationUtilsTest::test()
24	frameMoves << std::make_pair(FrameItem(layer2, "content", 10), FrameItem(layer1, "content", 10));
25	frameMoves << std::make_pair(FrameItem(layer2, "content", 20), FrameItem(layer1, "content", 20));
26
27	- cmd.reset(createMoveKeyframesCommand(frameMoves, false));
28	+ cmd.reset(createMoveKeyframesCommand(frameMoves, false, false));
29	cmd->redo();
30
31	referenceRects << std::make_tuple( 0, QRect( 0, 10, 10, 10), QRect( 0, 0, 10, 10));
32	@@ -203,7 +203,7 @@ void KisAnimationUtilsTest::test()
33	frameMoves << std::make_pair(FrameItem(layer2, "content", 10), FrameItem(layer1, "content", 9));
34	frameMoves << std::make_pair(FrameItem(layer2, "content", 20), FrameItem(layer1, "content", 20));
35
36	- cmd.reset(createMoveKeyframesCommand(frameMoves, false));
37	+ cmd.reset(createMoveKeyframesCommand(frameMoves, false, false));
38	cmd->redo();
39
40	referenceRects << std::make_tuple( 0, QRect( 0, 10, 10, 10), QRect( 0, 0, 10, 10));

Fixed and built the tests, and then pushed! Closing. Thanks again, Dmitry.

Revision Contents
Changeset List

			Path	Packages
M			plugins/dockers/animation/kis_animation_curves_model.cpp (3 lines)
M			plugins/dockers/animation/kis_animation_utils.h (8 lines)
M			plugins/dockers/animation/kis_animation_utils.cpp (70 lines)
M			plugins/dockers/animation/kis_time_based_item_model.h (6 lines)
M			plugins/dockers/animation/kis_time_based_item_model.cpp (26 lines)
M			plugins/dockers/animation/timeline_frames_model.cpp (6 lines)
M			plugins/dockers/animation/timeline_frames_view.h (2 lines)
M			plugins/dockers/animation/timeline_frames_view.cpp (21 lines)