Bug 406697

What you describe in the NOTES is at least intentional; the other actions should not be possible.

If two developers agree on something being a bug it is confirmed.

Git commit bac3cdd85570404f17c156d79a4200f9932b4d92 by Dmitry Kazakov.
Committed on 14/08/2020 at 19:54.
Pushed by dkazakov into branch 'krita/4.3'.

Fix flatten/merge down/merge multiple action handle locked groups correctly

This patch extends the already existing strategy, which was previously
used for merge down operation only:

1) If there is at least one unlocked layer in the selection, then the
   merge will happen.
2) The merge result will be written into nearest unlocked parent
3) All unlocked layers of the selection will be removed
4) All locked layers will be kept in the layers stack, but switched
   into *invisible* state.
CC:kimageshop@kde.org

M  +85   -20   libs/image/kis_layer_utils.cpp
M  +18   -0    libs/ui/kis_layer_manager.cc

https://invent.kde.org/graphics/krita/commit/bac3cdd85570404f17c156d79a4200f9932b4d92

Git commit 7db68af4d1b9fd5816eb8d206627ec4231dfc672 by Dmitry Kazakov.
Committed on 14/08/2020 at 19:56.
Pushed by dkazakov into branch 'krita/4.3'.

Fix layer duplication handle locked layers properly

M  +7    -0    libs/ui/kis_node_juggler_compressed.cpp

https://invent.kde.org/graphics/krita/commit/7db68af4d1b9fd5816eb8d206627ec4231dfc672

Git commit fe9741d27cb87a59b3add558489ce5b407a35d9a by Dmitry Kazakov.
Committed on 14/08/2020 at 20:02.
Pushed by dkazakov into branch 'master'.

Fix flatten/merge down/merge multiple action handle locked groups correctly

This patch extends the already existing strategy, which was previously
used for merge down operation only:

1) If there is at least one unlocked layer in the selection, then the
   merge will happen.
2) The merge result will be written into nearest unlocked parent
3) All unlocked layers of the selection will be removed
4) All locked layers will be kept in the layers stack, but switched
   into *invisible* state.
CC:kimageshop@kde.org

M  +85   -20   libs/image/kis_layer_utils.cpp
M  +18   -0    libs/ui/kis_layer_manager.cc

https://invent.kde.org/graphics/krita/commit/fe9741d27cb87a59b3add558489ce5b407a35d9a

Git commit bccda781066a1e9be37d45ad6e5db6aa5d7f70b1 by Dmitry Kazakov.
Committed on 14/08/2020 at 20:02.
Pushed by dkazakov into branch 'master'.

Fix layer duplication handle locked layers properly

M  +7    -0    libs/ui/kis_node_juggler_compressed.cpp

https://invent.kde.org/graphics/krita/commit/bccda781066a1e9be37d45ad6e5db6aa5d7f70b1

Git commit 9fe41211deb5a77aa501e379086df93e980240ab by Dmitry Kazakov.
Committed on 17/08/2020 at 22:57.
Pushed by dkazakov into branch 'krita/4.3'.

Make sure the user cannot "cut" locked layer

M  +24   -3    libs/ui/kis_node_manager.cpp

https://invent.kde.org/graphics/krita/commit/9fe41211deb5a77aa501e379086df93e980240ab

Git commit 1893c0a79a225d01b0bb364bafe6bb37cd23315c by Dmitry Kazakov.
Committed on 17/08/2020 at 22:57.
Pushed by dkazakov into branch 'krita/4.3'.

Fix actions not to modify locked layers

1) When trying to D&D **from** a locked layer Krita will
   force a "copy" mode

2) When trying to D&D **into** a locked group the operation will
   be forbidden. If the user still managed to do that, the layer
   will be put into a new position.

3) Layer->Properties should not be allowed for locked layers or
   masks.

4) Layer->Convert should not be allowed for locked layers

5) Lower/Raise node should not move nodes in a locked group

6) Lower/Raise node should not try to enter a locked group
Related: bug 425412

M  +18   -5    libs/ui/kis_layer_manager.cc
M  +9    -15   libs/ui/kis_mask_manager.cc
M  +17   -13   libs/ui/kis_node_juggler_compressed.cpp
M  +84   -29   libs/ui/kis_node_manager.cpp
M  +8    -1    libs/ui/kis_node_manager.h
M  +7    -3    libs/ui/kis_node_model.cpp

https://invent.kde.org/graphics/krita/commit/1893c0a79a225d01b0bb364bafe6bb37cd23315c

Git commit 36d91be01f3059bc1c8db9a814d1248021047c34 by Dmitry Kazakov.
Committed on 17/08/2020 at 22:57.
Pushed by dkazakov into branch 'krita/4.3'.

Make sure pasted (and D&D'ed) layers are not pasted into locked groups

M  +1    -1    libs/ui/kis_mimedata.cpp

https://invent.kde.org/graphics/krita/commit/36d91be01f3059bc1c8db9a814d1248021047c34

Git commit e885115b21a96c47734a126fd3de43f71e897f95 by Dmitry Kazakov.
Committed on 17/08/2020 at 23:00.
Pushed by dkazakov into branch 'master'.

Fix actions not to modify locked layers

1) When trying to D&D **from** a locked layer Krita will
   force a "copy" mode

2) When trying to D&D **into** a locked group the operation will
   be forbidden. If the user still managed to do that, the layer
   will be put into a new position.

3) Layer->Properties should not be allowed for locked layers or
   masks.

4) Layer->Convert should not be allowed for locked layers

5) Lower/Raise node should not move nodes in a locked group

6) Lower/Raise node should not try to enter a locked group
Related: bug 425412

# Conflicts:
#	libs/ui/kis_layer_manager.cc
#	libs/ui/kis_mask_manager.cc

M  +19   -4    libs/ui/kis_layer_manager.cc
M  +9    -15   libs/ui/kis_mask_manager.cc
M  +17   -13   libs/ui/kis_node_juggler_compressed.cpp
M  +84   -29   libs/ui/kis_node_manager.cpp
M  +8    -1    libs/ui/kis_node_manager.h
M  +7    -3    libs/ui/kis_node_model.cpp

https://invent.kde.org/graphics/krita/commit/e885115b21a96c47734a126fd3de43f71e897f95

Git commit f4ad7de3bcdc285729e0764dd850210b05af1380 by Dmitry Kazakov.
Committed on 17/08/2020 at 22:57.
Pushed by dkazakov into branch 'master'.

Make sure the user cannot "cut" locked layer

M  +24   -3    libs/ui/kis_node_manager.cpp

https://invent.kde.org/graphics/krita/commit/f4ad7de3bcdc285729e0764dd850210b05af1380

Git commit f0c27aa07d3bda35a1bec298c1345c4331f2a6f4 by Dmitry Kazakov.
Committed on 17/08/2020 at 22:57.
Pushed by dkazakov into branch 'master'.

Make sure pasted (and D&D'ed) layers are not pasted into locked groups

M  +1    -1    libs/ui/kis_mimedata.cpp

https://invent.kde.org/graphics/krita/commit/f0c27aa07d3bda35a1bec298c1345c4331f2a6f4

Git commit c1ab1ee97f196e73d8340918e2d9b7a79eafbbca by Agata Cacko.
Committed on 18/11/2020 at 18:45.
Pushed by tymond into branch 'master'.

Fix assert on New Layer From Visible on invisible active layer

Before this commit, Krita would assert on New Layer from Visible
when used on invisible layer. This commit fixes this.

Explanation:
Buckle up, it's going to be a ride :)

In 2016 there was a problem that invisible layers would show up
when they were merged, see bug 359707.
https://bugs.kde.org/show_bug.cgi?id=359707
As a solution, "just remove the layers separately instead of
merging" strategy was used.

In 2018 it turned out that in case when putAfter (the layer
Krita puts the merged result above of) is invisible, it gets
removed, so then it's null and Krita tries to merge other layers
in instead of putAfter, see bug 395903.
https://bugs.kde.org/show_bug.cgi?id=395903
As a solution, if putAfter was invisible, one of the visible
nodes-to-be-merged were assigned to putAfter to replace the
original putAfter node, to make sure that when original putAfter
is removed, some other layer is doing its job.
(The fact that it's a soon-to-be-merged layer is not a problem).

In 2020 it turned out that there is quite a lot of actions
that are possible in locked groups. It required some more changes
to the code. See bug 406697.
https://bugs.kde.org/show_bug.cgi?id=406697
A solution included a change to the relevant code such that
invisible putAfter would be unconditionally replaced with
one of the nodes from the to-be-merged list, and then
if putAfter had no parent, Krita would encounter an assert.

Problem with this behaviour:
In case of newLayerFromVisible(), mergedNodes list would only
contain a root node (which has no parent). If the putAfter is
invisible, then it's replaced with "one of" mergedNodes, which
means the root node. Which means it's going to assert.

However it's not always necessary to replace putAfter with one
of the to-be-merged nodes. It's only needed when it's going to
be removed. Hence moving the replacing into the if.
Because root is not invisible, the list of invisible nodes
will be empty and nothing will be replaced and nothing will
be removed.

NOTE: I also added a new condition: '&& cleanupNodes'.
It's not stricly necessary to cover all usecases of
mergeMultipleNodesImpl, because the only usecase where
cleanupNodes is false is the newLayerFromVisible case
(which will fail the first condition).
However it makes it more future-proof since the only case
those nodes needs to be removed is when cleanupNodes is true
(otherwise removing nodes would be a dataloss bug).

NOTE2: It's possible that putAfter replacement needs one more
check (to see if it is in fact in the invisibleNodes list).
I couldn't find a situation  where it was needed, though,
so I left it as it is.
Related: bug 428683, bug 359707, bug 395903

M  +15   -8    libs/image/kis_layer_utils.cpp

https://invent.kde.org/graphics/krita/commit/c1ab1ee97f196e73d8340918e2d9b7a79eafbbca

Git commit d25e08a3c6298f98850e1f22284f30536425eec1 by Agata Cacko.
Committed on 19/11/2020 at 15:49.
Pushed by tymond into branch 'krita/4.3'.

Fix assert on New Layer From Visible on invisible active layer

Before this commit, Krita would assert on New Layer from Visible
when used on invisible layer. This commit fixes this.

Explanation:
Buckle up, it's going to be a ride :)

In 2016 there was a problem that invisible layers would show up
when they were merged, see bug 359707.
https://bugs.kde.org/show_bug.cgi?id=359707
As a solution, "just remove the layers separately instead of
merging" strategy was used.

In 2018 it turned out that in case when putAfter (the layer
Krita puts the merged result above of) is invisible, it gets
removed, so then it's null and Krita tries to merge other layers
in instead of putAfter, see bug 395903.
https://bugs.kde.org/show_bug.cgi?id=395903
As a solution, if putAfter was invisible, one of the visible
nodes-to-be-merged were assigned to putAfter to replace the
original putAfter node, to make sure that when original putAfter
is removed, some other layer is doing its job.
(The fact that it's a soon-to-be-merged layer is not a problem).

In 2020 it turned out that there is quite a lot of actions
that are possible in locked groups. It required some more changes
to the code. See bug 406697.
https://bugs.kde.org/show_bug.cgi?id=406697
A solution included a change to the relevant code such that
invisible putAfter would be unconditionally replaced with
one of the nodes from the to-be-merged list, and then
if putAfter had no parent, Krita would encounter an assert.

Problem with this behaviour:
In case of newLayerFromVisible(), mergedNodes list would only
contain a root node (which has no parent). If the putAfter is
invisible, then it's replaced with "one of" mergedNodes, which
means the root node. Which means it's going to assert.

However it's not always necessary to replace putAfter with one
of the to-be-merged nodes. It's only needed when it's going to
be removed. Hence moving the replacing into the if.
Because root is not invisible, the list of invisible nodes
will be empty and nothing will be replaced and nothing will
be removed.

NOTE: I also added a new condition: '&& cleanupNodes'.
It's not stricly necessary to cover all usecases of
mergeMultipleNodesImpl, because the only usecase where
cleanupNodes is false is the newLayerFromVisible case
(which will fail the first condition).
However it makes it more future-proof since the only case
those nodes needs to be removed is when cleanupNodes is true
(otherwise removing nodes would be a dataloss bug).

NOTE2: It's possible that putAfter replacement needs one more
check (to see if it is in fact in the invisibleNodes list).
I couldn't find a situation  where it was needed, though,
so I left it as it is.
Related: bug 428683, bug 359707, bug 395903


(cherry picked from commit c1ab1ee97f196e73d8340918e2d9b7a79eafbbca)

M  +15   -8    libs/image/kis_layer_utils.cpp

https://invent.kde.org/graphics/krita/commit/d25e08a3c6298f98850e1f22284f30536425eec1