Skip to content

Commit

Permalink
Fix assertion failure after panning
Browse files Browse the repository at this point in the history
Don't assert that mPotentiallySelecting is true, as it's OK to
simply return early; the canvas' internal state should be fine.

Fixes #50.
  • Loading branch information
mitchcurtis committed May 8, 2018
1 parent fbc459a commit decb80f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
8 changes: 7 additions & 1 deletion app/imagecanvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,13 @@ void ImageCanvas::updateOrMoveSelectionArea()

void ImageCanvas::updateSelectionArea()
{
Q_ASSERT(mPotentiallySelecting);
if (!mPotentiallySelecting) {
// updateSelectionArea() can be called by updateOrMoveSelectionArea() as a result
// of moving after panning (all without releasing the mouse). In that case,
// we can't be selecting, as we were just panning, so we return early.
// Previously we would assert that mPotentiallySelecting was true, but that's too strict.
return;
}

QRect newSelectionArea(mPressScenePosition.x(), mPressScenePosition.y(),
mCursorSceneX - mPressScenePosition.x(), mCursorSceneY - mPressScenePosition.y());
Expand Down
39 changes: 39 additions & 0 deletions tests/tst_app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ private Q_SLOTS:
void flipPastedImage();
void selectionEdgePan_data();
void selectionEdgePan();
void panThenMoveSelection();

void fillImageCanvas_data();
void fillImageCanvas();
Expand Down Expand Up @@ -2673,6 +2674,44 @@ void tst_App::selectionEdgePan()
QTest::mouseRelease(window, Qt::LeftButton, Qt::NoModifier, cursorWindowPos);
}

// https://github.com/mitchcurtis/slate/issues/50
// When panning while having an active selection, and then releasing the space bar
// and hence starting a drag, there would be an assertion failure (Q_ASSERT(mPotentiallySelecting))
// in ImageCanvas::updateSelectionArea().
void tst_App::panThenMoveSelection()
{
QVERIFY2(createNewLayeredImageProject(), failureMessage);

QVERIFY2(switchTool(ImageCanvas::SelectionTool), failureMessage);

setCursorPosInScenePixels(100, 100);
QTest::mousePress(window, Qt::LeftButton, Qt::NoModifier, cursorWindowPos);

// Add an extra move otherwise the pan isn't started for some reason.
QTest::mouseMove(window, cursorWindowPos + QPoint(1, 1));

setCursorPosInScenePixels(120, 120);
QTest::mouseMove(window, cursorWindowPos);
QTest::mouseRelease(window, Qt::LeftButton, Qt::NoModifier, cursorWindowPos);
QVERIFY(layeredImageCanvas->hasSelection());

// Start panning.
QTest::keyPress(window, Qt::Key_Space);
QCOMPARE(window->cursor().shape(), Qt::OpenHandCursor);

QTest::mousePress(window, Qt::LeftButton, Qt::NoModifier, cursorWindowPos);
QCOMPARE(window->cursor().shape(), Qt::ClosedHandCursor);

// Stop panning but don't release the mouse.
QTest::keyRelease(window, Qt::Key_Space);

// Move the mouse slightly and it would previously crash with an assertion failure.
QTest::mouseMove(window, cursorWindowPos + QPoint(1, 1));

// Release to finish up.
QTest::mouseRelease(window, Qt::LeftButton, Qt::NoModifier, cursorWindowPos);
}

void tst_App::fillImageCanvas_data()
{
addImageProjectTypes();
Expand Down

0 comments on commit decb80f

Please sign in to comment.