Bug 407416 - Timeline 'Remove Layer' option does in fact delete the layer
Summary: Timeline 'Remove Layer' option does in fact delete the layer
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Animation (show other bugs)
Version: unspecified
Platform: Debian stable Linux
: NOR normal
Target Milestone: ---
Assignee: Krita Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-11 08:43 UTC by Ahab Greybeard
Modified: 2020-05-15 23:34 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahab Greybeard 2019-05-11 08:43:14 UTC
SUMMARY
When you use the top left icon in the Timeline docker to 'Add Existing Layer' to the timeline then the obvious matching/complementary action is to remove it from the timeline (but not to delete it from the image which is what happens).

STEPS TO REPRODUCE
1. In the Timeline docker, right click a layer (or click the small icon next to the audio icon at the top of the layer list) and click the Remove Layer option.

OBSERVED RESULT
The layer is deleted from the image.

EXPECTED RESULT
The layer should be removed from the timeline docker but not deleted from the image. 

SOFTWARE/OS VERSIONS
All versions.

ADDITIONAL INFORMATION
Comment 1 Halla Rempt 2019-05-11 08:56:10 UTC
Yes, this is quite wrong... Remove layer also seems confused about which layer gets removed.
Comment 2 Tiar 2019-06-19 13:41:16 UTC
Just below the "Remove layer" there is a checkbox "[ ] Show in Timeline", so I wouldn't say just changing the behaviour of Remove Layer from "remove entirely" to "remove from the timeline" is reasonable in this case; there is a way to remove layer from the timeline after all. And "Remove Layer" is the matching action for "New Layer" since the first one removes layer entirely from the layer stack, while the second adds an entirely new layer to the layer stack.

So I see two possibilities:
- change "Remove Layer" to "Delete Layer"
- remove "Remove Layer" action from the menu

Maybe there are even more.
Comment 3 Ahab Greybeard 2019-06-19 15:51:30 UTC
All the required functions are there in the Timeline docker but they are confusing. Until I got used to it, I often accidentally deleted a layer from the image by using Remove Layer.  'Add' and 'Remove' sound like and should be complementay actions within the Timeline docker.

Also, I don't think that the Timeline docker should be the place to create and delete layers from the image, that is the function of the Layers docker (my personal perception and personal opinion).
It is crowded in that area.

If the creation/deletion is to be kept in the Timeline docker, then the current 'Remove Layer' should be renamed as 'Delete Layer' and the current 'New Layer' should be renamed as 'Create (New) Layer'.

The current status/option tickbox 'Show in Timeline' (which is common with the Layers docker status/option for a layer) should be replaced by an action 'Remove from Timeline' because that will always be ticked when seen in the Timeline and only be used to remove a layer from within the Timeline.

'Add Existing Layer' and the (renamed) 'Remove from Timeline' should both be above the separator.
The (renamed) 'Create New Layer' and the (renamed) 'Delete Layer' should both be below the separator since they are secondary functions of the Timeline docker.
Comment 4 Tiar 2019-06-19 16:25:06 UTC
Ahab,
you're wrong about the "Show in Timeline" checkbox being always checked: when you select any layer it appears in the Timeline automatically, so there is a possibility that there is a layer in the Timeline that isn't added to the Timeline yet.

I have no issues with the rest.
Comment 5 Ahab Greybeard 2019-06-19 18:15:57 UTC
Yes, I made an assumption and didn't check it thoroughly so thank you for pointing that out.

This is actually a 'Show in Timeline (even if it's not selected)' option tickbox and has overlapping functionality with 'Add Existing Layer'.

I still think there should be a 'Remove From Timeline' action option. If the 'Show In Timeline' option tickbox is to remain in the Timeline docker then it should be in the middle, isolated by separators above and below it.
Comment 6 Scott Petrovic 2019-09-18 13:49:59 UTC
I think where people want complete layer control is some animators. They don't want to have both the layers docker and the timeline docker open when they work. They want to be able to manage layers and everything with just the timeline docker. This is probably how most other animation packages work. 

We might need to think about the usability with this. We might just need to work on some wording with these terms or how you work with the timeline to achieve this.
Comment 7 Emmet O'Neill 2020-04-08 20:21:06 UTC
Git commit 46f9f7e9130aa6ca2b7eef1ee7805df7568922dc by Emmet O'Neill.
Committed on 08/04/2020 at 19:54.
Pushed by emmetoneill into branch 'master'.

Animation UI: "Show on Timeline">"Pin to Timeline"

Terminology change. Adding a layer to the animation timeline even when
inactive will now be referred to as "pinning" that layer to the
timeline.

M  +1    -1    krita/data/shortcuts/krita_default.shortcuts
M  +1    -1    krita/data/shortcuts/paint_tool_sai_compatible.shortcuts
M  +1    -1    krita/data/shortcuts/photoshop_compatible.shortcuts
M  +1    -1    krita/data/shortcuts/tablet_pro.shortcuts
M  +3    -3    krita/krita.action
M  +8    -8    libs/image/kis_base_node.cpp
M  +2    -2    libs/image/kis_base_node.h
M  +4    -4    libs/image/kis_layer_utils.cpp
M  +4    -4    libs/libkis/Node.cpp
M  +4    -5    libs/libkis/Node.h
M  +8    -8    libs/ui/kis_node_manager.cpp
M  +1    -1    libs/ui/kis_node_manager.h
M  +1    -1    plugins/dockers/animation/kis_animation_utils.cpp
M  +1    -1    plugins/dockers/animation/kis_animation_utils.h
M  +4    -4    plugins/dockers/animation/tests/timeline_model_test.cpp
M  +1    -2    plugins/dockers/animation/timeline_docker.cpp
M  +3    -3    plugins/dockers/animation/timeline_frames_index_converter.cpp
M  +4    -4    plugins/dockers/animation/timeline_frames_model.cpp
M  +12   -14   plugins/dockers/animation/timeline_frames_view.cpp
M  +1    -1    plugins/dockers/animation/timeline_frames_view.h
M  +1    -1    plugins/dockers/animation/timeline_node_list_keeper.cpp
M  +1    -1    plugins/dockers/layerdocker/LayerBox.cpp
M  +2    -2    plugins/extensions/pykrita/sip/krita/Node.sip
M  +1    -1    plugins/impex/libkra/kis_kra_loader.cpp
M  +3    -3    plugins/impex/libkra/kis_kra_savexml_visitor.cpp
M  +3    -3    plugins/impex/libkra/tests/kis_kra_saver_test.cpp

https://invent.kde.org/kde/krita/commit/46f9f7e9130aa6ca2b7eef1ee7805df7568922dc
Comment 8 Emmet O'Neill 2020-05-15 23:34:16 UTC
Git commit bcce547d569722dd91e5691d01c410245aca5498 by Emmet O'Neill.
Committed on 15/05/2020 at 23:07.
Pushed by emmetoneill into branch 'emmetpdx/T12769/aninext'.

Animation UI: "Show on Timeline">"Pin to Timeline"

Terminology change. Adding a layer to the animation timeline even when
inactive will now be referred to as "pinning" that layer to the
timeline.

M  +1    -1    krita/data/shortcuts/krita_default.shortcuts
M  +1    -1    krita/data/shortcuts/paint_tool_sai_compatible.shortcuts
M  +1    -1    krita/data/shortcuts/photoshop_compatible.shortcuts
M  +1    -1    krita/data/shortcuts/tablet_pro.shortcuts
M  +3    -3    krita/krita.action
M  +8    -8    libs/image/kis_base_node.cpp
M  +2    -2    libs/image/kis_base_node.h
M  +4    -4    libs/image/kis_layer_utils.cpp
M  +4    -4    libs/libkis/Node.cpp
M  +4    -5    libs/libkis/Node.h
M  +8    -8    libs/ui/kis_node_manager.cpp
M  +1    -1    libs/ui/kis_node_manager.h
M  +1    -1    plugins/dockers/animation/kis_animation_utils.cpp
M  +1    -1    plugins/dockers/animation/kis_animation_utils.h
M  +4    -4    plugins/dockers/animation/tests/timeline_model_test.cpp
M  +1    -2    plugins/dockers/animation/timeline_docker.cpp
M  +3    -3    plugins/dockers/animation/timeline_frames_index_converter.cpp
M  +4    -4    plugins/dockers/animation/timeline_frames_model.cpp
M  +12   -14   plugins/dockers/animation/timeline_frames_view.cpp
M  +1    -1    plugins/dockers/animation/timeline_frames_view.h
M  +1    -1    plugins/dockers/animation/timeline_node_list_keeper.cpp
M  +1    -1    plugins/dockers/layerdocker/LayerBox.cpp
M  +2    -2    plugins/extensions/pykrita/sip/krita/Node.sip
M  +1    -1    plugins/impex/libkra/kis_kra_loader.cpp
M  +3    -3    plugins/impex/libkra/kis_kra_savexml_visitor.cpp
M  +3    -3    plugins/impex/libkra/tests/kis_kra_saver_test.cpp

https://invent.kde.org/kde/krita/commit/bcce547d569722dd91e5691d01c410245aca5498