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.

Sign up for GitHub

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

iss: 1873 - Fix frame not being drawn on the timeline #1875

Closed
wants to merge 2 commits into from
Closed

iss: 1873 - Fix frame not being drawn on the timeline #1875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f2f1c31
AC-1873 - Fix frame not being drawn on the timeline
MrStevns Aug 25, 2024
f85237c
Only deselect layer if the index has changed
MrStevns Aug 25, 2024
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
25 changes: 12 additions & 13 deletions app/src/timelinecells.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,12 +444,19 @@ void TimeLineCells::paintFrames(QPainter& painter, QColor trackCol, const Layer*

int recHeight = height - 4;

layer->foreachKeyFrame([&](KeyFrame* key)
const QList<int> selectedFrames = layer->getSelectedFramesByPos();
for (auto pair : layer->keyframes())
{
const KeyFrame* key = pair.second;
int framePos = key->pos();
int recWidth = standardWidth;
int recLeft = getFrameX(framePos) - recWidth;

// Selected frames are painted separately
if (selectedFrames.contains(framePos)) {
continue;
}

if (key->length() > 1)
{
// This is especially for sound clips.
Expand All @@ -463,19 +470,11 @@ void TimeLineCells::paintFrames(QPainter& painter, QColor trackCol, const Layer*
// Paint the frame contents
if (selected)
{
if (key->isSelected()) {
painter.setBrush(QColor(60, 60, 60));
}
else
{
painter.setBrush(QColor(trackCol.red(), trackCol.green(), trackCol.blue(), 150));
}
painter.setBrush(QColor(trackCol.red(), trackCol.green(), trackCol.blue(), 150));
}

if (!key->isSelected()) {
painter.drawRect(recLeft, recTop, recWidth, recHeight);
}
});
painter.drawRect(recLeft, recTop, recWidth, recHeight);
}
}

void TimeLineCells::paintCurrentFrameBorder(QPainter &painter, int recLeft, int recTop, int recWidth, int recHeight) const
Expand Down Expand Up @@ -729,7 +728,7 @@ void TimeLineCells::paintEvent(QPaintEvent*)
int currentFrame = mEditor->currentFrame();
Layer* currentLayer = mEditor->layers()->currentLayer();
KeyFrame* keyFrame = currentLayer->getKeyFrameWhichCovers(currentFrame);
if (keyFrame != nullptr && !keyFrame->isSelected())
if (keyFrame != nullptr)
{
int recWidth = keyFrame->length() == 1 ? mFrameSize - 2 : mFrameSize * keyFrame->length();
int recLeft = getFrameX(keyFrame->pos()) - (mFrameSize - 2);
Expand Down
4 changes: 3 additions & 1 deletion core_lib/src/managers/layermanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ void LayerManager::setCurrentLayer(int layerIndex)

// Deselect frames of previous layer.
Layer* previousLayer = currentLayer();
previousLayer->deselectAll();
if (previousLayer != object()->getLayer(layerIndex)) {
previousLayer->deselectAll();
}

emit currentLayerWillChange(layerIndex);

Expand Down
2 changes: 0 additions & 2 deletions core_lib/src/structure/keyframe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ KeyFrame::KeyFrame(const KeyFrame& k2)
mFrame = k2.mFrame;
mLength = k2.mLength;
mIsModified = k2.mIsModified;
mIsSelected = k2.mIsSelected;
mAttachedFileName = k2.mAttachedFileName;
// intentionally not copying event listeners
}
Expand All @@ -50,7 +49,6 @@ KeyFrame& KeyFrame::operator=(const KeyFrame& k2)
mFrame = k2.mFrame;
mLength = k2.mLength;
mIsModified = k2.mIsModified;
mIsSelected = k2.mIsSelected;
mAttachedFileName = k2.mAttachedFileName;
// intentionally not copying event listeners
return *this;
Expand Down
4 changes: 0 additions & 4 deletions core_lib/src/structure/keyframe.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ class KeyFrame
void setModified(bool b) { mIsModified = b; }
bool isModified() const { return mIsModified; }

void setSelected(bool b) { mIsSelected = b; }
bool isSelected() const { return mIsSelected; }

QString fileName() const { return mAttachedFileName; }
void setFileName(QString strFileName) { mAttachedFileName = strFileName; }

Expand All @@ -65,7 +62,6 @@ class KeyFrame
int mFrame = -1;
int mLength = 1;
bool mIsModified = true;
bool mIsSelected = false;
QString mAttachedFileName;

std::vector<KeyFrameEventListener*> mEventListeners;
Expand Down
14 changes: 2 additions & 12 deletions core_lib/src/structure/layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,7 @@ bool Layer::removeKeyFrame(int position)

if (frame)
{
if (frame->isSelected()) {
removeFromSelectionList(frame->pos());
}
removeFromSelectionList(frame->pos());
mKeyFrames.erase(frame->pos());
markFrameAsDirty(frame->pos());
delete frame;
Expand Down Expand Up @@ -383,9 +381,7 @@ bool Layer::isFrameSelected(int position) const
KeyFrame* keyFrame = getKeyFrameWhichCovers(position);
if (keyFrame == nullptr) { return false; }

int frameFound = mSelectedFrames_byLast.contains(keyFrame->pos());
Q_ASSERT(!frameFound || keyFrame->isSelected());
return frameFound;
return mSelectedFrames_byLast.contains(keyFrame->pos());
}

void Layer::setFrameSelected(int position, bool isSelected)
Expand All @@ -410,7 +406,6 @@ void Layer::setFrameSelected(int position, bool isSelected)
mSelectedFrames_byLast.removeOne(startPosition);
mSelectedFrames_byPosition.removeOne(startPosition);
}
keyFrame->setSelected(isSelected);
}
}

Expand Down Expand Up @@ -496,11 +491,6 @@ void Layer::deselectAll()
{
mSelectedFrames_byLast.clear();
mSelectedFrames_byPosition.clear();

for (auto pair : mKeyFrames)
{
pair.second->setSelected(false);
}
}

bool Layer::canMoveSelectedFramesToOffset(int offset) const
Expand Down
2 changes: 2 additions & 0 deletions core_lib/src/structure/layer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ class Layer
/** Clear the list of dirty keyframes */
void clearDirtyFrames() { mDirtyFrames.clear(); }

std::map<int, KeyFrame*, std::greater<int>> keyframes() const { return mKeyFrames; }

protected:
virtual KeyFrame* createKeyFrame(int position) = 0;
bool loadKey(KeyFrame*);
Expand Down
28 changes: 14 additions & 14 deletions tests/src/test_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ TEST_CASE("Layer::moveKeyFrame(int position, int offset)")
layer->moveKeyFrame(2, 1);

// Confirm that both frames are still selected.
REQUIRE(layer->getKeyFrameAt(2)->isSelected());
REQUIRE(layer->getKeyFrameAt(3)->isSelected());
REQUIRE(layer->isFrameSelected(2));
REQUIRE(layer->isFrameSelected(3));

// Verify that poiners has been swapped
REQUIRE(frame1 == layer->getKeyFrameAt(3));
Expand All @@ -376,8 +376,8 @@ TEST_CASE("Layer::moveKeyFrame(int position, int offset)")
layer->moveKeyFrame(2, 1);

// Confirm that both frames are still selected.
REQUIRE(layer->getKeyFrameAt(2)->isSelected());
REQUIRE_FALSE(layer->getKeyFrameAt(3)->isSelected());
REQUIRE(layer->isFrameSelected(2));
REQUIRE_FALSE(layer->isFrameSelected(3));
}

SECTION("move non selected frame across multiple selected frames")
Expand All @@ -402,11 +402,11 @@ TEST_CASE("Layer::moveKeyFrame(int position, int offset)")
layer->moveKeyFrame(8, 1);

// Confirm that both frames are still selected.
REQUIRE(layer->getKeyFrameAt(4)->isSelected());
REQUIRE(layer->getKeyFrameAt(5)->isSelected());
REQUIRE(layer->getKeyFrameAt(6)->isSelected());
REQUIRE(layer->getKeyFrameAt(7)->isSelected());
REQUIRE_FALSE(layer->getKeyFrameAt(9)->isSelected());
REQUIRE(layer->isFrameSelected(4));
REQUIRE(layer->isFrameSelected(5));
REQUIRE(layer->isFrameSelected(6));
REQUIRE(layer->isFrameSelected(7));
REQUIRE_FALSE(layer->isFrameSelected(9));
}

SECTION("move selected frame across multiple not selected frames")
Expand All @@ -433,11 +433,11 @@ TEST_CASE("Layer::moveKeyFrame(int position, int offset)")
layer->moveKeyFrame(4, -1);

// Confirm that both frames are still selected.
REQUIRE(layer->getKeyFrameAt(3)->isSelected());
REQUIRE_FALSE(layer->getKeyFrameAt(5)->isSelected());
REQUIRE_FALSE(layer->getKeyFrameAt(6)->isSelected());
REQUIRE_FALSE(layer->getKeyFrameAt(7)->isSelected());
REQUIRE_FALSE(layer->getKeyFrameAt(8)->isSelected());
REQUIRE(layer->isFrameSelected(3));
REQUIRE_FALSE(layer->isFrameSelected(5));
REQUIRE_FALSE(layer->isFrameSelected(6));
REQUIRE_FALSE(layer->isFrameSelected(7));
REQUIRE_FALSE(layer->isFrameSelected(8));
}

delete obj;
Expand Down
Loading