mirror of
https://github.com/qelectrotech/qelectrotech-source-mirror.git
synced 2026-08-13 10:04:13 +02:00
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.
This commit is contained in:
@@ -39,17 +39,32 @@ m_diagram(diagram)
|
|||||||
|
|
||||||
if(!m_diagram->isReadOnly())
|
if(!m_diagram->isReadOnly())
|
||||||
{
|
{
|
||||||
//Shared pivot for group rotation: the bounding-box center of
|
/* Shared pivot for group rotation: the bounding-box centre of
|
||||||
//everything selected, computed once up front from the
|
* the whole selection, computed once up front (not just from
|
||||||
//selection as a whole (not just the items that end up being
|
* the items that end up being individually repositioned
|
||||||
//individually repositioned below).
|
* 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;
|
QPointF pivot;
|
||||||
if (rotate_as_group)
|
if (rotate_as_group)
|
||||||
{
|
{
|
||||||
QRectF bounding_rect;
|
QRectF bounding_rect;
|
||||||
for (QGraphicsItem *item : m_diagram->selectedItems())
|
for (QGraphicsItem *item : m_diagram->selectedItems())
|
||||||
bounding_rect |= item->sceneBoundingRect();
|
bounding_rect |= item->sceneBoundingRect();
|
||||||
pivot = bounding_rect.center();
|
pivot = Diagram::snapToGrid(bounding_rect.center());
|
||||||
}
|
}
|
||||||
|
|
||||||
for (QGraphicsItem *item : m_diagram->selectedItems())
|
for (QGraphicsItem *item : m_diagram->selectedItems())
|
||||||
@@ -124,13 +139,35 @@ m_diagram(diagram)
|
|||||||
void RotateSelectionCommand::addGroupPositionUndo(QGraphicsItem *item, const QPointF &pivot, qreal angle)
|
void RotateSelectionCommand::addGroupPositionUndo(QGraphicsItem *item, const QPointF &pivot, qreal angle)
|
||||||
{
|
{
|
||||||
const QPointF old_pos = item->pos();
|
const QPointF old_pos = item->pos();
|
||||||
const qreal radians = qDegreesToRadians(angle);
|
|
||||||
const QPointF delta = old_pos - pivot;
|
const QPointF delta = old_pos - pivot;
|
||||||
const QPointF new_pos(
|
|
||||||
pivot.x() + delta.x() * qCos(radians) - delta.y() * qSin(radians),
|
/* Exact arithmetic for the right angles instead of qCos()/qSin().
|
||||||
pivot.y() + delta.x() * qSin(radians) + delta.y() * qCos(radians)
|
* 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
|
||||||
m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "pos", QVariant(old_pos), QVariant(new_pos), this);
|
* 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);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user