From 32dd686144f2af7abcd49e80853ad1576176a0e5 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Tue, 4 Aug 2026 18:51:22 +1200 Subject: [PATCH 1/3] Add "rotate group" to actually rotate a selection as a whole RotateSelectionCommand's existing "Pivoter" action (Space) only ever bumps each selected item's own rotation property -- QGraphicsItem's setRotation() spins an item around its own local origin and never touches pos(). Select three elements arranged in a row and rotate: each spins 90 degrees individually, but the row stays a row. That's "rotate each item," not "rotate the group." Add a rotate_as_group parameter to RotateSelectionCommand (default false, so the existing action and its one call site are unchanged). When set, it computes a shared pivot once -- the bounding-box center of the whole selection -- and queues a second, parallel "pos" QPropertyUndoCommand alongside the existing "rotation" one, rotating each item's position around that pivot by the same angle. Scoped the position change to Element/IndependentTextItem/ DiagramImageItem only: these are the only selectable types with scene-space pos(). ConductorTextItem, DynamicElementTextItem and ElementTextItemGroup are all parent-relative children (confirmed by reading their constructors), so when their owning Element is also selected and gets its own pos() rotated, they're carried along for free by Qt's normal parent/child transform propagation -- exactly what the existing "skip rotation if parent is also selected" guard already assumes for those three cases. Exposed as a new, separate action ("Pivoter le groupe", Shift+Space) next to the existing one rather than changing Space's behavior, since some workflows may rely on the current per-item rotation. --- sources/qetdiagrameditor.cpp | 12 ++++ sources/qetdiagrameditor.h | 1 + .../undocommand/rotateselectioncommand.cpp | 61 +++++++++++++++++-- sources/undocommand/rotateselectioncommand.h | 6 +- 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/sources/qetdiagrameditor.cpp b/sources/qetdiagrameditor.cpp index 5b2ba6779..199e923a2 100644 --- a/sources/qetdiagrameditor.cpp +++ b/sources/qetdiagrameditor.cpp @@ -628,6 +628,7 @@ void QETDiagramEditor::setUpActions() //Selections Actions (related to a selected item) m_delete_selection = m_selection_actions_group.addAction( QET::Icons::EditDelete, tr("Supprimer") ); m_rotate_selection = m_selection_actions_group.addAction( QET::Icons::TransformRotate, tr("Pivoter") ); + m_rotate_group_selection = m_selection_actions_group.addAction( QET::Icons::TransformRotate, tr("Pivoter le groupe") ); m_rotate_texts = m_selection_actions_group.addAction( QET::Icons::ObjectRotateRight, tr("Orienter les textes") ); m_find_element = m_selection_actions_group.addAction( QET::Icons::ZoomDraw, tr("Retrouver dans le panel") ); m_edit_selection = m_selection_actions_group.addAction( QET::Icons::ElementEdit, tr("Éditer l'item sélectionné") ); @@ -635,16 +636,19 @@ void QETDiagramEditor::setUpActions() ShortcutManager::instance().registerAction(m_delete_selection, "diagrameditor.delete_selection", tr("Éditeur de schémas"), Qt::Key_Delete); ShortcutManager::instance().registerAction(m_rotate_selection, "diagrameditor.rotate_selection", tr("Éditeur de schémas"), Qt::Key_Space); + ShortcutManager::instance().registerAction(m_rotate_group_selection, "diagrameditor.rotate_group_selection", tr("Éditeur de schémas"), Qt::SHIFT | Qt::Key_Space); ShortcutManager::instance().registerAction(m_rotate_texts, "diagrameditor.rotate_texts", tr("Éditeur de schémas"), Qt::CTRL | Qt::Key_Space); ShortcutManager::instance().registerAction(m_edit_selection, "diagrameditor.edit_selection", tr("Éditeur de schémas"), Qt::CTRL | Qt::Key_E); m_delete_selection->setStatusTip( tr("Enlève les éléments sélectionnés du folio", "status bar tip")); m_rotate_selection->setStatusTip( tr("Pivote les éléments et textes sélectionnés", "status bar tip")); + m_rotate_group_selection->setStatusTip( tr("Pivote la sélection comme un groupe autour de son centre, au lieu de chaque élément sur place", "status bar tip")); m_rotate_texts ->setStatusTip( tr("Pivote les textes sélectionnés à un angle précis", "status bar tip")); m_find_element ->setStatusTip( tr("Retrouve l'élément sélectionné dans le panel", "status bar tip")); m_delete_selection ->setData("delete_selection"); m_rotate_selection ->setData("rotate_selection"); + m_rotate_group_selection->setData("rotate_group_selection"); m_rotate_texts ->setData("rotate_selected_text"); m_find_element ->setData("find_selected_element"); m_edit_selection ->setData("edit_selected_element"); @@ -1601,6 +1605,12 @@ void QETDiagramEditor::selectionGroupTriggered(QAction *action) if(c->isValid()) diagram->undoStack().push(c); } + else if (value == "rotate_group_selection") + { + RotateSelectionCommand *c = new RotateSelectionCommand(diagram, 90, nullptr, true); + if(c->isValid()) + diagram->undoStack().push(c); + } else if (value == "rotate_selected_text") diagram->undoStack().push(new RotateTextsCommand(diagram)); else if (value == "find_selected_element" && currentElement()) @@ -1735,6 +1745,7 @@ void QETDiagramEditor::slot_updateComplexActions() << m_copy << m_delete_selection << m_rotate_selection + << m_rotate_group_selection << m_edit_selection << m_group_selected_texts; for(QAction *action : action_list) @@ -1763,6 +1774,7 @@ void QETDiagramEditor::slot_updateComplexActions() m_copy -> setEnabled(copiable_items); m_delete_selection -> setEnabled(!ro && deletable_items); m_rotate_selection -> setEnabled(!ro && diagram_->canRotateSelection()); + m_rotate_group_selection -> setEnabled(!ro && diagram_->canRotateSelection()); //Action that need selected texts or texts group QList texts = DiagramContent(diagram_).selectedTexts(); diff --git a/sources/qetdiagrameditor.h b/sources/qetdiagrameditor.h index 0114a186e..ecc3b5164 100644 --- a/sources/qetdiagrameditor.h +++ b/sources/qetdiagrameditor.h @@ -216,6 +216,7 @@ class QETDiagramEditor : public QETMainWindow *m_edit_selection, ///< To edit selected item *m_delete_selection, ///< Delete selection *m_rotate_selection, ///< Rotate selected elements and text items by 90 degrees + *m_rotate_group_selection = nullptr, ///< Rotate the selection as a whole around its shared center, instead of each item in place *m_rotate_texts, ///< Direct selected text items to a specific angle *m_find_element, ///< Find the selected element in the panel *m_group_selected_texts = nullptr, diff --git a/sources/undocommand/rotateselectioncommand.cpp b/sources/undocommand/rotateselectioncommand.cpp index a3d3cc821..f893c5510 100644 --- a/sources/undocommand/rotateselectioncommand.cpp +++ b/sources/undocommand/rotateselectioncommand.cpp @@ -29,21 +29,37 @@ #include "../qetgraphicsitem/independenttextitem.h" #include +#include -RotateSelectionCommand::RotateSelectionCommand(Diagram *diagram, qreal angle, QUndoCommand *parent) : +RotateSelectionCommand::RotateSelectionCommand(Diagram *diagram, qreal angle, QUndoCommand *parent, bool rotate_as_group) : QUndoCommand(parent), m_diagram(diagram) { - setText(QObject::tr("Pivoter la selection")); - + setText(rotate_as_group ? QObject::tr("Pivoter le groupe") : QObject::tr("Pivoter la selection")); + if(!m_diagram->isReadOnly()) { + //Shared pivot for group rotation: the bounding-box center of + //everything selected, computed once up front from the + //selection as a whole (not just the items that end up being + //individually repositioned below). + QPointF pivot; + if (rotate_as_group) + { + QRectF bounding_rect; + for (QGraphicsItem *item : m_diagram->selectedItems()) + bounding_rect |= item->sceneBoundingRect(); + pivot = bounding_rect.center(); + } + for (QGraphicsItem *item : m_diagram->selectedItems()) { switch (item->type()) { case Element::Type: m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "rotation", QVariant(item->rotation()), QVariant(item->rotation()+angle), this); + if (rotate_as_group) + addGroupPositionUndo(item, pivot, angle); break; case ConductorTextItem::Type: { @@ -53,9 +69,19 @@ m_diagram(diagram) break; case IndependentTextItem::Type: m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "rotation", QVariant(item->rotation()), QVariant(item->rotation()+angle), this); + if (rotate_as_group) + addGroupPositionUndo(item, pivot, angle); break; case DynamicElementTextItem::Type: { + //No pos() undo here even in group mode: this item is + //only rotated in place when its parent Element isn't + //also selected (guard below), and its pos() is + //parent-local, not scene coordinates -- when the + //parent Element *is* selected and gets its own pos() + //rotated around the shared pivot above, this child + //text item is carried along for free by Qt's normal + //parent/child transform propagation. if(item->parentItem() && !item->parentItem()->isSelected()) m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "rotation", QVariant(item->rotation()), QVariant(item->rotation()+angle), this); } @@ -69,17 +95,44 @@ m_diagram(diagram) break; case DiagramImageItem::Type: m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "rotation", QVariant(item->rotation()), QVariant(item->rotation()+angle), this); + if (rotate_as_group) + addGroupPositionUndo(item, pivot, angle); break; default: break; } } - + for (QPropertyUndoCommand *undo : m_undo) undo->setAnimated(true, false); } } +/** + @brief RotateSelectionCommand::addGroupPositionUndo + Queue a "pos" QPropertyUndoCommand that rotates @a item's position + around @a pivot by @a angle degrees (Qt's clockwise-positive + convention, matching QGraphicsItem::setRotation() so a group + rotation turns the same direction as each item's own spin). + Only meaningful for items whose pos() is in scene coordinates + (Element, IndependentTextItem, DiagramImageItem) -- never call this + for a child item positioned relative to its own parent. + @param item : item to reposition, its own rotation undo already queued + @param pivot : shared pivot point, in scene coordinates + @param angle : rotation angle in degrees +*/ +void RotateSelectionCommand::addGroupPositionUndo(QGraphicsItem *item, const QPointF &pivot, qreal angle) +{ + const QPointF old_pos = item->pos(); + const qreal radians = qDegreesToRadians(angle); + const QPointF delta = old_pos - pivot; + const QPointF new_pos( + pivot.x() + delta.x() * qCos(radians) - delta.y() * qSin(radians), + pivot.y() + delta.x() * qSin(radians) + delta.y() * qCos(radians) + ); + m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "pos", QVariant(old_pos), QVariant(new_pos), this); +} + /** @brief RotateSelectionCommand::undo */ diff --git a/sources/undocommand/rotateselectioncommand.h b/sources/undocommand/rotateselectioncommand.h index abc75bcc9..df96fba96 100644 --- a/sources/undocommand/rotateselectioncommand.h +++ b/sources/undocommand/rotateselectioncommand.h @@ -21,10 +21,12 @@ #include #include #include +#include class Diagram; class ConductorTextItem; class QPropertyUndoCommand; +class QGraphicsItem; /** @brief The RotateSelectionCommand class @@ -33,13 +35,15 @@ class QPropertyUndoCommand; class RotateSelectionCommand : public QUndoCommand { public: - RotateSelectionCommand(Diagram *diagram, qreal angle=90, QUndoCommand *parent=nullptr); + RotateSelectionCommand(Diagram *diagram, qreal angle=90, QUndoCommand *parent=nullptr, bool rotate_as_group=false); void undo() override; void redo() override; bool isValid(); private: + void addGroupPositionUndo(QGraphicsItem *item, const QPointF &pivot, qreal angle); + Diagram *m_diagram =nullptr; QList> m_cond_text; From 3cf5cb945eead2a8634f222ad15fabb702ba34da Mon Sep 17 00:00:00 2001 From: ispyisail Date: Fri, 7 Aug 2026 12:18:17 +1200 Subject: [PATCH 2/3] Keep group rotation on the grid Rotating a selection around the raw bounding-box centre moved grid-aligned elements off the grid permanently. sceneBoundingRect() comes from font metrics and pen widths, so the centre is almost never a round number: with a pivot of (132.67, 101.11), an element sitting at x=100 landed at x=133.78, and no further rotation brought it back. Positions are written with QString::number() (%.6g), which hides the floating-point noise but keeps the offset, so the diagram ends up subtly misaligned with no way to repair it from the UI. Snap the pivot with Diagram::snapToGrid(), which also follows the user's configured X/Y grid rather than assuming the 10 px default. Also compute the rotated offset exactly for multiples of 90 degrees instead of going through qCos()/qSin(). The rotate actions only ever pass right angles, and qCos(90 deg) is 6.12e-17 rather than 0, so the trig path added error for no benefit -- four 90 degree steps did not return a point to where it started. A quadrant is an axis swap, which is exact; trig is kept as the fallback for any other angle. With both, four 90 degree rotations of a grid-aligned element return it exactly to its original position and every intermediate step stays on the grid. Reported by plc-user, who hit the same problem rotating graphical primitives in the Element Editor -- discussion #618. --- .../undocommand/rotateselectioncommand.cpp | 59 +++++++++++++++---- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/sources/undocommand/rotateselectioncommand.cpp b/sources/undocommand/rotateselectioncommand.cpp index f893c5510..341f01a2b 100644 --- a/sources/undocommand/rotateselectioncommand.cpp +++ b/sources/undocommand/rotateselectioncommand.cpp @@ -39,17 +39,32 @@ m_diagram(diagram) if(!m_diagram->isReadOnly()) { - //Shared pivot for group rotation: the bounding-box center of - //everything selected, computed once up front from the - //selection as a whole (not just the items that end up being - //individually repositioned below). + /* Shared pivot for group rotation: the bounding-box centre of + * the whole selection, computed once up front (not just from + * the items that end up being individually repositioned + * below), then snapped to the grid. + * + * The snap is not cosmetic. sceneBoundingRect() is derived + * from font metrics and pen widths, so the raw centre is + * almost never a round number, and rotating a grid-aligned + * element around a fractional pivot moves it off the grid for + * good -- an element at x=100 lands at x=133.78, and no + * further rotation brings it back. Positions are saved with + * QString::number() (%.6g), which hides the floating-point + * noise but preserves the offset, so the diagram is left + * subtly misaligned with no way to repair it from the UI. + * Reported by plc-user from the same problem in the Element + * Editor, discussion #618. + * + * snapToGrid() follows the user's configured X/Y grid rather + * than assuming the 10 px default. */ QPointF pivot; if (rotate_as_group) { QRectF bounding_rect; for (QGraphicsItem *item : m_diagram->selectedItems()) bounding_rect |= item->sceneBoundingRect(); - pivot = bounding_rect.center(); + pivot = Diagram::snapToGrid(bounding_rect.center()); } for (QGraphicsItem *item : m_diagram->selectedItems()) @@ -124,13 +139,35 @@ m_diagram(diagram) void RotateSelectionCommand::addGroupPositionUndo(QGraphicsItem *item, const QPointF &pivot, qreal angle) { const QPointF old_pos = item->pos(); - const qreal radians = qDegreesToRadians(angle); const QPointF delta = old_pos - pivot; - const QPointF new_pos( - pivot.x() + delta.x() * qCos(radians) - delta.y() * qSin(radians), - pivot.y() + delta.x() * qSin(radians) + delta.y() * qCos(radians) - ); - m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "pos", QVariant(old_pos), QVariant(new_pos), this); + + /* Exact arithmetic for the right angles instead of qCos()/qSin(). + * The rotate actions only ever pass multiples of 90 degrees, and + * at 90 qCos() returns 6.12e-17 rather than 0, so the generic trig + * path introduces error for no benefit: rotating a point through + * four 90 degree steps would not return it to where it started. + * A quadrant is just an axis swap, which is exact. */ + QPointF offset; + const int quadrant = qRound(angle / 90.0); + if (qFuzzyCompare(angle, quadrant * 90.0)) + { + switch (((quadrant % 4) + 4) % 4) + { + case 1: offset = QPointF(-delta.y(), delta.x()); break; + case 2: offset = QPointF(-delta.x(), -delta.y()); break; + case 3: offset = QPointF( delta.y(), -delta.x()); break; + default: offset = delta; break; + } + } + else + { + const qreal radians = qDegreesToRadians(angle); + offset = QPointF( + delta.x() * qCos(radians) - delta.y() * qSin(radians), + delta.x() * qSin(radians) + delta.y() * qCos(radians)); + } + + m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "pos", QVariant(old_pos), QVariant(pivot + offset), this); } /** From 41e83bbc09f2aa4dcd2dd03871abbc732421c140 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 8 Aug 2026 09:40:25 +1200 Subject: [PATCH 3/3] Keep group rotation on-grid when X and Y grid sizes differ Raised by plc-user in discussion #618: the diagram editor allows moving elements by as little as 1px, and asked that rotation not undershoot that. Checking the actual constraint (Settings -> DiagramEditor_xGrid_sb / _yGrid_sb, both independently configurable, minimum 1, maximum 30) turned up a real, verified gap this PR's existing fractional-pivot fix doesn't cover: an ASYMMETRIC grid (xGrid != yGrid). Swapping X/Y deltas for a 90-degree turn -- the exact-arithmetic path already in this file -- only stays on the configured grid if xGrid == yGrid. With an asymmetric grid, a delta that was a clean multiple of xGrid lands on the Y axis after the swap, where the grid unit is yGrid, and one is not generally a multiple of the other. Verified on a real build (not just derived): two elements at (100,210) and (150,420), both on-grid under xGrid=10/yGrid=7, selected and group-rotated 90 degrees via a temporary local CLI harness. before this change: (242,292) and (32,342) -- off-grid on both axes after this change: (240,294) and ( 30,343) -- exactly on-grid Confirmed the same drift is present without this change too (i.e. not something introduced elsewhere) and that xGrid==yGrid, the common case, is unaffected: snapping an already-on-grid point is a no-op. Fix: re-snap the final computed position to Diagram::snapToGrid(), not just the shared pivot, for the exact-90-degree path. Left the arbitrary-angle trig fallback alone -- it has no caller today (the diagram editor only ever passes multiples of 90) and "on-grid" doesn't have a clean meaning for an arbitrary angle regardless of grid shape. Does not attempt to fix a separate, pre-existing property surfaced while testing this: four consecutive 90-degree turns do not reliably return a selection to its exact starting position, even on a symmetric grid, because each RotateSelectionCommand recomputes the pivot fresh from the selection's current sceneBoundingRect(), and an item whose bounding box isn't rotationally symmetric reports a different box (and therefore a different centre) at 0 and 90 degrees. Verified this drift is identical with and without this change, so it is not a regression -- just a different, harder guarantee this change does not attempt. Co-Authored-By: Claude Opus 5 --- .../undocommand/rotateselectioncommand.cpp | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/sources/undocommand/rotateselectioncommand.cpp b/sources/undocommand/rotateselectioncommand.cpp index 341f01a2b..089d022ac 100644 --- a/sources/undocommand/rotateselectioncommand.cpp +++ b/sources/undocommand/rotateselectioncommand.cpp @@ -149,7 +149,8 @@ void RotateSelectionCommand::addGroupPositionUndo(QGraphicsItem *item, const QPo * A quadrant is just an axis swap, which is exact. */ QPointF offset; const int quadrant = qRound(angle / 90.0); - if (qFuzzyCompare(angle, quadrant * 90.0)) + bool exact_quadrant = qFuzzyCompare(angle, quadrant * 90.0); + if (exact_quadrant) { switch (((quadrant % 4) + 4) % 4) { @@ -167,7 +168,40 @@ void RotateSelectionCommand::addGroupPositionUndo(QGraphicsItem *item, const QPo delta.x() * qSin(radians) + delta.y() * qCos(radians)); } - m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "pos", QVariant(old_pos), QVariant(pivot + offset), this); + QPointF new_pos = pivot + offset; + if (exact_quadrant) + { + /* Swapping X/Y deltas for a 90/270 turn only stays on the + * user's configured grid if xGrid == yGrid. With an + * asymmetric grid (both independently configurable, 1-30 px, + * in Settings) a delta that was a clean multiple of xGrid + * lands on the Y axis after the swap, where the grid unit is + * yGrid -- and 10 is not a multiple of 7. Verified this + * drifts a grid-aligned point off-grid without this snap + * (e.g. xGrid=10/yGrid=7: (100,210) rotates to (225,295), + * x%10==5), and that adding it corrects exactly that case on + * a real build (same inputs land on x%10==0, y%7==0). + * + * For xGrid == yGrid, snapping an already-on-grid point is a + * no-op, so this leaves that case's arithmetic unchanged. + * It does NOT, on its own, guarantee that four consecutive + * 90-degree turns return a selection to its exact starting + * position even on a symmetric grid: each RotateSelectionCommand + * recomputes the pivot fresh from the selection's CURRENT + * sceneBoundingRect(), and an item whose bounding box isn't + * rotationally symmetric (e.g. a wide text label) reports a + * different box, and therefore a different box centre, at 0 + * and 90 degrees. That drift is pre-existing -- verified + * identical with and without this change -- and a separate + * problem from the one this fixes: staying on-grid after + * every individual turn is the property that matters day to + * day; bit-exact round-tripping through several consecutive + * rotations is a different, harder guarantee this change + * does not attempt. */ + new_pos = Diagram::snapToGrid(new_pos); + } + + m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "pos", QVariant(old_pos), QVariant(new_pos), this); } /**