Merge pull request #1089 from ispyisail/fix/1069-align-lone-shape

Fix Align centre commands when a group is left with only a shape

sources/alignment.h

@@ -54,6 +54,24 @@ namespace Alignment
 		QPointF ref;
 	};
 
+	/**
+		@return @a items taken as one piece, the way a group lines up:
+		their edges together, and the middle of that box as its centre.
+		A single item keeps its own centre.
+	*/
+	inline Item combined(const QList<Item> &items)
+	{
+		Item result;
+		for (const Item &item : items)
+			result.edges = result.edges.isNull() ? item.edges
+							     : result.edges.united(item.edges);
+		if (items.size() == 1)
+			result.ref = items.first().ref;
+		else
+			result.ref = result.edges.center();
+		return result;
+	}
+
 	/**
 		@return true if aligning on @a edge moves items along x
 	*/

sources/undocommand/alignselectioncommand.cpp

@@ -97,12 +97,14 @@ AlignSelectionCommand::AlignSelectionCommand(Diagram *diagram, Mode mode, QUndoC
 		//group come along; shapes outside one are left out, as above.
 	struct Unit {
 		QList<QGraphicsObject *> members;
-		Alignment::Item geometry;
+		QList<Alignment::Item> parts;
 		QGraphicsObject *snap_item = nullptr; ///< lands on its grid
 		qreal divisor = 1;
 	};
 	QList<Unit> units;
 	QHash<QUuid, int> group_units;
+		//Returns a reference into units, which a later call can grow:
+		//use it before calling again, never keep it
 	auto unitFor = [&](QGraphicsObject *item) -> Unit &
 	{
 		const QUuid group = ItemGroups::groupOf(item);
@@ -120,10 +122,7 @@ AlignSelectionCommand::AlignSelectionCommand(Diagram *diagram, Mode mode, QUndoC
 	{
 		Unit &unit = unitFor(entry.item);
 		unit.members << entry.item;
-		unit.geometry.edges = unit.geometry.edges.isNull()
-				? entry.geometry.edges
-				: unit.geometry.edges.united(entry.geometry.edges);
-		unit.geometry.ref = entry.geometry.ref;
+		unit.parts << entry.geometry;
 			//Symbols come first in entries, so a group with one snaps on it
 		if (!unit.snap_item) {
 			unit.snap_item = entry.item;
@@ -136,16 +135,11 @@ AlignSelectionCommand::AlignSelectionCommand(Diagram *diagram, Mode mode, QUndoC
 			continue;
 		Unit &unit = unitFor(shape);
 		unit.members << shape;
-		unit.geometry.edges = unit.geometry.edges.isNull()
-				? shape->sceneBoundingRect()
-				: unit.geometry.edges.united(shape->sceneBoundingRect());
+		const QRectF rect = shape->sceneBoundingRect();
+		unit.parts << Alignment::Item{rect, rect.center()};
 		if (!unit.snap_item)
 			unit.snap_item = shape;
 	}
-	for (Unit &unit : units) {
-		if (unit.members.size() > 1)
-			unit.geometry.ref = unit.geometry.edges.center();
-	}
 	m_item_count = units.size();
 
 		//Lining up a single item on itself would only snap it
@@ -165,7 +159,7 @@ AlignSelectionCommand::AlignSelectionCommand(Diagram *diagram, Mode mode, QUndoC
 
 	QList<Alignment::Item> geometry;
 	for (const Unit &unit : std::as_const(units))
-		geometry << unit.geometry;
+		geometry << Alignment::combined(unit.parts);
 	const QList<QPointF> offsets = Alignment::alignOffsets(geometry, edge);
 
 	for (int i = 0 ; i < units.size() ; ++i)

tests/qttest/tst_alignment.cpp

@@ -155,6 +155,33 @@ private slots:
 		QCOMPARE(offset.y(), 0.0);
 	}
 
+	// A group lines up on the middle of its members' box; a unit with a
+	// single member, such as a shape whose group-mate is locked, keeps
+	// that member's own centre instead of the origin of the folio.
+	void combinedUnits()
+	{
+		const Alignment::Item a{QRectF(100, 200, 40, 20), QPointF(110, 210)};
+		const Alignment::Item b{QRectF(300, 260, 20, 60), QPointF(310, 270)};
+
+		const Alignment::Item one = Alignment::combined({a});
+		QCOMPARE(one.edges, a.edges);
+		QCOMPARE(one.ref, a.ref);
+
+		const Alignment::Item both = Alignment::combined({a, b});
+		QCOMPARE(both.edges, QRectF(100, 200, 220, 120));
+		QCOMPARE(both.ref, QPointF(210, 260));
+
+		// a lone shape centred on x = 200 and a symbol at x = 400 meet
+		// half way, at 300; with the folio origin as the shape's centre
+		// they would meet at 200 and the shape would move 200 px
+		const QRectF shape(180, 50, 40, 40);
+		const QList<QPointF> offsets = Alignment::alignOffsets(
+			{Alignment::combined({{shape, shape.center()}}), {QRectF(390, 0, 20, 20), QPointF(400, 10)}},
+			Alignment::HCenter);
+		QCOMPARE(offsets.at(0), QPointF(100, 0));
+		QCOMPARE(offsets.at(1), QPointF(-100, 0));
+	}
+
 	void emptySelection()
 	{
 		QVERIFY(Alignment::alignOffsets({}, Alignment::Left).isEmpty());