From dd0c194a3c6f8c6e84eeaf2d5c44e7191d558bd2 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Fri, 18 Sep 2026 19:57:59 +1200 Subject: [PATCH] Paste under the cursor, and give the context menu back (#913) Two fixes in the placement tool Ctrl+V starts. Paste appeared on top of the original, not under the cursor. 55c2c0df9 added the placement tool precisely so the copy would not land invisibly on top of what was copied, but 53a0f07ca then warped the pointer to the group's grid-snapped origin -- which is the original's position -- so the copy reappeared exactly over the original until the mouse was moved. The start_pos the caller computes from the cursor was left unread: three mentions in the file, all declaration or comment. Reported on #913, where it reads as Ctrl+V pasting in place. Move the group to the cursor instead of the cursor to the group. Both put the copy under the pointer; only one of them takes the pointer away from where the user put it. start_pos is honoured, the items are translated once at construction, their conductors re-routed before anything is drawn, and the movement baseline is set there too rather than waiting for the first mouse move. That made the baseline sentinel matter, so it is now the m_baseline_captured flag the header already declared and nothing used, rather than m_initial_cursor.isNull() -- which cannot tell "not set yet" from a baseline that is legitimately scene (0,0). Separately: one Ctrl+V killed the folio's right-click menu for the rest of the session. init() sets Qt::NoContextMenu so a right click cancels the placement instead of opening a menu over it, and nothing ever set it back, taking "Coller ici", "Collage multiple", the folio properties and add/remove column/row with it. Every other DiagramEvent* class restores the policy in its destructor; this one did not. Confirmed against an unmodified master build: the menu opens before a paste and not after one. It matters here because "use the right-click menu instead" is the answer people are given when Ctrl+V does not place where they wanted. Verified on a built binary driven through a virtual X display, against examples/741.qet and convertisseur.qet: - the pointer does not move across Ctrl+V (1300,870 before and after; on master it jumps to the original at 798,455), and the copy is under it - a multi-element selection keeps its layout and its conductors re-route - Escape and right-click both cancel, leaving nothing behind and nothing on the undo stack; click and Return both commit; one undo removes the whole paste and redo restores it - with the pointer outside the viewport the copy lands at the viewport centre, visible, and follows correctly once the mouse enters - pasting into a different folio from the one copied from works - the context menu opens before a paste, after a cancelled one, and after a committed one, with "Coller ici" present ctest 9/9. Co-Authored-By: Claude Opus 5 --- sources/diagramevent/diagrameventaddpaste.cpp | 85 +++++++++++++------ sources/diagramevent/diagrameventaddpaste.h | 15 ++-- 2 files changed, 70 insertions(+), 30 deletions(-) diff --git a/sources/diagramevent/diagrameventaddpaste.cpp b/sources/diagramevent/diagrameventaddpaste.cpp index ac29b522a..5c867936c 100644 --- a/sources/diagramevent/diagrameventaddpaste.cpp +++ b/sources/diagramevent/diagrameventaddpaste.cpp @@ -59,9 +59,8 @@ if (movable.isEmpty()) return; //Compute the top-left of all items' positions (not bounding - //rects) and snap to grid — used only for the initial cursor - //warp. Items stay at their original XML positions; - //moveTo() handles grid-snapped movement via deltas. + //rects) and snap to grid: this is the point that gets placed + //under the cursor, and the baseline moveTo() measures from. QPointF top_left; bool first = true; for (auto *item : movable) { @@ -79,17 +78,42 @@ Diagram::xGrid).toInt(); const int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"), Diagram::yGrid).toInt(); - const QPointF grid_origin( - qRound(top_left.x() / xGrid) * xGrid, - qRound(top_left.y() / yGrid) * yGrid); + const auto snapGrid = [xGrid, yGrid](const QPointF &p) -> QPointF { + return QPointF( + qRound(p.x() / xGrid) * xGrid, + qRound(p.y() / yGrid) * yGrid); + }; + const QPointF grid_origin = snapGrid(top_left); - //Store each item's position. moveTo() applies a grid-snapped - //delta from the baseline, so items preserve their layout and - //move in whole grid steps. + //Move the group to the cursor, rather than the cursor to the + //group. Both put the copy under the pointer, but warping the + //pointer also drags it back to the original's position, so the + //copy appears exactly on top of what was copied until the mouse + //is moved -- which is the thing pasting under the cursor was + //meant to avoid (issue #913). Taking the pointer away from + //where the user put it is also its own surprise. + m_group_origin = snapGrid(start_pos); + const QPointF offset = m_group_origin - grid_origin; + + //Store each item's position after the move. moveTo() applies a + //grid-snapped delta from the baseline to these, so items + //preserve their layout and move in whole grid steps. for (auto *item : movable) { + item->setPos(item->pos() + offset); m_relative_pos.insert(item, item->pos()); } - m_group_origin = grid_origin; + + //The conductors were laid out against the old terminal + //positions, so re-route them before anything is drawn. + const QList conductors = m_content.conductors(DiagramContent::AnyConductor); + for (auto *conductor : conductors) { + conductor->updatePath(); + } + + //The baseline is known now, so moveTo() does not have to + //capture one from the first mouse movement. + m_initial_cursor = m_group_origin; + m_baseline_captured = true; m_diagram->clearSelection(); for (auto *item : movable) { @@ -101,11 +125,6 @@ if (const auto qde = QETApp::diagramEditorAncestorOf(view)) { m_status_bar = qde->statusBar(); } - //Warp the cursor close to the group origin so the - //first mouseMoveEvent captures the correct baseline. - const QPoint view_pos = view->mapFromScene(m_group_origin); - const QPoint global_pos = view->viewport()->mapToGlobal(view_pos); - QCursor::setPos(global_pos); } } showHint(); @@ -129,6 +148,20 @@ DiagramEventAddPaste::~DiagramEventAddPaste() if (m_status_bar) { m_status_bar->clearMessage(); } + + //Give the context menu back. init() turned it off so a right + //click would cancel the placement instead of opening a menu over + //it, and nothing turned it on again: one Ctrl+V left the folio's + //right-click menu dead for the rest of the session, taking + //"Coller ici", "Collage multiple" and the folio properties with + //it. Every other DiagramEvent* class restores it here; this one + //did not. + if (m_diagram) { + const auto views = m_diagram->views(); + for (auto *view : views) { + view->setContextMenuPolicy(Qt::DefaultContextMenu); + } + } } /** @@ -166,9 +199,10 @@ void DiagramEventAddPaste::showHint() /** @brief DiagramEventAddPaste::moveTo Compute a grid-snapped delta from the initial cursor position and - apply it to every item's grid-shifted position. This keeps all - items exactly on grid points regardless of modifier keys or - sub-pixel cursor-warp rounding. + apply it to every item's stored position. Working from a delta + against a fixed baseline, rather than from the previous position, + keeps all items exactly on grid points regardless of modifier keys + and stops rounding accumulating over a long drag. */ void DiagramEventAddPaste::moveTo(const QPointF &scene_pos) { @@ -184,14 +218,15 @@ void DiagramEventAddPaste::moveTo(const QPointF &scene_pos) qRound(p.y() / yGrid) * yGrid); }; - //On the very first call, record the actual grid-snapped - //cursor position as baseline. The cursor warp in the - //constructor goes through integer rounding (mapFromScene → - //QPoint) so the real position may differ slightly from - //m_initial_cursor. Using the actual scene position avoids - //a one-grid-unit jump on the first mouse movement. - if (m_initial_cursor.isNull()) { + //The constructor normally sets the baseline, having just put the + //group there. This covers the case where it could not -- no view + //to map through -- by taking the first cursor position instead. + //Tested with m_baseline_captured rather than + //m_initial_cursor.isNull(), which silently re-baselines when the + //baseline is legitimately scene (0,0). + if (!m_baseline_captured) { m_initial_cursor = snapGrid(scene_pos); + m_baseline_captured = true; return; } diff --git a/sources/diagramevent/diagrameventaddpaste.h b/sources/diagramevent/diagrameventaddpaste.h index 28ae3854b..2cb7eaf9f 100644 --- a/sources/diagramevent/diagrameventaddpaste.h +++ b/sources/diagramevent/diagrameventaddpaste.h @@ -79,13 +79,18 @@ class DiagramEventAddPaste : public DiagramEventInterface ///Each movable item's position relative to the group's top left, ///taken once so repeated moves cannot accumulate rounding drift. QHash m_relative_pos; - ///Top-left corner of the bounding rect of all movable items, - ///in scene coordinates, captured when the paste starts. + ///Where the group's grid-snapped top left was put when the paste + ///started, in scene coordinates -- the cursor, so the copy + ///appears under the pointer rather than on top of what was + ///copied. QPointF m_group_origin; - ///Cursor position (scene coords) at the moment the paste starts, - ///so delta-based movement can compute offsets from the initial point. + ///Cursor position (scene coords) the delta-based movement in + ///moveTo() measures from. Equal to m_group_origin, since the + ///group is placed at the cursor. QPointF m_initial_cursor; - ///Set to true once the first moveTo() captures the real cursor position. + ///Whether m_initial_cursor holds a usable baseline. A flag rather + ///than testing m_initial_cursor.isNull(), which cannot tell "not + ///set yet" from a baseline that is legitimately scene (0,0). bool m_baseline_captured{false}; QPointer m_status_bar; bool m_finished{false};