From 1330dbb4351a4cd89d6222bffe5b67e06f0b2110 Mon Sep 17 00:00:00 2001 From: Andre Rummler Date: Fri, 25 Sep 2026 13:37:49 +0200 Subject: [PATCH] Fix janking behaviour during resize and Ctrl action. --- sources/diagramevent/diagrameventaddshape.cpp | 46 ++++++++++--------- sources/diagramevent/diagrameventaddshape.h | 2 +- sources/qetgraphicsitem/qetshapeitem.cpp | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/sources/diagramevent/diagrameventaddshape.cpp b/sources/diagramevent/diagrameventaddshape.cpp index 9795d86f8..26072980a 100644 --- a/sources/diagramevent/diagrameventaddshape.cpp +++ b/sources/diagramevent/diagrameventaddshape.cpp @@ -87,9 +87,9 @@ DiagramEventAddShape::~DiagramEventAddShape() Applies a drag/click position to the in-progress shape, honouring two modifiers that mirror how the very same shape can already be edited afterward, once placed: - - Ctrl, for Rectangle/Ellipse only: the first click becomes the - shape's *center* rather than a corner, growing symmetrically as - the cursor moves away from it -- the same meaning Ctrl already + - Ctrl, for Rectangle/Ellipse only: the first click's point acts as + the shape's *center* rather than a corner, growing symmetrically + as the cursor moves away from it -- the same meaning Ctrl already has on a Resize handle (anchor at center). Deliberately not offered for Line: unlike the Rectangle/Ellipse case, there's no established convention for "a line grows symmetrically from its @@ -100,12 +100,18 @@ DiagramEventAddShape::~DiagramEventAddShape() dragged dimensions is currently larger and mirroring that onto the other, preserving the direction the user is actually dragging in. - Both can combine (Ctrl+Shift: a centered square/circle). Whether or - not Ctrl is currently held, the non-anchored branch always rebuilds - from m_anchor_point rather than nudging the existing rect/line -- - otherwise, if Ctrl had been held earlier in the same drag (moving the - shape's own first point to a mirrored position), releasing it would - leave that point stuck there instead of actually restoring it. + Both can combine (Ctrl+Shift: a centered square/circle). Whether Ctrl + currently anchors from the center is re-decided on every call, from + the live modifiers passed in here -- not frozen at whatever was held + on the first click. Every comparable tool (Illustrator, Photoshop, + Figma, Inkscape...) lets you press or release the center-origin + modifier at any point mid-drag, with the shape immediately jumping to + match; that jump is the expected feedback for changing which point is + anchored, not a glitch. Freezing the choice at the first click instead + meant holding Ctrl anywhere other than the initial mouse-down did + nothing visible -- which, tried the more natural way (drag first, + then reach for Ctrl once you decide you want it centered), read as + "Ctrl doesn't work" rather than as a deliberate one-shot decision. */ void DiagramEventAddShape::applyPosition(const QPointF &pos, Qt::KeyboardModifiers mods) { @@ -125,14 +131,13 @@ void DiagramEventAddShape::applyPosition(const QPointF &pos, Qt::KeyboardModifie return; } - // m_center_anchored is decided once, in mousePressEvent, not - // re-checked here on every call -- re-checking it live meant - // releasing Ctrl mid-drag (something you'd naturally do the moment - // your hand gets tired holding it, long before you're done resizing) - // silently snapped the shape back to corner-anchored, discarding - // what felt like an already-made decision. Deciding it once at the - // first click matches "I held Ctrl when I clicked, so this shape is - // centered" -- a single, predictable rule instead of a live toggle. + m_center_anchored = (mods & Qt::ControlModifier) + && (m_shape_type == QetShapeItem::Rectangle || m_shape_type == QetShapeItem::Ellipse); + if (m_center_anchored) + showCenterMarker(m_anchor_point); + else + hideCenterMarker(); + QPointF target = pos; if ((mods & Qt::ShiftModifier) @@ -218,10 +223,9 @@ void DiagramEventAddShape::mousePressEvent(QGraphicsSceneMouseEvent *event) { m_shape_item = new QetShapeItem(pos, pos, m_shape_type); m_anchor_point = pos; - // Decided once, here, rather than re-checked on every mouse - // move for the rest of the drag -- see applyPosition()'s doc - // comment for why continuous re-checking made releasing Ctrl - // mid-drag feel like a bug rather than a deliberate choice. + // Initial feedback only -- applyPosition() re-decides this + // live on every subsequent move, from whatever Ctrl state is + // held at the time. m_center_anchored = (event->modifiers() & Qt::ControlModifier) && (m_shape_type == QetShapeItem::Rectangle || m_shape_type == QetShapeItem::Ellipse); if (m_center_anchored) diff --git a/sources/diagramevent/diagrameventaddshape.h b/sources/diagramevent/diagrameventaddshape.h index 7999897f6..9f46b4408 100644 --- a/sources/diagramevent/diagrameventaddshape.h +++ b/sources/diagramevent/diagrameventaddshape.h @@ -59,7 +59,7 @@ class DiagramEventAddShape : public DiagramEventInterface QGraphicsLineItem *m_help_horiz, *m_help_verti; QPointF m_anchor_point; // the shape's first-click point -- meaningful once m_shape_item exists QGraphicsEllipseItem *m_center_marker = nullptr; // shown only while Ctrl-anchoring is actually in effect, so it doubles as confirmation that it is - bool m_center_anchored = false; // decided once, at the first click -- see applyPosition()'s doc comment for why + bool m_center_anchored = false; // re-decided live on every applyPosition() call, from current Ctrl state QPointF m_last_mouse_scene_pos; // raw, unsnapped -- lets a modifier-only change re-snap correctly when reapplied }; diff --git a/sources/qetgraphicsitem/qetshapeitem.cpp b/sources/qetgraphicsitem/qetshapeitem.cpp index cca68d38c..f12c3b3f5 100644 --- a/sources/qetgraphicsitem/qetshapeitem.cpp +++ b/sources/qetgraphicsitem/qetshapeitem.cpp @@ -2385,7 +2385,7 @@ void QetShapeItem::dragResize(int index, const QPointF &localPos, Qt::KeyboardMo : QetGraphicsHandlerUtility::rectForPosAtIndex(localRect(), localPos, index); if (mods & Qt::ShiftModifier) - newRect = lockAspectRatio(localRect(), newRect, index, mirrored); + newRect = lockAspectRatio(QRectF(m_old_P1, m_old_P2).normalized(), newRect, index, mirrored); setRect(newRect.normalized()); }