mirror of
https://github.com/qelectrotech/qelectrotech-source-mirror.git
synced 2026-08-13 10:04:13 +02:00
41e83bbc09
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 <noreply@anthropic.com>
249 lines
9.4 KiB
C++
249 lines
9.4 KiB
C++
/*
|
|
Copyright 2006-2026 The QElectroTech Team
|
|
This file is part of QElectroTech.
|
|
|
|
QElectroTech is free software: you can redistribute it and/or modify
|
|
it under the terms of the GNU General Public License as published by
|
|
the Free Software Foundation, either version 2 of the License, or
|
|
(at your option) any later version.
|
|
|
|
QElectroTech is distributed in the hope that it will be useful,
|
|
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
GNU General Public License for more details.
|
|
|
|
You should have received a copy of the GNU General Public License
|
|
along with QElectroTech. If not, see <http://www.gnu.org/licenses/>.
|
|
*/
|
|
#include "rotateselectioncommand.h"
|
|
|
|
#include "../QPropertyUndoCommand/qpropertyundocommand.h"
|
|
#include "../diagram.h"
|
|
#include "../qet.h"
|
|
#include "../qetgraphicsitem/conductor.h"
|
|
#include "../qetgraphicsitem/conductortextitem.h"
|
|
#include "../qetgraphicsitem/diagramimageitem.h"
|
|
#include "../qetgraphicsitem/dynamicelementtextitem.h"
|
|
#include "../qetgraphicsitem/element.h"
|
|
#include "../qetgraphicsitem/elementtextitemgroup.h"
|
|
#include "../qetgraphicsitem/independenttextitem.h"
|
|
|
|
#include <QGraphicsItem>
|
|
#include <QtMath>
|
|
|
|
RotateSelectionCommand::RotateSelectionCommand(Diagram *diagram, qreal angle, QUndoCommand *parent, bool rotate_as_group) :
|
|
QUndoCommand(parent),
|
|
m_diagram(diagram)
|
|
{
|
|
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:
|
|
{
|
|
m_cond_text << static_cast<ConductorTextItem *>(item);
|
|
m_undo << new QPropertyUndoCommand(item->toGraphicsObject(), "rotation", QVariant(item->rotation()), QVariant(item->rotation()+angle), this);
|
|
}
|
|
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);
|
|
}
|
|
break;
|
|
case QGraphicsItemGroup::Type:
|
|
{
|
|
if(ElementTextItemGroup *grp = dynamic_cast<ElementTextItemGroup *>(item))
|
|
if(grp->parentElement() && !grp->parentElement()->isSelected())
|
|
m_undo << new QPropertyUndoCommand(grp, "rotation", QVariant(item->rotation()), QVariant(item->rotation()+angle), this);
|
|
}
|
|
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
|
|
*/
|
|
void RotateSelectionCommand::undo()
|
|
{
|
|
m_diagram->showMe();
|
|
QUndoCommand::undo();
|
|
|
|
for(const QPointer<ConductorTextItem>& cti : m_cond_text)
|
|
{
|
|
cti->forceRotateByUser(m_rotate_by_user.value(cti.data()));
|
|
if(!cti->wasRotatedByUser())
|
|
cti->parentConductor()->calculateTextItemPosition();
|
|
}
|
|
}
|
|
|
|
/**
|
|
@brief RotateSelectionCommand::redo
|
|
*/
|
|
void RotateSelectionCommand::redo()
|
|
{
|
|
m_diagram->showMe();
|
|
QUndoCommand::redo();
|
|
|
|
for(const QPointer<ConductorTextItem>& cti : m_cond_text)
|
|
{
|
|
m_rotate_by_user.insert(cti, cti->wasRotatedByUser());
|
|
cti->forceRotateByUser(true);
|
|
}
|
|
}
|
|
|
|
/**
|
|
@brief RotateSelectionCommand::isValid
|
|
@return true if this command rotate a least one item.
|
|
*/
|
|
bool RotateSelectionCommand::isValid()
|
|
{
|
|
if(childCount())
|
|
return true;
|
|
else
|
|
return false;
|
|
}
|