From 75636afd64527f361d57c4111931f4252ce1ea79 Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 26 Sep 2019 19:09:23 -0300 Subject: [PATCH] Preserve cel links when we copy/move a range of cels/frames This is a common solution in the DocApi wrapper that takes cares of the duplicated cels that we're copying with copyCel() and that are linked in the source, so then it maps the links into the destination. Solving this in DocApi we fixed the problem on the timeline drag-and-drop (doc_range_ops), copy/paste clipboard ranges, and merged the code in NewFrame to duplicate (linked) cels. We've also added 3 variants of Duplicate Cels with this change: - Duplicate Cels: Copies the whole cel block without linking to previous cels. - Duplicate Linked Cels: Copies the whole cel block linking all cels to previous cels. - Duplicate Cels w/Layer Mode: Depending on the layer mode (continuous or not) the cels will be linked or not (this is how "duplicate linked cels" was working before, and was added just in case for backward compatibility). Fixes: http://steamcommunity.com/app/431730/discussions/1/142261352649813598/ --- data/gui.xml | 17 ++++---- data/strings/en.ini | 5 ++- src/app/cmd/copy_frame.cpp | 12 +----- src/app/commands/cmd_new_frame.cpp | 61 +++++++++++---------------- src/app/doc_api.cpp | 67 +++++++++++++++++++++--------- src/app/doc_api.h | 15 ++++--- src/app/doc_range_ops.cpp | 3 +- src/app/util/clipboard.cpp | 41 ++---------------- 8 files changed, 104 insertions(+), 117 deletions(-) diff --git a/data/gui.xml b/data/gui.xml index 9aa0d5aa8..4415f50eb 100644 --- a/data/gui.xml +++ b/data/gui.xml @@ -92,13 +92,13 @@ - + - + - + @@ -496,6 +496,9 @@ + + + @@ -816,10 +819,10 @@ - + - + @@ -1005,10 +1008,10 @@ - + - + diff --git a/data/strings/en.ini b/data/strings/en.ini index f61a769a7..e56e52da7 100644 --- a/data/strings/en.ini +++ b/data/strings/en.ini @@ -337,8 +337,9 @@ NewFile = New File NewFile_FromClipboard = New File from Clipboard NewFrame = New Frame NewFrame_NewEmptyFrame = New Empty Frame -NewFrame_DuplicateCels = Duplicate Linked Cels -NewFrame_DuplicateCelsBlock = Duplicate Cels +NewFrame_DuplicateCels = Duplicate Cels w/Layer Mode +NewFrame_DuplicateCelsCopies = Duplicate Cels +NewFrame_DuplicateCelsLinked = Duplicate Linked Cels NewFrameTag = New Frame Tag NewLayer = New {} NewLayer_BeforeActiveLayer = New {} Below diff --git a/src/app/cmd/copy_frame.cpp b/src/app/cmd/copy_frame.cpp index e7e97ca09..94ff4efa8 100644 --- a/src/app/cmd/copy_frame.cpp +++ b/src/app/cmd/copy_frame.cpp @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2019 Igara Studio S.A. // Copyright (C) 2001-2016 David Capello // // This program is distributed under the terms of @@ -37,16 +38,7 @@ void CopyFrame::onExecute() executeAndAdd(new cmd::AddFrame(sprite, m_newFrame)); executeAndAdd(new cmd::SetFrameDuration(sprite, m_newFrame, msecs)); - if (fromFrame >= m_newFrame) - ++fromFrame; - - for (Layer* layer : sprite->allLayers()) { - if (layer->isImage()) { - executeAndAdd(new cmd::CopyCel( - static_cast(layer), fromFrame, - static_cast(layer), m_newFrame, layer->isContinuous())); - } - } + // Do not copy cels (cmd::CopyCel must be called from outside) } } // namespace cmd diff --git a/src/app/commands/cmd_new_frame.cpp b/src/app/commands/cmd_new_frame.cpp index 36a9db769..a19f60d6b 100644 --- a/src/app/commands/cmd_new_frame.cpp +++ b/src/app/commands/cmd_new_frame.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018 Igara Studio S.A. +// Copyright (C) 2018-2019 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -42,7 +42,8 @@ public: DUPLICATE_FRAME, NEW_EMPTY_FRAME, DUPLICATE_CELS, - DUPLICATE_CELS_BLOCK, + DUPLICATE_CELS_COPIES, + DUPLICATE_CELS_LINKED, }; NewFrameCommand(); @@ -74,8 +75,11 @@ void NewFrameCommand::onLoadParams(const Params& params) m_content = Content::NEW_EMPTY_FRAME; else if (content == "cel") m_content = Content::DUPLICATE_CELS; - else if (content == "celblock") - m_content = Content::DUPLICATE_CELS_BLOCK; + else if (content == "celblock" || + content == "celcopies") + m_content = Content::DUPLICATE_CELS_COPIES; + else if (content == "cellinked") + m_content = Content::DUPLICATE_CELS_LINKED; } bool NewFrameCommand::onEnabled(Context* context) @@ -104,13 +108,12 @@ void NewFrameCommand::onExecute(Context* context) break; case Content::DUPLICATE_CELS: - case Content::DUPLICATE_CELS_BLOCK: { + case Content::DUPLICATE_CELS_LINKED: + case Content::DUPLICATE_CELS_COPIES: { const Site* site = writer.site(); if (site->inTimeline() && !site->selectedLayers().empty() && !site->selectedFrames().empty()) { - std::map relatedCels; - #if ENABLE_UI auto timeline = App::instance()->timeline(); timeline->prepareToMoveRange(); @@ -129,36 +132,19 @@ void NewFrameCommand::onExecute(Context* context) (site->selectedFrames().lastFrame() - site->selectedFrames().firstFrame() + 1); + std::unique_ptr continuous = nullptr; + switch (m_content) { + case Content::DUPLICATE_CELS_COPIES: continuous.reset(new bool(false)); break; + case Content::DUPLICATE_CELS_LINKED: continuous.reset(new bool(true)); break; + } + for (Layer* layer : selLayers) { if (layer->isImage()) { for (frame_t srcFrame : site->selectedFrames().reversed()) { frame_t dstFrame = srcFrame+frameRange; - bool continuous; - CelData* srcCelData = nullptr; - - if (m_content == Content::DUPLICATE_CELS_BLOCK) { - continuous = false; - - Cel* srcCel = static_cast(layer)->cel(srcFrame); - if (srcCel) { - srcCelData = srcCel->data(); - - auto it = relatedCels.find(srcCelData); - if (it != relatedCels.end()) { - srcFrame = it->second->frame(); - continuous = true; - } - } - } - else - continuous = layer->isContinuous(); - api.copyCel( static_cast(layer), srcFrame, - static_cast(layer), dstFrame, continuous); - - if (srcCelData && !relatedCels[srcCelData]) - relatedCels[srcCelData] = layer->cel(dstFrame); + static_cast(layer), dstFrame, continuous.get()); } } } @@ -168,10 +154,10 @@ void NewFrameCommand::onExecute(Context* context) timeline->moveRange(range); #endif } - else { + else if (auto layer = static_cast(writer.layer())) { api.copyCel( - static_cast(writer.layer()), writer.frame(), - static_cast(writer.layer()), writer.frame()+1); + layer, writer.frame(), + layer, writer.frame()+1); #ifdef ENABLE_UI // TODO the active frame should be part of the Site // TODO should we use DocObserver? @@ -216,8 +202,11 @@ std::string NewFrameCommand::onGetFriendlyName() const case Content::DUPLICATE_CELS: text = Strings::commands_NewFrame_DuplicateCels(); break; - case Content::DUPLICATE_CELS_BLOCK: - text = Strings::commands_NewFrame_DuplicateCelsBlock(); + case Content::DUPLICATE_CELS_COPIES: + text = Strings::commands_NewFrame_DuplicateCelsCopies(); + break; + case Content::DUPLICATE_CELS_LINKED: + text = Strings::commands_NewFrame_DuplicateCelsLinked(); break; } diff --git a/src/app/doc_api.cpp b/src/app/doc_api.cpp index 318f8302f..b4eb56602 100644 --- a/src/app/doc_api.cpp +++ b/src/app/doc_api.cpp @@ -337,19 +337,33 @@ void DocApi::addEmptyFramesTo(Sprite* sprite, frame_t newFrame) } void DocApi::copyFrame(Sprite* sprite, - const frame_t fromFrame, - const frame_t newFrame, - const DropFramePlace dropFramePlace, - const TagsHandling tagsHandling) + frame_t fromFrame, + const frame_t newFrame0, + const DropFramePlace dropFramePlace, + const TagsHandling tagsHandling) { ASSERT(sprite); + + frame_t newFrame = + (dropFramePlace == kDropBeforeFrame ? newFrame0: + newFrame0+1); + m_transaction.execute( new cmd::CopyFrame( - sprite, fromFrame, - (dropFramePlace == kDropBeforeFrame ? newFrame: - newFrame+1))); + sprite, fromFrame, newFrame)); - adjustFrameTags(sprite, newFrame, +1, + if (fromFrame >= newFrame) + ++fromFrame; + + for (Layer* layer : sprite->allLayers()) { + if (layer->isImage()) { + copyCel( + static_cast(layer), fromFrame, + static_cast(layer), newFrame); + } + } + + adjustFrameTags(sprite, newFrame0, +1, dropFramePlace, tagsHandling); } @@ -553,26 +567,41 @@ void DocApi::moveCel( void DocApi::copyCel( LayerImage* srcLayer, frame_t srcFrame, - LayerImage* dstLayer, frame_t dstFrame) -{ - copyCel( - srcLayer, srcFrame, - dstLayer, dstFrame, dstLayer->isContinuous()); -} - -void DocApi::copyCel( - LayerImage* srcLayer, frame_t srcFrame, - LayerImage* dstLayer, frame_t dstFrame, bool continuous) + LayerImage* dstLayer, frame_t dstFrame, + const bool* forceContinuous) { ASSERT(srcLayer != dstLayer || srcFrame != dstFrame); if (srcLayer == dstLayer && srcFrame == dstFrame) return; // Nothing to be done + Cel* srcCel = srcLayer->cel(srcFrame); + if (srcCel) { + auto it = m_linkedCels.find(srcCel->data()); + if (it != m_linkedCels.end()) { + Cel* dstRelated = it->second; + if (dstRelated && dstRelated->layer() == dstLayer) { + // Create a link + m_transaction.execute( + new cmd::CopyCel( + dstRelated->layer(), dstRelated->frame(), + dstLayer, dstFrame, true)); + return; + } + } + } + m_transaction.execute( new cmd::CopyCel( srcLayer, srcFrame, - dstLayer, dstFrame, continuous)); + dstLayer, dstFrame, + (forceContinuous ? *forceContinuous: + dstLayer->isContinuous()))); + + if (srcCel && srcCel->links()) { + if (Cel* dstCel = dstLayer->cel(dstFrame)) + m_linkedCels[srcCel->data()] = dstCel; + } } void DocApi::swapCel( diff --git a/src/app/doc_api.h b/src/app/doc_api.h index e7d0203b3..e526595f3 100644 --- a/src/app/doc_api.h +++ b/src/app/doc_api.h @@ -17,8 +17,11 @@ #include "doc/image_ref.h" #include "gfx/rect.h" +#include + namespace doc { class Cel; + class CelData; class Image; class Layer; class LayerGroup; @@ -55,7 +58,7 @@ namespace app { void addEmptyFrame(Sprite* sprite, frame_t newFrame); void addEmptyFramesTo(Sprite* sprite, frame_t newFrame); void copyFrame(Sprite* sprite, - const frame_t fromFrame, + frame_t fromFrame, const frame_t newFrame, const DropFramePlace dropFramePlace, const TagsHandling tagsHandling); @@ -82,10 +85,8 @@ namespace app { LayerImage* dstLayer, frame_t dstFrame); void copyCel( LayerImage* srcLayer, frame_t srcFrame, - LayerImage* dstLayer, frame_t dstFrame); - void copyCel( - LayerImage* srcLayer, frame_t srcFrame, - LayerImage* dstLayer, frame_t dstFrame, bool continuous); + LayerImage* dstLayer, frame_t dstFrame, + const bool* forceContinuous = nullptr); void swapCel( LayerImage* layer, frame_t frame1, frame_t frame2); @@ -133,6 +134,10 @@ namespace app { Doc* m_document; Transaction& m_transaction; + + // Map used in copyCel() to re-create the original set of linked + // cels from the src layers when we copy a block of cels. + std::map m_linkedCels; }; } // namespace app diff --git a/src/app/doc_range_ops.cpp b/src/app/doc_range_ops.cpp index 1665576da..90db4b9b1 100644 --- a/src/app/doc_range_ops.cpp +++ b/src/app/doc_range_ops.cpp @@ -54,7 +54,6 @@ static void move_or_copy_cels( LayerImage* srcLayer = static_cast(srcLayers[i]); if (i < dstLayers.size() && dstLayers[i]->isImage()) { - LayerImage* srcLayer = static_cast(srcLayers[i]); LayerImage* dstLayer = static_cast(dstLayers[i]); #ifdef TRACE_RANGE_OPS @@ -68,6 +67,8 @@ static void move_or_copy_cels( case Copy: api.copyCel(srcLayer, *srcFrame, dstLayer, *dstFrame); break; } } + // All cels moved from a image layer and dropped in other kind + // of layer (e.g. a group) will be discarded/deleted. else if (op == Move) { api.clearCel(srcLayer, *srcFrame); } diff --git a/src/app/util/clipboard.cpp b/src/app/util/clipboard.cpp index c3ea99fa0..77ba57bae 100644 --- a/src/app/util/clipboard.cpp +++ b/src/app/util/clipboard.cpp @@ -496,50 +496,17 @@ void paste(Context* ctx, const bool interactive) !dstLayer->isImage()) continue; - // Maps a linked Cel in the original sprite with its - // corresponding copy in the new sprite. In this way - // we can. - std::map relatedCels; - frame_t dstFrame = dstFrameFirst; for (frame_t srcFrame : srcRange.selectedFrames()) { Cel* srcCel = srcLayer->cel(srcFrame); - Cel* srcLink = nullptr; if (srcCel && srcCel->image()) { - bool createCopy = true; - - if (dstLayer->isContinuous() && - srcCel->links()) { - srcLink = srcCel->link(); - if (!srcLink) - srcLink = srcCel; - - if (srcLink) { - Cel* dstRelated = relatedCels[srcLink]; - if (dstRelated) { - createCopy = false; - - // Create a link from dstRelated - api.copyCel( - static_cast(dstLayer), dstRelated->frame(), - static_cast(dstLayer), dstFrame); - } - } - } - - if (createCopy) { - api.copyCel( - static_cast(srcLayer), srcFrame, - static_cast(dstLayer), dstFrame); - - if (srcLink) - relatedCels[srcLink] = dstLayer->cel(dstFrame); - } + api.copyCel( + static_cast(srcLayer), srcFrame, + static_cast(dstLayer), dstFrame); } else { - Cel* dstCel = dstLayer->cel(dstFrame); - if (dstCel) + if (Cel* dstCel = dstLayer->cel(dstFrame)) api.clearCel(dstCel); }