Merge pull request #660 from ispyisail/feature-rotate-group

Add "rotate group" to actually rotate a selection as a whole

sources/undocommand/rotateselectioncommand.cpp

@@ -29,21 +29,52 @@
 #include "../qetgraphicsitem/independenttextitem.h"
 
 #include <QGraphicsItem>
+#include <QtMath>
 
-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 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 = Diagram::snapToGrid(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 +84,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 +110,100 @@ 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 QPointF delta = old_pos - pivot;
+
+		/* 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);
+	bool exact_quadrant = qFuzzyCompare(angle, quadrant * 90.0);
+	if (exact_quadrant)
+	{
+		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));
+	}
+
+	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);
+}
+
 /**
 	@brief RotateSelectionCommand::undo
 */

sources/undocommand/rotateselectioncommand.h

@@ -21,10 +21,12 @@
 #include <QUndoCommand>
 #include <QPointer>
 #include <QHash>
+#include <QPointF>
 
 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<QPointer<ConductorTextItem>> m_cond_text;