Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when importing image to empty frame on hidden layer #1820

Merged
merged 5 commits into from
Apr 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/src/timelinecells.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 16 additions & 14 deletions core_lib/src/interface/editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@ GNU General Public License for more details.

#include "editor.h"

#include <QApplication>
#include <QClipboard>
#include <QTimer>
#include <QImageReader>
#include <QDropEvent>
#include <QMimeData>
#include <QTemporaryDir>

#include "object.h"
Expand Down Expand Up @@ -919,7 +916,13 @@ Status Editor::importBitmapImage(const QString& filePath)
QImageReader reader(filePath);

Q_ASSERT(layers()->currentLayer()->type() == Layer::BITMAP);
auto layer = static_cast<LayerBitmap*>(layers()->currentLayer());
const auto layer = static_cast<LayerBitmap*>(layers()->currentLayer());

if (!layer->visible())
{
mScribbleArea->showLayerNotVisibleWarning();
return Status::SAFE;
}

Status status = Status::OK;
DebugDetails dd;
Expand All @@ -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.");
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down
17 changes: 16 additions & 1 deletion core_lib/src/interface/editor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second parameter renamed so declaration matches definition


QList<QTemporaryDir*> mTemporaryDirs;

Expand Down
7 changes: 6 additions & 1 deletion core_lib/src/structure/layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions core_lib/src/structure/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions core_lib/src/util/pencilerror.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ GNU General Public License for more details.

#include "pencilerror.h"
#include <map>
#include <QObject>
#include <QSysInfo>
#include "pencildef.h"

DebugDetails::DebugDetails()
{
Expand Down
8 changes: 4 additions & 4 deletions core_lib/src/util/pencilerror.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Status
OK = 0,
SAFE,
FAIL,
CANCELED,
CANCELED,
FILE_NOT_FOUND,
NOT_SUPPORTED,
INVALID_ARGUMENT,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading