mirror/aseprite
1
0
Fork 0
mirror of https://github.com/aseprite/aseprite.git synced 2025-02-23 00:40:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix crash moving a group inside itself

Browse Source
This commit is contained in:
David Capello 2016-12-26 12:42:49 -03:00
parent 0014673e41
commit b6744d03f3
5 changed files with 189 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/app/cmd/move_layer.cpp
View File

@ -41,6 +41,16 @@ void MoveLayer::onExecute()
ASSERT(oldParent);
ASSERT(newParent);
#if _DEBUG // Check that we are not inserting a layer inside itself
{
Layer* p = newParent;
while (p) {
ASSERT(p != layer);
p = p->parent();
}
}
#endif
oldParent->removeLayer(layer);
newParent->insertLayer(layer, afterThis);
@ -60,6 +70,16 @@ void MoveLayer::onUndo()
ASSERT(oldParent);
ASSERT(newParent);
#if _DEBUG // Check that we are not inserting a layer inside itself
{
Layer* p = newParent;
while (p) {
ASSERT(p != layer);
p = p->parent();
}
}
#endif
newParent->removeLayer(layer);
oldParent->insertLayer(layer, afterThis);

11
src/app/document_range.cpp
View File

@ -65,8 +65,19 @@ void DocumentRange::endRange(Layer* toLayer, frame_t toFrame)
selectFrameRange(m_selectingFromFrame, toFrame);
}
void DocumentRange::selectLayer(Layer* layer)
{
if (m_type == kNone)
m_type = kLayers;
m_selectedLayers.insert(layer);
}
void DocumentRange::selectLayers(const SelectedLayers& selLayers)
{
if (m_type == kNone)
m_type = kLayers;
for (auto layer : selLayers)
m_selectedLayers.insert(layer);
}

1
src/app/document_range.h
View File

@ -48,6 +48,7 @@ namespace app {
void startRange(Layer* fromLayer, frame_t fromFrame, Type type);
void endRange(Layer* toLayer, frame_t toFrame);
void selectLayer(Layer* layer);
void selectLayers(const SelectedLayers& selLayers);
frame_t firstFrame() const { return m_selectedFrames.firstFrame(); }

20
src/app/document_range_ops.cpp
View File

@ -166,6 +166,18 @@ static DocumentRange move_or_copy_frames(
return result;
}
static bool has_child(LayerGroup* parent, Layer* child)
{
for (auto c : parent->layers()) {
if (c == child)
return true;
else if (c->isGroup() &&
has_child(static_cast<LayerGroup*>(c), child))
return true;
}
return false;
}
static DocumentRange drop_range_op(
Document* doc, Op op, const DocumentRange& from,
DocumentRangePlace place, DocumentRange to)
@ -186,6 +198,14 @@ static DocumentRange drop_range_op(
else {
parent = (*to.selectedLayers().begin())->parent();
}
// Check that we're not moving a group inside itself
for (auto moveThis : from.selectedLayers()) {
if (moveThis == parent ||
(moveThis->isGroup() &&
has_child(static_cast<LayerGroup*>(moveThis), parent)))
return from;
}
}
if (place != kDocumentRangeBefore &&

137
src/app/document_range_tests.cpp
View File

@ -1147,3 +1147,140 @@ TEST_F(DocRangeOps, ReverseFrames) {
TEST_F(DocRangeOps, ReverseCels) {
// TODO
}
// TODO this should be a TEST(), but Google Test doesn't allow to mix
// a TEST() and TEST_F() in the same file.
TEST_F(DocRangeOps, DropInsideBugs) {
doc->close();
doc.reset(static_cast<app::Document*>(ctx.documents().add(4, 4)));
auto sprite = doc->sprite();
auto layer1 = dynamic_cast<LayerImage*>(sprite->root()->firstLayer());
auto layer2 = new LayerGroup(sprite);
auto layer3 = new LayerGroup(sprite);
auto layer4 = new LayerGroup(sprite);
sprite->root()->addLayer(layer2);
sprite->root()->addLayer(layer3);
sprite->root()->addLayer(layer4);
EXPECT_EQ(layer1, sprite->root()->layers()[0]);
EXPECT_EQ(layer2, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, sprite->root()->layers()[2]);
EXPECT_EQ(layer4, sprite->root()->layers()[3]);
// layer4 layer4
// layer3 --> layer1
// layer2 layer3
// layer1 layer2
DocumentRange from, to;
from.selectLayer(layer1);
to.selectLayer(layer4);
move_range(doc, from, to, kDocumentRangeFirstChild);
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer3, sprite->root()->layers()[1]);
EXPECT_EQ(layer4, sprite->root()->layers()[2]);
EXPECT_EQ(layer1, layer4->layers()[0]);
// layer4 layer4
// layer1 --> layer1
// layer3 layer3
// layer2 layer2
from.clearRange(); from.selectLayer(layer3);
to.clearRange(); to.selectLayer(layer1);
move_range(doc, from, to, kDocumentRangeBefore);
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer4->layers()[1]);
// layer4 layer4
// layer1 --> layer3
// layer3 layer1
// layer2 layer2
from.clearRange(); from.selectLayer(layer1);
to.clearRange(); to.selectLayer(layer3);
move_range(doc, from, to, kDocumentRangeFirstChild);
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer3->layers()[0]);
// move layer4 as first child of layer4 (invalid operation)
from.clearRange(); from.selectLayer(layer4);
to.clearRange(); to.selectLayer(layer4);
move_range(doc, from, to, kDocumentRangeFirstChild);
// Everything is exactly the same (no new undo operation)
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer3->layers()[0]);
// move layer4 as first child of layer3 (invalid operation)
from.clearRange(); from.selectLayer(layer4);
to.clearRange(); to.selectLayer(layer3);
move_range(doc, from, to, kDocumentRangeFirstChild);
// Everything is exactly the same (no new undo operation)
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer3->layers()[0]);
// move layer4 after layer1 (invalid operation)
from.clearRange(); from.selectLayer(layer4);
to.clearRange(); to.selectLayer(layer1);
move_range(doc, from, to, kDocumentRangeAfter);
// Everything is exactly the same (no new undo operation)
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer3->layers()[0]);
// move layer4 after layer1 (invalid operation)
from.clearRange(); from.selectLayer(layer4);
to.clearRange(); to.selectLayer(layer1);
move_range(doc, from, to, kDocumentRangeBefore);
// Everything is exactly the same (no new undo operation)
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer3->layers()[0]);
// move layer2 inside layer2 (invalid operation)
from.clearRange(); from.selectLayer(layer2);
to.clearRange(); to.selectLayer(layer2);
move_range(doc, from, to, kDocumentRangeFirstChild);
// Everything is exactly the same (no new undo operation)
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer3->layers()[0]);
// layer4 layer4
// layer1 <-- layer3
// layer3 layer1
// layer2 layer2
doc->undoHistory()->undo();
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer4, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, layer4->layers()[0]);
EXPECT_EQ(layer1, layer4->layers()[1]);
// layer4 layer4
// layer1 <-- layer1
// layer3 layer3
// layer2 layer2
doc->undoHistory()->undo();
EXPECT_EQ(layer2, sprite->root()->layers()[0]);
EXPECT_EQ(layer3, sprite->root()->layers()[1]);
EXPECT_EQ(layer4, sprite->root()->layers()[2]);
EXPECT_EQ(layer1, layer4->layers()[0]);
// layer4 layer4
// layer3 <-- layer1
// layer2 layer3
// layer1 layer2
doc->undoHistory()->undo();
EXPECT_EQ(layer1, sprite->root()->layers()[0]);
EXPECT_EQ(layer2, sprite->root()->layers()[1]);
EXPECT_EQ(layer3, sprite->root()->layers()[2]);
EXPECT_EQ(layer4, sprite->root()->layers()[3]);
}