From 53a0f07ca3eedf256a33f2fc6c063c6f71b8716a Mon Sep 17 00:00:00 2001 From: Kellermorph Date: Thu, 17 Sep 2026 17:54:57 +0200 Subject: [PATCH] Fix copy-paste bugs in DiagramEventAddPaste and PasteDiagramCommand Fix multiple issues introduced by commit 55c2c0df9 (interactive paste): Paste placement (diagrameventaddpaste): - Items now load at their original XML coordinates instead of being snapped to the cursor position, preventing jumps on initial placement - Use delta-based movement: record the actual grid-snapped cursor position on first mouseMoveEvent as baseline, then compute grid-snapped deltas from there - Bypass Diagram::snapToGrid() in moveTo() to avoid Ctrl modifier causing pixel-snapping instead of grid-snapping - Remove moveTo() from mouseReleaseEvent to prevent a final jump on click - Warp cursor to group bounding rect origin for visual feedback Paste command (diagramcommands): - Always clear PLC slave data (type, address, function, comment, cross-ref, label, TC, T1-T4) on paste regardless of the erase-label-on-copy user preference - Block alignment (m_block_alignment / blockAlignmentUpdate) before setElementInformations() for both slaves and non-slaves, preventing finishAlignment() from shifting right/center-aligned text items - Clear non-UserText items directly after setElementInformations() for slaves as a safety net --- sources/diagramcommands.cpp | 87 +++++++++++++++---- sources/diagramevent/diagrameventaddpaste.cpp | 86 ++++++++++++++---- sources/diagramevent/diagrameventaddpaste.h | 20 +++-- 3 files changed, 153 insertions(+), 40 deletions(-) diff --git a/sources/diagramcommands.cpp b/sources/diagramcommands.cpp index 3bf1be437..8c19636f6 100644 --- a/sources/diagramcommands.cpp +++ b/sources/diagramcommands.cpp @@ -19,6 +19,7 @@ #include "diagram.h" #include "qetgraphicsitem/conductortextitem.h" +#include "qetgraphicsitem/dynamicelementtextitem.h" #include "qetgraphicsitem/element.h" #include "qetgraphicsitem/elementtextitemgroup.h" #include "qetinformation.h" @@ -90,6 +91,62 @@ void PasteDiagramCommand::redo() //make new uuid, because old uuid are the uuid of the copied element e -> newUuid(); + // PLC slaves carry master-specific data (type, address, + // function, cross-ref, etc.) in their elementInformations. + // Always clear those on paste so the duplicate starts clean, + // regardless of the user's erase-label-on-copy preference. + const bool is_slave = (e->linkType() == Element::Slave); + if (is_slave) { + DiagramContext dc = e->elementInformations(); + dc.remove(QETInformation::ELMT_PLC_TYPE); + dc.remove(QETInformation::ELMT_PLC_ADDRESS); + dc.remove(QETInformation::ELMT_PLC_FUNCTION); + dc.remove(QETInformation::ELMT_PLC_COMMENT); + dc.remove(QETInformation::ELMT_PLC_CROSSREF); + dc.remove(QETInformation::ELMT_LABEL); + dc.remove(QETInformation::ELMT_PLC_TC); + dc.remove(QETInformation::ELMT_PLC_T1); + dc.remove(QETInformation::ELMT_PLC_T2); + dc.remove(QETInformation::ELMT_PLC_T3); + dc.remove(QETInformation::ELMT_PLC_T4); + dc.remove(QStringLiteral("xref")); + + // Block alignment before setElementInformations so + // that elementInfoChanged() resolves texts without + // finishAlignment() shifting right/center-aligned items. + for (DynamicElementTextItem *deti : e->dynamicTextItems()) + deti->m_block_alignment = true; + for (auto *group : e->textGroups()) + group->blockAlignmentUpdate(true); + + e->setElementInformations(dc); + + for (DynamicElementTextItem *deti : e->dynamicTextItems()) + deti->m_block_alignment = false; + for (auto *group : e->textGroups()) + group->blockAlignmentUpdate(false); + + // After setElementInformations, elementInfoChanged() + // resolves composite text with cleaned dc. Clear + // all non-UserText items directly as a safety net. + for (DynamicElementTextItem *deti : e->dynamicTextItems()) { + if (deti->textFrom() != DynamicElementTextItem::UserText) { + deti->m_block_alignment = true; + deti->setPlainText(QString()); + deti->m_block_alignment = false; + } + } + for (auto *group : e->textGroups()) { + for (DynamicElementTextItem *deti : group->texts()) { + if (deti->textFrom() != DynamicElementTextItem::UserText) { + deti->m_block_alignment = true; + deti->setPlainText(QString()); + deti->m_block_alignment = false; + } + } + } + } + if (settings.value("diagramcommands/erase-label-on-copy", true).toBool()) { //Reset the information about the label, the comment and location @@ -99,27 +156,21 @@ void PasteDiagramCommand::redo() dc.addValue("comment", ""); dc.addValue("location", ""); - // PLC slaves store master data (type, address, comment, - // cross-ref, etc.) in their own elementInformations. - // Remove them the same way MasterElement::unlinkElement() - // does, so pasted PLC slaves start clean like regular - // slaves. - if (e->linkType() == Element::Slave) { - dc.remove(QETInformation::ELMT_PLC_TYPE); - dc.remove(QETInformation::ELMT_PLC_ADDRESS); - dc.remove(QETInformation::ELMT_PLC_FUNCTION); - dc.remove(QETInformation::ELMT_PLC_COMMENT); - dc.remove(QETInformation::ELMT_PLC_CROSSREF); - dc.remove(QETInformation::ELMT_LABEL); - dc.remove(QETInformation::ELMT_PLC_TC); - dc.remove(QETInformation::ELMT_PLC_T1); - dc.remove(QETInformation::ELMT_PLC_T2); - dc.remove(QETInformation::ELMT_PLC_T3); - dc.remove(QETInformation::ELMT_PLC_T4); - dc.remove(QStringLiteral("xref")); + // Block alignment during setElementInformations + // for non-slaves, same as Element::fromXml() (line 890-896). + if (!is_slave) { + for (DynamicElementTextItem *deti : e->dynamicTextItems()) + deti->m_block_alignment = true; + for (auto *group : e->textGroups()) + group->blockAlignmentUpdate(true); } e->setElementInformations(dc); + + for (DynamicElementTextItem *deti : e->dynamicTextItems()) + deti->m_block_alignment = false; + for (auto *group : e->textGroups()) + group->blockAlignmentUpdate(false); //Reset the text of conductors, the same way the label/comment/ //location above are reset to "" rather than to some other diff --git a/sources/diagramevent/diagrameventaddpaste.cpp b/sources/diagramevent/diagrameventaddpaste.cpp index ccc105192..44fe49e24 100644 --- a/sources/diagramevent/diagrameventaddpaste.cpp +++ b/sources/diagramevent/diagrameventaddpaste.cpp @@ -23,6 +23,8 @@ #include "../qetdiagrameditor.h" #include "../qetgraphicsitem/conductor.h" +#include + #include #include #include @@ -35,7 +37,7 @@ @param start_pos : where the pasted items first appear, in scene coordinates -- normally the cursor */ -DiagramEventAddPaste::DiagramEventAddPaste(Diagram *diagram, const QPointF &start_pos) : + DiagramEventAddPaste::DiagramEventAddPaste(Diagram *diagram, const QPointF &start_pos) : DiagramEventInterface(diagram) { //DiagramEventInterface::init() is called by Diagram::setEventInterface @@ -49,20 +51,37 @@ DiagramEventAddPaste::DiagramEventAddPaste(Diagram *diagram, const QPointF &star QDomDocument document_xml; if (!document_xml.setContent(clipboard_text)) return; - m_diagram->fromXml(document_xml, Diagram::snapToGrid(start_pos), false, &m_content); + //Load items at their original XML coordinates. + m_diagram->fromXml(document_xml, QPointF(), false, &m_content); if (!m_content.count()) return; - //Remember where each item sits relative to the group's top left, so a - //move is one assignment per item rather than an accumulated delta. - QRectF group_rect; const QList movable = m_content.items(MovableItems); + if (movable.isEmpty()) return; + + //Compute the bounding rect centre — this is where the cursor + //will start. Items stay at their original XML positions; + //moveTo() handles grid-snapped movement via deltas. + QRectF group_rect; for (auto *item : movable) { group_rect = group_rect.united(item->mapToScene(item->boundingRect()).boundingRect()); } - const QPointF top_left = group_rect.topLeft(); + + QSettings settings; + const int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"), + Diagram::xGrid).toInt(); + const int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"), + Diagram::yGrid).toInt(); + const QPointF grid_origin( + qRound(group_rect.topLeft().x() / xGrid) * xGrid, + qRound(group_rect.topLeft().y() / yGrid) * yGrid); + + //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. for (auto *item : movable) { - m_relative_pos.insert(item, item->pos() - top_left); + m_relative_pos.insert(item, item->pos()); } + m_group_origin = grid_origin; m_diagram->clearSelection(); for (auto *item : movable) { @@ -70,8 +89,15 @@ DiagramEventAddPaste::DiagramEventAddPaste(Diagram *diagram, const QPointF &star } if (!m_diagram->views().isEmpty()) { - if (const auto qde = QETApp::diagramEditorAncestorOf(m_diagram->views().at(0))) { - m_status_bar = qde->statusBar(); + if (auto *view = m_diagram->views().at(0)) { + 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(); @@ -131,19 +157,48 @@ void DiagramEventAddPaste::showHint() /** @brief DiagramEventAddPaste::moveTo - Put the group's top left corner at @a scene_pos, snapped to the grid. + 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. */ void DiagramEventAddPaste::moveTo(const QPointF &scene_pos) { - const QPointF anchor = Diagram::snapToGrid(scene_pos); + QSettings settings; + const int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"), + Diagram::xGrid).toInt(); + const int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"), + Diagram::yGrid).toInt(); + + const auto snapGrid = [xGrid, yGrid](const QPointF &p) -> QPointF { + return QPointF( + qRound(p.x() / xGrid) * xGrid, + 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()) { + m_initial_cursor = snapGrid(scene_pos); + return; + } + + const QPointF delta = snapGrid(scene_pos) - m_initial_cursor; + for (auto it = m_relative_pos.constBegin() ; it != m_relative_pos.constEnd() ; ++it) { if (it.key()) { - it.key()->setPos(anchor + it.value()); + it.key()->setPos(it.value() + delta); } } - const auto conductors = m_content.conductors(); // AnyConductor by default - for (auto *cond : conductors) { - cond->updatePath(); + + //Update conductor paths so they follow the moved terminals. + const QList conductors = m_content.conductors(DiagramContent::AnyConductor); + for (auto *conductor : conductors) { + conductor->updatePath(); } } @@ -168,7 +223,6 @@ void DiagramEventAddPaste::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) event->setAccepted(true); if (event->button() == Qt::LeftButton) { - moveTo(event->scenePos()); commit(); } else if (event->button() == Qt::RightButton) { cancel(); diff --git a/sources/diagramevent/diagrameventaddpaste.h b/sources/diagramevent/diagrameventaddpaste.h index 8c7bcf86f..28ae3854b 100644 --- a/sources/diagramevent/diagrameventaddpaste.h +++ b/sources/diagramevent/diagrameventaddpaste.h @@ -75,12 +75,20 @@ class DiagramEventAddPaste : public DiagramEventInterface void showHint(); void removeItems(); - DiagramContent m_content; - ///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; - QPointer m_status_bar; - bool m_finished{false}; + DiagramContent m_content; + ///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. + 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. + QPointF m_initial_cursor; + ///Set to true once the first moveTo() captures the real cursor position. + bool m_baseline_captured{false}; + QPointer m_status_bar; + bool m_finished{false}; }; #endif // DIAGRAMEVENTADDPASTE_H