From 37cbd9763f3d256bf6d4d02cd53a4e837962dabe Mon Sep 17 00:00:00 2001 From: Jakob Date: Sun, 14 Apr 2024 10:36:34 +0200 Subject: [PATCH] Fix crash when importing image to empty frame on hidden layer (#1820) * Fix crash when importing image to empty frame on hidden layer * Disable timeline hover effect for hidden layers * Document various frame adding methods * Fix potential memory leak in Layer::addNewKeyFrameAt * Clean up some code --- app/src/timelinecells.cpp | 5 ++++- core_lib/src/interface/editor.cpp | 30 ++++++++++++++++-------------- core_lib/src/interface/editor.h | 17 ++++++++++++++++- core_lib/src/structure/layer.cpp | 7 ++++++- core_lib/src/structure/layer.h | 11 +++++++++++ core_lib/src/util/pencilerror.cpp | 2 -- core_lib/src/util/pencilerror.h | 8 ++++---- 7 files changed, 57 insertions(+), 23 deletions(-) diff --git a/app/src/timelinecells.cpp b/app/src/timelinecells.cpp index 4524cf38c1..1adf723a8a 100644 --- a/app/src/timelinecells.cpp +++ b/app/src/timelinecells.cpp @@ -747,7 +747,10 @@ void TimeLineCells::paintEvent(QPaintEvent*) { paintHighlightedFrame(painter, mHighlightedFrame, recTop, standardWidth, recHeight); } - paintFrameCursorOnCurrentLayer(painter, recTop, standardWidth, recHeight); + if (currentLayer->visible()) + { + paintFrameCursorOnCurrentLayer(painter, recTop, standardWidth, recHeight); + } } // --- draw the position of the current frame diff --git a/core_lib/src/interface/editor.cpp b/core_lib/src/interface/editor.cpp index 49af52acb7..b997168995 100644 --- a/core_lib/src/interface/editor.cpp +++ b/core_lib/src/interface/editor.cpp @@ -17,12 +17,9 @@ GNU General Public License for more details. #include "editor.h" -#include -#include #include #include #include -#include #include #include "object.h" @@ -919,7 +916,13 @@ Status Editor::importBitmapImage(const QString& filePath) QImageReader reader(filePath); Q_ASSERT(layers()->currentLayer()->type() == Layer::BITMAP); - auto layer = static_cast(layers()->currentLayer()); + const auto layer = static_cast(layers()->currentLayer()); + + if (!layer->visible()) + { + mScribbleArea->showLayerNotVisibleWarning(); + return Status::SAFE; + } Status status = Status::OK; DebugDetails dd; @@ -942,7 +945,7 @@ Status Editor::importBitmapImage(const QString& filePath) break; case QImageReader::UnsupportedFormatError: errorDesc = tr("Image format is not supported. Please convert the image file to one of the following formats and try again:\n%1") - .arg((QString)reader.supportedImageFormats().join(", ")); + .arg(QString::fromUtf8(reader.supportedImageFormats().join(", "))); break; default: errorDesc = tr("An error has occurred while reading the image. Please check that the file is a valid image and try again."); @@ -956,7 +959,8 @@ Status Editor::importBitmapImage(const QString& filePath) if (!layer->keyExists(mFrame)) { - addNewKey(); + const bool ok = addNewKey(); + Q_ASSERT(ok); } BitmapImage* bitmapImage = layer->getBitmapImageAtFrame(mFrame); BitmapImage importedBitmapImage(pos, img); @@ -1211,7 +1215,7 @@ KeyFrame* Editor::addNewKey() return addKeyFrame(layers()->currentLayerIndex(), currentFrame()); } -KeyFrame* Editor::addKeyFrame(int layerNumber, int frameIndex) +KeyFrame* Editor::addKeyFrame(const int layerNumber, int frameIndex) { Layer* layer = mObject->getLayer(layerNumber); Q_ASSERT(layer); @@ -1237,13 +1241,11 @@ KeyFrame* Editor::addKeyFrame(int layerNumber, int frameIndex) } } - bool ok = layer->addNewKeyFrameAt(frameIndex); - if (ok) - { - scrubTo(frameIndex); // currentFrameChanged() emit inside. - emit frameModified(frameIndex); - layers()->notifyAnimationLengthChanged(); - } + const bool ok = layer->addNewKeyFrameAt(frameIndex); + Q_ASSERT(ok); // We already ensured that there is no keyframe at frameIndex, so this should always succeed + scrubTo(frameIndex); // currentFrameChanged() emit inside. + emit frameModified(frameIndex); + layers()->notifyAnimationLengthChanged(); return layer->getKeyFrameAt(frameIndex); } diff --git a/core_lib/src/interface/editor.h b/core_lib/src/interface/editor.h index a3ba0e179e..61bd2a81d8 100644 --- a/core_lib/src/interface/editor.h +++ b/core_lib/src/interface/editor.h @@ -182,6 +182,12 @@ class Editor : public QObject void scrubForward(); void scrubBackward(); + /** + * Attempts to create a new keyframe at the current frame and layer. If the current frame already holds a keyframe, + * the new one will instead be created on the first empty frame that follows. If the current layer is not visible, a + * warning dialog will be shown to the user and no new keyframe is created. + * @return The newly created keyframe or `nullptr` if the layer is not visible + */ KeyFrame* addNewKey(); void removeKey(); @@ -268,7 +274,16 @@ class Editor : public QObject bool mAutosaveNeverAskAgain = false; void makeConnections(); - KeyFrame* addKeyFrame(int layerNumber, int frameNumber); + /** + * Attempts to create a new keyframe at the given position and layer. If a keyframe already exists at the position, + * the new one will instead be created on the first empty frame that follows. If the layer is not visible, a warning + * dialog will be shown to the user and no new keyframe is created. + * @param layerNumber The number of an existing layer on which the keyframe is to be created + * @param frameIndex The desired position of the new keyframe + * @return The newly created keyframe or `nullptr` if the layer is not visible + * @see Editor::addNewKey() + */ + KeyFrame* addKeyFrame(int layerNumber, int frameIndex); QList mTemporaryDirs; diff --git a/core_lib/src/structure/layer.cpp b/core_lib/src/structure/layer.cpp index f272e59120..c05ca919f6 100644 --- a/core_lib/src/structure/layer.cpp +++ b/core_lib/src/structure/layer.cpp @@ -172,7 +172,12 @@ bool Layer::addNewKeyFrameAt(int position) if (position <= 0) return false; KeyFrame* key = createKeyFrame(position); - return addKeyFrame(position, key); + if (!addKeyFrame(position, key)) + { + delete key; + return false; + } + return true; } void Layer::addOrReplaceKeyFrame(int position, KeyFrame* pKeyFrame) diff --git a/core_lib/src/structure/layer.h b/core_lib/src/structure/layer.h index 0e82e2b1db..addd3ccb37 100644 --- a/core_lib/src/structure/layer.h +++ b/core_lib/src/structure/layer.h @@ -94,8 +94,19 @@ class Layer : public QObject */ bool insertExposureAt(int position); + /** + * Creates a new keyframe at the given position, unless one already exists. + * @param position The position of the new keyframe + * @return false if a keyframe already exists at the position, true if the new keyframe was successfully added + */ bool addNewKeyFrameAt(int position); void addOrReplaceKeyFrame(int position, KeyFrame* pKeyFrame); + /** + * Adds a keyframe at the given position, unless one already exists. + * @param position The new position of the keyframe + * @param pKeyFrame The keyframe to add. Its previous position will be overwritten + * @return false if a keyframe already exists at the position, true if the keyframe was successfully added + */ virtual bool addKeyFrame(int position, KeyFrame* pKeyFrame); virtual bool removeKeyFrame(int position); bool swapKeyFrames(int position1, int position2); diff --git a/core_lib/src/util/pencilerror.cpp b/core_lib/src/util/pencilerror.cpp index 9ba3046929..37198c48c3 100644 --- a/core_lib/src/util/pencilerror.cpp +++ b/core_lib/src/util/pencilerror.cpp @@ -17,9 +17,7 @@ GNU General Public License for more details. #include "pencilerror.h" #include -#include #include -#include "pencildef.h" DebugDetails::DebugDetails() { diff --git a/core_lib/src/util/pencilerror.h b/core_lib/src/util/pencilerror.h index e35870f6cd..ecd8848bf7 100644 --- a/core_lib/src/util/pencilerror.h +++ b/core_lib/src/util/pencilerror.h @@ -45,7 +45,7 @@ class Status OK = 0, SAFE, FAIL, - CANCELED, + CANCELED, FILE_NOT_FOUND, NOT_SUPPORTED, INVALID_ARGUMENT, @@ -65,8 +65,8 @@ class Status // Sound ERROR_LOAD_SOUND_FILE, - // Export - ERROR_FFMPEG_NOT_FOUND, + // Export + ERROR_FFMPEG_NOT_FOUND, // Layer ERROR_NEED_AT_LEAST_ONE_CAMERA_LAYER @@ -100,7 +100,7 @@ class Status #ifndef STATUS_CHECK #define STATUS_CHECK( x )\ - { Status st = (x); if (!st.ok()) { return st; } } + { Status st = (x); if (!st.ok()) { return st; } } #endif #ifndef STATUS_FAILED