https://bugs.kde.org/show_bug.cgi?id=411394

Dmitry Kazakov <dimul...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CONFIRMED                   |RESOLVED
      Latest Commit|                            |https://invent.kde.org/grap
                   |                            |hics/krita/commit/a87dced5f
                   |                            |88f26e282dfc86d18a3c32ecf68
                   |                            |fa96
         Resolution|FIXED                       |FIXED
             Status|RESOLVED                    |CONFIRMED
      Latest Commit|https://invent.kde.org/grap |https://invent.kde.org/grap
                   |hics/krita/commit/a87dced5f |hics/krita/commit/a87dced5f
                   |88f26e282dfc86d18a3c32ecf68 |88f26e282dfc86d18a3c32ecf68
                   |fa96                        |fa96
             Status|CONFIRMED                   |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Dmitry Kazakov <dimul...@gmail.com> ---
Git commit a87dced5f88f26e282dfc86d18a3c32ecf68fa96 by Dmitry Kazakov.
Committed on 29/05/2020 at 11:34.
Pushed by dkazakov into branch 'master'.

Fix undo breakage after converting a pixel selecion into a vector one

Since 2013, when a selection is switched into a vector mode, its pixel
selection is used as a projection. It is done intentionally, because we
need a way to auto-flatten selection when someone opens a transaction on
this devices. It lets all our painting tools work on selections smoothly
without any conversions.

And this switching used to break undo on the projection device. Obviously,
the vectors are rendered on the device without any transactions, so we
cannot undo them.

To overcome this issue, a special mechanism "reincarnation with detached
history" for paint devices was introduced
(KisPaintDevice::reincarnateWithDetachedHistory). When a device reincarnates,
it starts a new undo history from scratch.The old undo history is kept
intact inside the returned command, so the device can later be recovered
to its original state.

As a consequence, there is an important API change in KisSelection,
now the seleciton should be converted to vector mode explicitly and
undo command for it should be kept in the common image's undo history
(that was the reason of the previous huge patch for KoShapeControllerBase).

M  +15   -0    libs/image/kis_paint_device.cc
M  +19   -0    libs/image/kis_paint_device.h
M  +40   -0    libs/image/kis_paint_device_data.h
M  +152  -28   libs/image/kis_selection.cc
M  +20   -5    libs/image/kis_selection.h
M  +38   -19   libs/image/kis_transaction_data.cpp
M  +2    -0    libs/image/kis_transaction_data.h
M  +1    -6    libs/image/tests/CMakeLists.txt
M  +6    -5    libs/image/tests/kis_selection_test.cpp
M  +2    -2    libs/libkis/Selection.cpp
M  +2    -6    libs/ui/flake/kis_shape_controller.cpp
M  +0    -4    libs/ui/flake/kis_shape_selection_model.cpp
M  +1    -1    libs/ui/kis_selection_decoration.cc
M  +3    -3    libs/ui/kis_selection_manager.cc
M  +167  -18   libs/ui/tests/kis_shape_selection_test.cpp
M  +2    -0    libs/ui/tests/kis_shape_selection_test.h
M  +1    -1    libs/ui/tests/util.h
M  +1    -1    plugins/impex/libkra/kis_kra_load_visitor.cpp
M  +2    -2    plugins/impex/libkra/kis_kra_save_visitor.cpp
M  +2    -2    plugins/impex/libkra/tests/kis_kra_saver_test.cpp
M  +4    -4    plugins/impex/libkra/tests/util.h

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

--- Comment #8 from Dmitry Kazakov <dimul...@gmail.com> ---
Git commit 5fcc49fb6126979e03739fa2fd8be063b5c2fc85 by Dmitry Kazakov.
Committed on 29/05/2020 at 11:34.
Pushed by dkazakov into branch 'master'.

Remove an assert when cloning a shape selection

The problem seem to have been fixed with my recent shapes' multithreading
refactoring that changed all the signal compressors to be thread-safe.

M  +0    -6    libs/ui/flake/kis_shape_selection.cpp

https://invent.kde.org/graphics/krita/commit/5fcc49fb6126979e03739fa2fd8be063b5c2fc85

--- Comment #9 from Dmitry Kazakov <dimul...@gmail.com> ---
Git commit 6172584f8512238bba8ceb3cda79fbbc87609dc9 by Dmitry Kazakov.
Committed on 29/05/2020 at 11:34.
Pushed by dkazakov into branch 'master'.

Remove shape tracking from KoShapeControllerBase

Long ago, when Flake was flourishing as a cross-application framework,
it supported shapes being toplevel shapes (that is, having no parent).
It created a lot of troubles, because all the **roots** still had to be
stored somewhere and some entity had to track shapes addition and
removal from the hierarchy.

That was exactly the reason why KoShapeControllerBase was born. Those
days it was called KoShapeDocumentBase, which meant "a document
that stores all shapes, including the root ones". Legends say
that it was also supposed to add/remove shapes from KoShapeManager,
but no written proofs of it survived.

When Krita introduced vector layers, every vector layer became a
flake's "canvas". Every canvas also had a KoShapeManager, that was
responsible for painting the shapes. But Krita diverged from the
expected way to track the shapes: instead of adding a
KoShapeDocumentBase to every vector layer, it added only one "document"
for all the layers. This document (now called KisShapeController)
dispatched add/remove notifications to the active layer. That was fun,
but never worked as expected. Due to complexity of signal delivery and
multithreading, it could dispatch add/remove notifications to a wrong
layer. It also made managing shapes' lifetime rather cumbersome: when
a group shape was destroyed, it had to destroy all the children, but it
had *no* link to the controller, so it couldn't notify it!

Due to these problems, at some point Krita switched to "no multiple
roots" paradigm. Every vector layer started to have exactly one root
at the top of the shapes hierarchy (it was the vector layer itself).
It solved all the notifications problems: the root could track all
its children and manage KoShapeManager properly. It also solved the
lifetime problem, since there was a single point where shapes could
be added/removed, KoShape::setParent().

After this change KoShapeControllerBase stopped serving its original
purpose and was only needed to add lazily create vecotor layers or
shape selections, when new shapes were dropped to the canvas. But we
still had to pass all shape additions through it. It resulted in a very
weird chain of calls, when one wanted to add a new shape to the image:

1) KoShapeController::addShapeDirect()
2) KoShapeCreateCommand::redo()
3) KoShapeControllerBase::addShape()
4) KoCanvasBase::addCommand()
#  NOTE ^^^: adding a command to the undo stack while another
#            command is running!

This patch breaks this chain. KoShapeControllerBase now doesn't track
anything and is not even allowed to track anything. Having multiple
toplevel (root) shapes **is now prohibited**.

Instead, all the shapes hierarchy is now tracked by the single root
shape. To achieve that, special methods were added to
SimpleShapeContainerModel to manage KoShapeManager contents
(they were moved from ShapeLayerContainerModel).

>From now on, the official ways to add a shape to a hierarchy is the
following:

1) If you know what parent you want to add the shape to, just create
   KoShapeCreateCommand() and it will do everything for you (adding
   and resolving z-index collisions)

2) If you just want to drop the shape to the canvas and want it to
   be added "somewhere", then use KoShapeController::addShapesDirect().
   If will find a suitable layer for the shape (or create a new one).
   All that will be done in undoable manner.

I hope in the next few iterations we will be able to remove
KoShapeControllerBase completely from the Krita's codebase. It doesn't
do much useful work anymore. It's main reason for exintence now is to
provide KisImage size and DPI value.

# Conflicts:
#       libs/flake/KoShapeController.cpp
#       plugins/flake/textshape/kotext/commands/AddAnnotationCommand.cpp

M  +1    -0    libs/flake/CMakeLists.txt
M  +13   -7    libs/flake/KoShapeController.cpp
M  +5    -5    libs/flake/KoShapeControllerBase.cpp
M  +9    -31   libs/flake/KoShapeControllerBase.h
M  +41   -0    libs/flake/SimpleShapeContainerModel.h
A  +64   -0    libs/flake/commands/KoAddRemoveShapeCommands.cpp     [License:
GPL (v2+)]
A  +54   -0    libs/flake/commands/KoAddRemoveShapeCommands.h     [License: GPL
(v2+)]
M  +0    -4    libs/flake/commands/KoPathCombineCommand.cpp
M  +4    -5    libs/flake/commands/KoShapeClipCommand.cpp
M  +20   -43   libs/flake/commands/KoShapeCreateCommand.cpp
M  +4    -6    libs/flake/commands/KoShapeDeleteCommand.cpp
M  +4    -5    libs/flake/commands/KoShapeUnclipCommand.cpp
M  +30   -30   libs/flake/tests/MockShapes.h
M  +14   -21   libs/flake/tests/TestPointMergeCommand.cpp
M  +21   -20   libs/flake/tests/TestPointRemoveCommand.cpp
M  +9    -11   libs/flake/tests/TestShapePainting.cpp
M  +5    -3    libs/ui/actions/kis_selection_action_factories.cpp
M  +42   -46   libs/ui/flake/kis_shape_controller.cpp
M  +1    -2    libs/ui/flake/kis_shape_controller.h
M  +6    -11   libs/ui/flake/kis_shape_layer.cc

https://invent.kde.org/graphics/krita/commit/6172584f8512238bba8ceb3cda79fbbc87609dc9

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to