Commit 54de0795 authored by Dmitry Kazakov's avatar Dmitry Kazakov

Fix colliding zIndex values when copy-pasting the shapes

See a warning comment in KoShape::compareShapeZIndex()
parent b3e59c91
...@@ -531,6 +531,19 @@ KoShape::ChildZOrderPolicy KoShape::childZOrderPolicy() ...@@ -531,6 +531,19 @@ KoShape::ChildZOrderPolicy KoShape::childZOrderPolicy()
bool KoShape::compareShapeZIndex(KoShape *s1, KoShape *s2) bool KoShape::compareShapeZIndex(KoShape *s1, KoShape *s2)
{ {
/**
* WARNING: Our definition of zIndex is not yet compatible with SVG2's
* definition. In SVG stacking context of groups with the same
* zIndex are **merged**, while in Krita the contents of groups
* is never merged. One group will always below than the other.
* Therefore, when zIndex of two groups inside the same parent
* coinside, the resulting painting order in Krita is
* **UNDEFINED**.
*
* To avoid this trouble we use KoShapeReorderCommand::mergeInShape()
* inside KoShapeCreateCommand.
*/
// First sort according to runThrough which is sort of a master level // First sort according to runThrough which is sort of a master level
KoShape *parentShapeS1 = s1->parent(); KoShape *parentShapeS1 = s1->parent();
KoShape *parentShapeS2 = s2->parent(); KoShape *parentShapeS2 = s2->parent();
......
...@@ -25,11 +25,16 @@ ...@@ -25,11 +25,16 @@
#include <klocalizedstring.h> #include <klocalizedstring.h>
#include "kis_assert.h"
#include <KoShapeLayer.h>
#include <KoShapeReorderCommand.h>
class Q_DECL_HIDDEN KoShapeCreateCommand::Private class Q_DECL_HIDDEN KoShapeCreateCommand::Private
{ {
public: public:
Private(KoShapeBasedDocumentBase *c, KoShape *s) Private(KoShapeBasedDocumentBase *c, KoShape *s)
: controller(c), : shapesDocument(c),
shape(s), shape(s),
shapeParent(shape->parent()), shapeParent(shape->parent()),
deleteShape(true) { deleteShape(true) {
...@@ -39,10 +44,12 @@ public: ...@@ -39,10 +44,12 @@ public:
delete shape; delete shape;
} }
KoShapeBasedDocumentBase *controller; KoShapeBasedDocumentBase *shapesDocument;
KoShape *shape; KoShape *shape;
KoShapeContainer *shapeParent; KoShapeContainer *shapeParent;
bool deleteShape; bool deleteShape;
QScopedPointer<KUndo2Command> reorderingCommand;
}; };
KoShapeCreateCommand::KoShapeCreateCommand(KoShapeBasedDocumentBase *controller, KoShape *shape, KUndo2Command *parent) KoShapeCreateCommand::KoShapeCreateCommand(KoShapeBasedDocumentBase *controller, KoShape *shape, KUndo2Command *parent)
...@@ -61,22 +68,40 @@ void KoShapeCreateCommand::redo() ...@@ -61,22 +68,40 @@ void KoShapeCreateCommand::redo()
{ {
KUndo2Command::redo(); KUndo2Command::redo();
Q_ASSERT(d->shape); Q_ASSERT(d->shape);
Q_ASSERT(d->controller); Q_ASSERT(d->shapesDocument);
if (d->shapeParent)
if (d->shapeParent) {
d->shapeParent->addShape(d->shape); d->shapeParent->addShape(d->shape);
}
// the parent has to be there when it is added to the KoShapeBasedDocumentBase // the parent has to be there when it is added to the KoShapeBasedDocumentBase
d->controller->addShape(d->shape); d->shapesDocument->addShape(d->shape);
d->shapeParent = d->shape->parent(); // update parent if the 'addShape' changed it d->shapeParent = d->shape->parent(); // update parent if the 'addShape' changed it
d->deleteShape = false; d->deleteShape = false;
KIS_SAFE_ASSERT_RECOVER_NOOP(d->shapeParent ||
dynamic_cast<KoShapeLayer*>(d->shape));
if (d->shapeParent) {
d->reorderingCommand.reset(KoShapeReorderCommand::mergeInShape(d->shapeParent->shapes(), d->shape));
if (d->reorderingCommand) {
d->reorderingCommand->redo();
}
}
} }
void KoShapeCreateCommand::undo() void KoShapeCreateCommand::undo()
{ {
KUndo2Command::undo(); KUndo2Command::undo();
Q_ASSERT(d->shape); Q_ASSERT(d->shape);
Q_ASSERT(d->controller); Q_ASSERT(d->shapesDocument);
if (d->reorderingCommand) {
d->reorderingCommand->undo();
d->reorderingCommand.reset();
}
// the parent has to be there when it is removed from the KoShapeBasedDocumentBase // the parent has to be there when it is removed from the KoShapeBasedDocumentBase
d->controller->removeShape(d->shape); d->shapesDocument->removeShape(d->shape);
if (d->shapeParent) if (d->shapeParent)
d->shapeParent->removeShape(d->shape); d->shapeParent->removeShape(d->shape);
d->deleteShape = true; d->deleteShape = true;
......
...@@ -179,3 +179,42 @@ KoShapeReorderCommand *KoShapeReorderCommand::createCommand(const QList<KoShape* ...@@ -179,3 +179,42 @@ KoShapeReorderCommand *KoShapeReorderCommand::createCommand(const QList<KoShape*
Q_ASSERT(changedShapes.count() == newIndexes.count()); Q_ASSERT(changedShapes.count() == newIndexes.count());
return changedShapes.isEmpty() ? 0: new KoShapeReorderCommand(changedShapes, newIndexes, parent); return changedShapes.isEmpty() ? 0: new KoShapeReorderCommand(changedShapes, newIndexes, parent);
} }
KoShapeReorderCommand *KoShapeReorderCommand::mergeInShape(QList<KoShape *> shapes, KoShape *newShape, KUndo2Command *parent)
{
qSort(shapes.begin(), shapes.end(), KoShape::compareShapeZIndex);
QList<KoShape*> reindexedShapes;
QList<int> reindexedIndexes;
const int originalShapeZIndex = newShape->zIndex();
int newShapeZIndex = originalShapeZIndex;
int lastOccupiedShapeZIndex = originalShapeZIndex + 1;
Q_FOREACH (KoShape *shape, shapes) {
if (shape == newShape) continue;
const int zIndex = shape->zIndex();
if (newShapeZIndex == originalShapeZIndex) {
if (zIndex == originalShapeZIndex) {
newShapeZIndex = originalShapeZIndex + 1;
lastOccupiedShapeZIndex = newShapeZIndex;
reindexedShapes << newShape;
reindexedIndexes << newShapeZIndex;
}
} else {
if (newShapeZIndex != originalShapeZIndex &&
zIndex >= newShapeZIndex &&
zIndex <= lastOccupiedShapeZIndex) {
lastOccupiedShapeZIndex = zIndex + 1;
reindexedShapes << shape;
reindexedIndexes << lastOccupiedShapeZIndex;
}
}
}
return !reindexedShapes.isEmpty() ? new KoShapeReorderCommand(reindexedShapes, reindexedIndexes, parent) : 0;
}
...@@ -63,6 +63,20 @@ public: ...@@ -63,6 +63,20 @@ public:
static KoShapeReorderCommand *createCommand(const QList<KoShape*> &shapes, KoShapeManager *manager, static KoShapeReorderCommand *createCommand(const QList<KoShape*> &shapes, KoShapeManager *manager,
MoveShapeType move, KUndo2Command *parent = 0); MoveShapeType move, KUndo2Command *parent = 0);
/**
* @brief mergeInShape adjust zIndex of all the \p shapes and \p newShape to
* avoid collisions between \p shapes and \p newShape.
*
* Note1: \p newShape may or may not be contained in \p shapes, there
* is no difference.
* Note2: the collisions inside \p shapes are ignored. They are just
* adjusted to avoid collisions with \p newShape only
* @param parent the parent command for grouping purposes.
* @return command for reording the shapes or 0 if no reordering happend
*/
static KoShapeReorderCommand *mergeInShape(QList<KoShape*> shapes, KoShape *newShape,
KUndo2Command *parent = 0);
/// redo the command /// redo the command
void redo(); void redo();
/// revert the actions done in redo /// revert the actions done in redo
......
...@@ -558,5 +558,51 @@ void TestShapeReorderCommand::testNoCommand() ...@@ -558,5 +558,51 @@ void TestShapeReorderCommand::testNoCommand()
cmd = KoShapeReorderCommand::createCommand(selectedShapes, &manager, KoShapeReorderCommand::LowerShape); cmd = KoShapeReorderCommand::createCommand(selectedShapes, &manager, KoShapeReorderCommand::LowerShape);
QVERIFY(cmd == 0); QVERIFY(cmd == 0);
} }
#include <kis_assert.h>
#include <kis_debug.h>
void testMergeInShapeImpl(const QVector<int> indexesProfile,
int newShapeIndex,
const QVector<int> expectedIndexes)
{
KIS_ASSERT(indexesProfile.size() == expectedIndexes.size());
QVector<MockShape> shapesStore(indexesProfile.size());
QList<KoShape*> managedShapes;
for (int i = 0; i < shapesStore.size(); i++) {
shapesStore[i].setSize(QSizeF(100,100));
shapesStore[i].setZIndex(indexesProfile[i]);
managedShapes << &shapesStore[i];
}
QScopedPointer<KUndo2Command> cmd(
KoShapeReorderCommand::mergeInShape(managedShapes, &shapesStore[newShapeIndex]));
cmd->redo();
for (int i = 0; i < shapesStore.size(); i++) {
//qDebug() << ppVar(i) << ppVar(shapesStore[i].zIndex());
QCOMPARE(shapesStore[i].zIndex(), expectedIndexes[i]);
}
}
void TestShapeReorderCommand::testMergeInShape()
{
QVector<int> indexesProfile({1,1,2,2,2,3,3,4,5,6});
int newShapeIndex = 3;
QVector<int> expectedIndexes({1,1,2,3,2,4,4,5,6,7});
testMergeInShapeImpl(indexesProfile, newShapeIndex, expectedIndexes);
}
void TestShapeReorderCommand::testMergeInShapeDistant()
{
QVector<int> indexesProfile({1,1,2,2,2,4,4,5,6,7});
int newShapeIndex = 3;
QVector<int> expectedIndexes({1,1,2,3,2,4,4,5,6,7});
testMergeInShapeImpl(indexesProfile, newShapeIndex, expectedIndexes);
}
QTEST_MAIN(TestShapeReorderCommand) QTEST_MAIN(TestShapeReorderCommand)
...@@ -41,6 +41,8 @@ private Q_SLOTS: ...@@ -41,6 +41,8 @@ private Q_SLOTS:
void testMoveDownOverlapping(); void testMoveDownOverlapping();
void testSendToBackChildren(); void testSendToBackChildren();
void testNoCommand(); void testNoCommand();
void testMergeInShape();
void testMergeInShapeDistant();
}; };
#endif // TESTSHAPEREORDERCOMMAND_H #endif // TESTSHAPEREORDERCOMMAND_H
...@@ -1091,17 +1091,12 @@ void DefaultTool::selectionReorder(KoShapeReorderCommand::MoveShapeType order) ...@@ -1091,17 +1091,12 @@ void DefaultTool::selectionReorder(KoShapeReorderCommand::MoveShapeType order)
return; return;
} }
QList<KoShape *> selectedShapes = selection->selectedShapes(); QList<KoShape *> selectedShapes = selection->selectedEditableShapes();
if (selectedShapes.count() < 1) { if (selectedShapes.isEmpty()) {
return;
}
QList<KoShape *> editableShapes = filterEditableShapes(selectedShapes);
if (editableShapes.count() < 1) {
return; return;
} }
KUndo2Command *cmd = KoShapeReorderCommand::createCommand(editableShapes, canvas()->shapeManager(), order); KUndo2Command *cmd = KoShapeReorderCommand::createCommand(selectedShapes, canvas()->shapeManager(), order);
if (cmd) { if (cmd) {
canvas()->addCommand(cmd); canvas()->addCommand(cmd);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment