From f654381868cb3bbb0e0d372ce1f4a77dcd1a09a5 Mon Sep 17 00:00:00 2001 From: Jon Kinney Date: Wed, 19 Aug 2026 00:48:45 -0500 Subject: [PATCH] fix(editor): a spotlight resizes by its handles The handles sit on a layer's bounds, but a spotlight is hit-tested against its opening, so for the default elliptical shape the corner handle lies outside the shape, a press there read as empty canvas, and dragging it started a marquee instead of resizing. The press now asks which handle is under the pointer before it asks which layer is, which also leaves one copy of the tolerance test instead of two. --- src/editor.cpp | 47 ++++++++++++++++--------- src/editor.hpp | 4 +++ tests/editor-smoke.cpp | 79 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 16 deletions(-) diff --git a/src/editor.cpp b/src/editor.cpp index 03b79f3d..fd80f323 100644 --- a/src/editor.cpp +++ b/src/editor.cpp @@ -551,6 +551,29 @@ bool CaptureEditor::annotationSelected(int index) const { return selectedAnnotations_.contains(index); } +CaptureEditor::Interaction +CaptureEditor::selectedHandleAt(const QPointF &point) const { + // A layer's handles can sit well outside the layer itself. A spotlight is + // hit-tested against its opening, and the corners of its bounds are out in + // the dimmed surround, so a press has to ask about handles before it asks + // what shape is under the pointer. Otherwise the handle misses, the press + // reads as empty canvas, and dragging a spotlight's corner starts a marquee + // instead of resizing it. + if (selectedAnnotation_ < 0 || selectedAnnotation_ >= annotations_.size()) + return Interaction::None; + const Annotation &selected = annotations_.at(selectedAnnotation_); + const qreal tolerance = 9.0 / std::max(editScale(), 0.01); + const QRectF bounds = annotationBounds(selected); + const bool endpoints = hasEndpointHandles(selected.kind); + const QPointF first = endpoints ? selected.start : bounds.topLeft(); + const QPointF last = endpoints ? selected.end : bounds.bottomRight(); + if (endpoints && QLineF(point, first).length() <= tolerance) + return Interaction::ResizeStart; + if (QLineF(point, last).length() <= tolerance) + return Interaction::ResizeEnd; + return Interaction::None; +} + int CaptureEditor::annotationAt(const QPointF &point) const { const auto containsPoint = [this, &point](const Annotation &annotation) { if (annotation.kind == Annotation::Kind::Arrow || @@ -2134,7 +2157,10 @@ void CaptureEditor::mousePressEvent(QMouseEvent *event) { const bool additive = heldModifiers(event->modifiers()).testFlag(Qt::ControlModifier) || heldModifiers(event->modifiers()).testFlag(Qt::MetaModifier); - const int hit = annotationAt(point); + // A handle of the layer already selected is a resize wherever it sits. + const Interaction handle = selectedHandleAt(point); + const int hit = + handle != Interaction::None ? selectedAnnotation_ : annotationAt(point); if (additive) { if (hit >= 0) { if (selectedAnnotations_.contains(hit)) { @@ -2176,21 +2202,10 @@ void CaptureEditor::mousePressEvent(QMouseEvent *event) { if (selectedAnnotations_.size() > 1) { interaction_ = Interaction::Move; } else { - const qreal tolerance = 9.0 / std::max(editScale(), 0.01); - const Annotation &selected = annotations_.at(hit); - const QRectF bounds = annotationBounds(selected); - const QPointF first = - hasEndpointHandles(selected.kind) ? selected.start : bounds.topLeft(); - const QPointF last = hasEndpointHandles(selected.kind) - ? selected.end - : bounds.bottomRight(); - interaction_ = - QLineF(point, first).length() <= tolerance && - hasEndpointHandles(selected.kind) - ? Interaction::ResizeStart - : QLineF(point, last).length() <= tolerance - ? Interaction::ResizeEnd - : Interaction::Move; + // selectedAnnotation_ is the layer under the press now, so this asks + // about that layer's own handles. + const Interaction onHit = selectedHandleAt(point); + interaction_ = onHit != Interaction::None ? onHit : Interaction::Move; } } if (selectedAnnotation_ >= 0) { diff --git a/src/editor.hpp b/src/editor.hpp index ed98eb60..a7dd5998 100644 --- a/src/editor.hpp +++ b/src/editor.hpp @@ -146,6 +146,10 @@ class CaptureEditor final : public QWidget { heldModifiers(Qt::KeyboardModifiers reported) const { return modifiersSeen_ ? reported : Qt::KeyboardModifiers(Qt::NoModifier); } + /// Which handle of the selected layer is under `point`, if any. Asked + /// before what shape is under the pointer, since a handle can sit outside + /// the layer it belongs to. + [[nodiscard]] Interaction selectedHandleAt(const QPointF &point) const; [[nodiscard]] int annotationAt(const QPointF &point) const; [[nodiscard]] int hoveredSpotlightAt(const QPointF &position) const; [[nodiscard]] QRectF normalizedSelection(const QPointF &first, diff --git a/tests/editor-smoke.cpp b/tests/editor-smoke.cpp index 8dba4921..c2c848f3 100644 --- a/tests/editor-smoke.cpp +++ b/tests/editor-smoke.cpp @@ -2036,6 +2036,81 @@ bool runStuckModifierSmoke(QApplication &application, QString &error) { return true; } +/** Checks that a spotlight resizes by its corner handle. The handle sits on + * the layer's bounds, which for an elliptical spotlight is outside the shape + * the hit test uses, so a press there used to read as empty canvas and start + * a marquee. */ +bool runSpotlightHandleSmoke(QApplication &application, QString &error) { + CaptureData capture; + capture.monitor.name = QStringLiteral("TEST"); + capture.monitor.geometry = {0, 0, 800, 600}; + capture.monitor.pixelSize = {800, 600}; + capture.monitor.scale = 1.0; + capture.source = QImage(800, 600, QImage::Format_ARGB32_Premultiplied); + // Fine banding, so a spotlight's lens is visible against the surround. + for (int y = 0; y < capture.source.height(); ++y) { + for (int x = 0; x < capture.source.width(); ++x) { + const int band = ((x / 3) + (y / 5)) % 3; + capture.source.setPixelColor( + x, y, QColor(40 + band * 70, 60 + band * 50, 90 + band * 40)); + } + } + capture.previewSize = capture.source.size(); + const QString snapshotPath = temporarySnapshotPath(); + + CaptureEditor editor(capture); + editor.resize(800, 600); + editor.show(); + application.processEvents(); + QTest::mousePress(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(100, 100)); + QTest::mouseMove(&editor, QPoint(700, 500), 20); + QTest::mouseRelease(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(700, 500)); + application.processEvents(); + + // A spotlight from (300,250) to (500,400), then selected by its middle. + QTest::keyClick(&editor, Qt::Key_S); + QTest::mousePress(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(300, 250)); + QTest::mouseMove(&editor, QPoint(500, 400), 20); + QTest::mouseRelease(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(500, 400)); + application.processEvents(); + QTest::keyClick(&editor, Qt::Key_V); + QTest::mouseClick(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(400, 325)); + application.processEvents(); + + const QImage placed = flushedSnapshot(editor, snapshotPath); + if (placed.isNull()) { + error = QStringLiteral("Spotlight handle smoke: nothing was rendered"); + return false; + } + // Its bottom-right handle is at widget (500,400), on the bounds and outside + // the ellipse. Dragging it in must resize the spotlight. + QTest::mousePress(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(500, 400)); + QTest::mouseMove(&editor, QPoint(430, 340), 20); + QTest::mouseRelease(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(430, 340)); + application.processEvents(); + const QImage resized = flushedSnapshot(editor, snapshotPath); + if (resized == placed) { + error = QStringLiteral("Dragging a spotlight's handle did nothing"); + return false; + } + // A resize keeps the opposite corner: moving it by the same delta would + // have produced a different picture. + QTest::keyClick(&editor, Qt::Key_Z, Qt::ControlModifier); + application.processEvents(); + QTest::mousePress(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(400, 325)); + QTest::mouseMove(&editor, QPoint(330, 265), 20); + QTest::mouseRelease(&editor, Qt::LeftButton, Qt::NoModifier, QPoint(330, 265)); + application.processEvents(); + if (flushedSnapshot(editor, snapshotPath) == resized) { + error = QStringLiteral("Dragging a spotlight's handle moved it instead of " + "resizing it"); + return false; + } + editor.close(); + QFile::remove(snapshotPath); + return true; +} + int main(int argc, char **argv) { // Re-executed by the instance-lock checks as the process holding the lock. const QString heldLockPath = @@ -2087,6 +2162,10 @@ int main(int argc, char **argv) { qWarning().noquote() << snapshotError; return 97; } + if (!runSpotlightHandleSmoke(application, snapshotError)) { + qWarning().noquote() << snapshotError; + return 98; + } if (!runAsyncCaptureRegionSmoke(application, snapshotError)) { qWarning().noquote() << snapshotError; return 82;