diff --git a/ChangeLog.md b/ChangeLog.md index ea550e159..8939fe888 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -3,12 +3,13 @@ ### Features + Introduce new Undo/Redo system [#1817](https://github.com/pencil2d/pencil/pull/1817) -### Enhancements +### Enhancements/Changes + Add checkbox to allow polyline to close automatically [#1863](https://github.com/pencil2d/pencil/pull/1863) + Maintain active layer track in view - [#1867](https://github.com/pencil2d/pencil/pull/1867) + Update shortcuts [#1866](https://github.com/pencil2d/pencil/pull/1866) + Improve dock layout for lower resolutions [#1840](https://github.com/pencil2d/pencil/pull/1840) + Add ability to remove Last Polyline Segment using backspace [#1861](https://github.com/pencil2d/pencil/pull/1861) ++ Changed handling of pcl projects - [#1896](https://github.com/pencil2d/pencil/pull/1896) ### Bugfixes: + Do not make a new keyframe if double clicking on an existing keyframe - [#1851](https://github.com/pencil2d/pencil/pull/1851) @@ -18,6 +19,9 @@ + Avoid updating width/feather sliders for tools that don’t use them [cce3107](https://github.com/pencil2d/pencil/commit/cce31079c871fcc04e957c44d5c6e65990f635f1) + Fix fill misbehaving when drawing was partly outside border [#1865](https://github.com/pencil2d/pencil/pull/1865) + Fix clearing selection with the delete shortcut [#1892](https://github.com/pencil2d/pencil/pull/1892) ++ Fixed memory leak when copying bitmap keyframes - [#1896](https://github.com/pencil2d/pencil/pull/1896) ++ Fixed potential issue on some systems when repeatedly copying bitmap frames - [#1896](https://github.com/pencil2d/pencil/pull/1896) ++ Fixed bitmap frame wipe that can occur under specific situations when using keyframe copy & paste - [#1896](https://github.com/pencil2d/pencil/pull/1896) ## Pencil2D v0.7.0 - 12 July 2024 diff --git a/app/src/actioncommands.cpp b/app/src/actioncommands.cpp index 347779eeb..11fba978c 100644 --- a/app/src/actioncommands.cpp +++ b/app/src/actioncommands.cpp @@ -758,10 +758,6 @@ void ActionCommands::duplicateLayer() { mEditor->sound()->processSound(static_cast(key)); } - else - { - key->modification(); - } }); if (!fromLayer->keyExists(1)) { toLayer->removeKeyFrame(1); @@ -803,11 +799,6 @@ void ActionCommands::duplicateKey() mEditor->sound()->processSound(dynamic_cast(dupKey)); showSoundClipWarningIfNeeded(); } - else - { - dupKey->setFileName(""); // don't share filename - dupKey->modification(); - } mEditor->layers()->notifyAnimationLengthChanged(); emit mEditor->layers()->currentLayerChanged(mEditor->layers()->currentLayerIndex()); // trigger timeline repaint. diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index fc1f1cc15..127fe6eee 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -18,6 +18,7 @@ GNU General Public License for more details. #include #include +#include #include #include #include @@ -100,7 +101,7 @@ BitmapImage* BitmapImage::clone() const b->setFileName(""); // don't link to the file of the source bitmap image const bool validKeyFrame = !fileName().isEmpty(); - if (validKeyFrame && !isLoaded()) + if (validKeyFrame && !isModified()) { // This bitmapImage is temporarily unloaded. // since it's not in the memory, we need to copy the linked png file to prevent data loss. @@ -108,14 +109,18 @@ BitmapImage* BitmapImage::clone() const Q_ASSERT(finfo.isAbsolute()); Q_ASSERT(QFile::exists(fileName())); - QString newFileName = QString("%1/%2-%3.%4") - .arg(finfo.canonicalPath()) - .arg(finfo.completeBaseName()) - .arg(uniqueString(12)) - .arg(finfo.suffix()); - b->setFileName(newFileName); + QString newFilePath; + do + { + newFilePath = QString("%1/temp-%2.%3") + .arg(finfo.canonicalPath()) + .arg(uniqueString(12)) + .arg(finfo.suffix()); + } + while (QFile::exists(newFilePath)); - bool ok = QFile::copy(fileName(), newFileName); + b->setFileName(newFilePath); + bool ok = QFile::copy(fileName(), newFilePath); Q_ASSERT(ok); qDebug() << "COPY>" << fileName(); } diff --git a/core_lib/src/interface/editor.cpp b/core_lib/src/interface/editor.cpp index 6642a7e70..db0263b59 100644 --- a/core_lib/src/interface/editor.cpp +++ b/core_lib/src/interface/editor.cpp @@ -330,19 +330,19 @@ void Editor::pasteToFrames() currentLayer->moveSelectedFrames(1); } + KeyFrame* key = it->second; // It's a bug if the keyframe is nullptr at this point... - Q_ASSERT(it->second != nullptr); + Q_ASSERT(key != nullptr); // TODO: undo/redo implementation - KeyFrame* keyClone = it->second->clone(); - currentLayer->addKeyFrame(newPosition, keyClone); + currentLayer->addKeyFrame(newPosition, key); if (currentLayer->type() == Layer::SOUND) { - auto soundClip = static_cast(keyClone); + auto soundClip = static_cast(key); sound()->loadSound(soundClip, soundClip->fileName()); } - currentLayer->setFrameSelected(keyClone->pos(), true); + currentLayer->setFrameSelected(key->pos(), true); } } @@ -354,7 +354,7 @@ void Editor::paste() if (!canPaste()) { return; } - if (clipboards()->getClipboardFrames().empty()) { + if (clipboards()->framesIsEmpty()) { backup(tr("Paste")); diff --git a/core_lib/src/managers/clipboardmanager.cpp b/core_lib/src/managers/clipboardmanager.cpp index cfb1ffb14..67ea7e07f 100644 --- a/core_lib/src/managers/clipboardmanager.cpp +++ b/core_lib/src/managers/clipboardmanager.cpp @@ -28,7 +28,11 @@ ClipboardManager::ClipboardManager(Editor* editor) : BaseManager(editor, "Clipbo ClipboardManager::~ClipboardManager() { - + for (auto it : mFrames) + { + KeyFrame* frame = it.second; + delete frame; + } } void ClipboardManager::setFromSystemClipboard(const QPointF& pos, const Layer* layer) @@ -75,24 +79,43 @@ void ClipboardManager::copyVectorImage(const VectorImage* vectorImage) mVectorImage = *vectorImage->clone(); } -void ClipboardManager::copySelectedFrames(const Layer* currentLayer) { +void ClipboardManager::copySelectedFrames(const Layer* currentLayer) +{ resetStates(); for (int pos : currentLayer->selectedKeyFramesPositions()) { KeyFrame* keyframe = currentLayer->getKeyFrameAt(pos); - Q_ASSERT(keyframe != nullptr); - keyframe->loadFile(); + KeyFrame* newKeyframe = keyframe->clone(); + // Unload unmodified keyframes now as they won't ever get unloaded + // by activeframepool while in clipboard manager. + newKeyframe->unloadFile(); - mFrames.insert(std::make_pair(keyframe->pos(), keyframe->clone())); + mFrames.insert(std::make_pair(keyframe->pos(), newKeyframe)); } mFramesType = currentLayer->type(); } +std::map ClipboardManager::getClipboardFrames() +{ + std::map resultMap; + for (auto it : mFrames) + { + resultMap.insert(std::make_pair(it.first, it.second->clone())); + } + return resultMap; +} + void ClipboardManager::resetStates() { + for (auto it : mFrames) + { + KeyFrame* frame = it.second; + delete frame; + } mFrames.clear(); + mBitmapImage = BitmapImage(); mVectorImage = VectorImage(); mFramesType = Layer::LAYER_TYPE::UNDEFINED; diff --git a/core_lib/src/managers/clipboardmanager.h b/core_lib/src/managers/clipboardmanager.h index 04d3b82ee..4ab7c1df4 100644 --- a/core_lib/src/managers/clipboardmanager.h +++ b/core_lib/src/managers/clipboardmanager.h @@ -61,7 +61,13 @@ class ClipboardManager: public BaseManager const BitmapImage& getBitmapClipboard() const { return mBitmapImage; } const VectorImage& getVectorClipboard() const { return mVectorImage; } - std::map getClipboardFrames() { return mFrames; } + + /** Return a copy of all clipboard frames keyed by their position. + * + * The caller takes ownership of the returned keyframe objects and is + * responsible for deleting them when no longer in use. + */ + std::map getClipboardFrames(); Layer::LAYER_TYPE framesLayerType() const { return mFramesType; } bool framesIsEmpty() const { return mFrames.empty(); } diff --git a/core_lib/src/structure/filemanager.cpp b/core_lib/src/structure/filemanager.cpp index 898ebb824..6289ce84c 100644 --- a/core_lib/src/structure/filemanager.cpp +++ b/core_lib/src/structure/filemanager.cpp @@ -48,8 +48,8 @@ Object* FileManager::load(const QString& sFileName) obj->setFilePath(sFileName); obj->createWorkingDir(); - QString strMainXMLFile; - QString strDataFolder; + QString strMainXMLFile = QDir(obj->workingDir()).filePath(PFF_XML_FILE_NAME); + QString strDataFolder = QDir(obj->workingDir()).filePath(PFF_DATA_DIR); // Test file format: new zipped .pclx or old .pcl? bool isArchive = isArchiveFormat(sFileName); @@ -59,8 +59,15 @@ Object* FileManager::load(const QString& sFileName) { dd << fileFormat.arg(".pcl"); - strMainXMLFile = sFileName; - strDataFolder = strMainXMLFile + "." + PFF_OLD_DATA_DIR; + if (!QFile::copy(sFileName, strMainXMLFile)) + { + dd << "Failed to copy main xml file"; + } + Status st = copyDir(sFileName + "." + PFF_OLD_DATA_DIR, strDataFolder); + if (!st.ok()) + { + dd.collect(st.details()); + } } else { @@ -89,9 +96,6 @@ Object* FileManager::load(const QString& sFileName) return nullptr; } } - - strMainXMLFile = QDir(workingDirPath).filePath(PFF_XML_FILE_NAME); - strDataFolder = QDir(workingDirPath).filePath(PFF_DATA_DIR); } obj->setDataDir(strDataFolder); @@ -754,6 +758,63 @@ Status FileManager::writePalette(const Object* object, const QString& dataFolder return Status::OK; } +/** Copy a directory to another directory recursively (depth-first). + * + * If any of the files being copied already exist at the destination, they will + * not be overwritten and a non-ok status will be returned. + * + * If any part of the copy fails, this function will still attempt to copy + * as many files as possible before returning a non-ok status. + * + * @param src The source directory to copy. + * @param dst The target directory to copy to. + * + * @return Status indicating the success or failure of the copy. + */ +Status FileManager::copyDir(const QDir src, const QDir dst) +{ + if (!src.exists()) return Status::FILE_NOT_FOUND; + + DebugDetails dd; + bool isOkay = true; + + if (!dst.mkpath(dst.absolutePath())) + { + dd << ("Failed to create target directory: " + dst.absolutePath()); + return Status(Status::FAIL, dd); + } + + foreach (QString dirName, src.entryList(QDir::Dirs | QDir::NoDotAndDotDot)) + { + Status st = copyDir(src.filePath(dirName), dst.filePath(dirName)); + if (!st.ok()) + { + isOkay = false; + dd.collect(st.details()); + } + } + + foreach (QString fileName, src.entryList(QDir::Files)) + { + if (!QFile::copy(src.filePath(fileName), dst.filePath(fileName))) + { + isOkay = false; + dd << "Failed to copy file" + << ("Source path: " + src.filePath(fileName)) + << ("Destination path: " + dst.filePath(fileName)); + } + } + + if (isOkay) + { + return Status::OK; + } + else + { + return Status(Status::FAIL, dd); + } +} + Status FileManager::unzip(const QString& strZipFile, const QString& strUnzipTarget) { // removes the previous directory first - better approach diff --git a/core_lib/src/structure/filemanager.h b/core_lib/src/structure/filemanager.h index 9fc81a1ca..0b1137a77 100644 --- a/core_lib/src/structure/filemanager.h +++ b/core_lib/src/structure/filemanager.h @@ -30,6 +30,7 @@ GNU General Public License for more details. class Object; class ObjectData; +class QDir; class FileManager : public QObject @@ -55,6 +56,7 @@ class FileManager : public QObject void progressRangeChanged(int maxValue); private: + Status copyDir(const QDir src, const QDir dst); Status unzip(const QString& strZipFile, const QString& strUnzipTarget); bool loadObject(Object*, const QDomElement& root);