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>
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.
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.
Placing an auto-numbered element or conductor advances a shared
NumerotationContext counter (QETProject::addConductorAutoNum/
addElementAutoNum) as a side effect that sat entirely outside the undo
stack. Undoing the placement removed the visible number but left the
counter advanced, so every undo of an auto-numbered placement silently
burned a number, with no way to get it back short of a manual reset.
Adds SetAutoNumContextCommand, a small QUndoCommand storing the old/new
NumerotationContext and calling the matching add*AutoNum() setter on
undo()/redo() -- the same shape QPropertyUndoCommand already uses next
to it in ConductorAutoNumerotation::applyText().
Wires it into the two conductor call sites (the static newProperties(),
and numerateNewConductor(), both in ConductorAutoNumerotation) and the
element call site (Element::setUpFormula(), called from
DiagramEventAddElement::addElement() when a new element is dropped onto
a diagram). setUpFormula() now takes an optional parent QUndoCommand;
addElement() calls it before pushing its own undo_object so the counter
change lands in the same undo macro as the element's placement -- one
Ctrl+Z reverts both together, instead of leaving the counter adrift.
The project-properties config dialog's own add*AutoNum() calls (editing
the numbering rule itself, not a side effect of placing something) are
deliberately left untouched, as are the load-time folio-sequential
bookkeeping calls in Diagram::loadElmtFolioSeq()/loadCndFolioSeq() and
the bulk folio-renumbering passes in QETProject -- none of those run as
part of an undoable user gesture.
Implements the scope proposed in discussion #608.
Three new QUndoCommand subclasses (AddDiagramCommand, RemoveDiagramCommand,
MoveDiagramCommand) pushed onto the project's existing (already
project-scoped) undo stack, so folio structure edits are undoable
alongside every item-level edit already on that stack.
- QETProject::addDiagram()/detachDiagram() are the shared attach/detach
primitives: they mutate the diagram list, connect/disconnect the two
per-diagram signals set up at add time, and emit diagramAdded/
diagramRemoved. AddDiagramCommand and RemoveDiagramCommand call these
(via friend access) for both redo and undo, so a removed diagram is
parked rather than destroyed -- it's only actually deleted if the
command itself falls out of undo history while still detached.
- ProjectView reacts to diagramRemoved the same way it already reacted to
diagramAdded (tearing down/rebuilding the tab), so both directions of
both commands go through the same reactive path every other diagram
listener (project database, cross-references, generic panel) already
relies on.
- MoveDiagramCommand wraps a new ProjectView::setDiagramPosition(), which
performs the tab move and the project's diagramOrderChanged() list
reorder synchronously in one step, instead of relying on the queued
tabMoved connection (needed for interactive drag-and-drop) to catch up
later -- avoiding a second, redundant reorder from that queued call.
- Multi-folio delete and multi-folio move (QETDiagramEditor::removeDiagrams()
and the moveDiagram*(QList<Diagram*>) batch slots) wrap their per-diagram
loop in QUndoStack::beginMacro()/endMacro(), so a multi-select action is
one undo step, matching current UX.
- Softened the delete confirmation's "this change is irreversible" wording
now that it no longer is.
Verified headlessly (Xvfb + xdotool + scrot): add/undo/redo, delete/undo/
redo (single and multi-select, single undo step for the batch), and
move/undo/redo all behave correctly against a 7-folio project.
QHash/QMap::keys() allocates a list of every key on each call, then
contains() searches it linearly - an accidental O(n) plus allocation
where a direct O(1) lookup was meant. 35 occurrences across 7 files,
found while profiling project load times (context: #553/#560).
The hot one is ElementPictureFactory::getPictures(), which runs once
per element instance on project load: on the 3399 KiB example project
(191 instances, 129 cache hits) the keys() detour cost 45 ms of the
1.34 s total - measured, not estimated; the fix reproducibly shaves
~35-45 ms off that load. The remaining call sites are UI paths
(search&replace, dynamic text model, undo commands) where the waste
scales with selection/model size.
No behavior change: for QHash/QMap, keys().contains(k) and
contains(k) are equivalent by definition.
(cherry picked from commit 0a7f8f072fa68de7c01a9fc134a4bc8e16d62062)
qAsConst was deprecated in Qt 6.6; std::as_const (C++17, already the
project standard) is the drop-in replacement. Clears 46 -Wdeprecated-
declarations warnings across 18 files. No behavioural change.
For conductors, the setPos() function can result in negative
coordinates.
For unknown reasons, this can lead to an offset in the scene coordinate
system, resulting in a free space above and to the left of the drawing
frame. This free space could not be removed.
It is better to set the conductors using the conductor::updatePath()
function. If the conductor text has been moved by the user, the new
position of the text must be calculated.
It is important to position the elements first and then 'connect' the
conductors.
Setting the conductor position via setPos() was done in elemntsmover.cpp
(corrected here) and in Diagram::fromXML (corrected in the commit
'Better handling of conductors when creating from XML').
In case of user try to delete a terminal element who is bridged
or belong to a physical terminal with more than one level, the deletion
is aborted to avoid mistake in the terminal strip parent of the terminal
element. A dialog is opened when the deletion can't be to explain to
user what to do for enable the deletion.
clazy is a compiler plugin which allows clang to understand Qt
semantics. You get more than 50 Qt related compiler warnings, ranging
from unneeded memory allocations to misusage of API, including fix-its
for automatic refactoring.
https://invent.kde.org/sdk/clazy
* terminal_strip:
Terminal strip item can saved / loaded to .qet file
See previous commit...
Move terminal strip drawer class in is own file
Fix wrong use of QStringLiteral and QLatin1String
Double click a TerminalStripItem open the editor
Minor change about checkable QAction of QetDiagramEditor
Minor : corrects a minor aesthetic defect when unbridge terminals
Revamp code
Add and move terminal strip item are now managed by undo command
TerminalStripItem : Draw terminal bridge
Terminal strip item can be added to diagram
Minor : add QGIUtility namespace
-Move MoveElementsCommand class from diagramcommands file to
movegraphicsitemcommand file.
-Rename the to class MoveGraphicsItemCommand.
-Minor code change to make it more modern.
Andre Rummler