From 2dbaa6918622245ff8c82d297c8ada81e3dadade Mon Sep 17 00:00:00 2001 From: ispyisail Date: Tue, 29 Sep 2026 23:23:32 +1300 Subject: [PATCH] Fix a group splitting apart when one of its items is locked (#1146) Grouping two symbols and then locking one of them (Lock position in its properties) left the group in a broken state: dragging the unlocked symbol pulled it away while the locked one stayed, and dragging the locked one did nothing. The move simply dropped locked items, so the rest of the group went without them. A group with a locked member now does not move at all, whichever member is dragged, and the status bar says why. This is the rule the item-groups proposal (discussion #1070) set out for this case. The same rule applies to the arrow keys and to the Align commands, which share DiagramContent::removeNonMovableItems(). Also fixed on the way, for a plain selection with a locked symbol: a wire between the locked symbol and one being dragged kept its user-placed text moving with the dragged end. Such a wire is now redrawn only, as a wire to an unselected symbol already is. Checked in the GUI on two symbols joined by a wire (grafcet example), master against this branch, positions read from the saved file: - drag the unlocked member: master moves it 190 px, this branch moves nothing and shows the message - arrow keys on the selected group (3 runs each): master moves the unlocked member, this branch nothing - the same two symbols ungrouped: both move the unlocked one, as before - user-placed wire text: master shifts it 190 px, this branch keeps it ctest: 34/34. Co-Authored-By: Claude Opus 5.5 --- sources/diagramcontent.cpp | 49 ++++++++++++++++++++- sources/diagramcontent.h | 3 ++ sources/elementsmover.cpp | 28 +++++++++++- sources/elementsmover.h | 2 + sources/qetgraphicsitem/diagramtextitem.cpp | 6 +++ sources/qetgraphicsitem/qetgraphicsitem.cpp | 5 +++ 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/sources/diagramcontent.cpp b/sources/diagramcontent.cpp index 99e58e476..fc54e83c9 100644 --- a/sources/diagramcontent.cpp +++ b/sources/diagramcontent.cpp @@ -18,6 +18,7 @@ #include "diagramcontent.h" #include "diagram.h" +#include "itemgroups.h" #include "qetgraphicsitem/ViewItem/qetgraphicstableitem.h" #include "qetgraphicsitem/conductor.h" #include "qetgraphicsitem/conductortextitem.h" @@ -261,7 +262,7 @@ void DiagramContent::clear() */ int DiagramContent::removeNonMovableItems() { - int count_ = 0; + int count_ = removePinnedGroups(); const QList elements_set = m_elements; for(Element *elmt : elements_set) { @@ -287,9 +288,55 @@ int DiagramContent::removeNonMovableItems() } } + //A wire whose two ends no longer both move is redrawn, not moved, + //or its text would be carried off with the end that still moves + const QList conductors_to_move = m_conductors_to_move; + for (Conductor *conductor : conductors_to_move) { + if (!m_elements.contains(conductor->terminal1->parentElement()) || + !m_elements.contains(conductor->terminal2->parentElement())) { + m_conductors_to_move.removeAll(conductor); + if (!m_conductors_to_update.contains(conductor)) + m_conductors_to_update << conductor; + } + } + return count_; } +/** + @brief DiagramContent::removePinnedGroups + A group (#1070) with a locked member does not move at all: moving the + rest would pull the group apart around the member that stays (#1146). + Only a locked member that is in this content pins its group. + Called first by removeNonMovableItems(), while the locked items are + still here to be found. + @return the number of removed items +*/ +int DiagramContent::removePinnedGroups() +{ + QSet pinned; + for (Element *elmt : std::as_const(m_elements)) + if (!elmt->isMovable()) + pinned << ItemGroups::groupOf(elmt); + for (DiagramImageItem *img : std::as_const(m_images)) + if (!img->isMovable()) + pinned << ItemGroups::groupOf(img); + for (QetShapeItem *shape : std::as_const(m_shapes)) + if (!shape->isMovable()) + pinned << ItemGroups::groupOf(shape); + pinned.remove(QUuid()); + if (pinned.isEmpty()) + return 0; + + auto isPinned = [&pinned](const QGraphicsItem *item) { + return pinned.contains(ItemGroups::groupOf(item)); + }; + return int(m_elements.removeIf(isPinned) + + m_images.removeIf(isPinned) + + m_shapes.removeIf(isPinned) + + m_text_fields.removeIf(isPinned)); +} + DiagramContent &DiagramContent::operator+=(const DiagramContent &other) { for(Element *elmt : other.m_elements) diff --git a/sources/diagramcontent.h b/sources/diagramcontent.h index 85f3dc7f2..92d7c1caf 100644 --- a/sources/diagramcontent.h +++ b/sources/diagramcontent.h @@ -100,6 +100,9 @@ class DiagramContent DiagramContent& operator+=(const DiagramContent& other); bool potentialIsManaged(QListconductors); bool hasTextEditing(); + + private: + int removePinnedGroups(); }; QDebug &operator<<(QDebug, DiagramContent &); #endif diff --git a/sources/elementsmover.cpp b/sources/elementsmover.cpp index 28c8c344d..d8127cae9 100644 --- a/sources/elementsmover.cpp +++ b/sources/elementsmover.cpp @@ -20,6 +20,7 @@ #include "autobreakconductor.h" #include "conductorautonumerotation.h" #include "diagram.h" +#include "itemgroups.h" #include "qetgraphicsitem/conductor.h" #include "qetgraphicsitem/conductortextitem.h" #include "qetgraphicsitem/diagramimageitem.h" @@ -61,6 +62,8 @@ bool ElementsMover::isReady() const */ int ElementsMover::beginMovement(Diagram *diagram, QGraphicsItem *driver_item) { + m_driver_held = false; + // They must be no movement in progress if (m_movement_running) return(-1); @@ -86,6 +89,17 @@ int ElementsMover::beginMovement(Diagram *diagram, QGraphicsItem *driver_item) m_moved_content = DiagramContent(diagram); m_moved_content.removeNonMovableItems(); + //A grouped driver left out of the move belongs to a group that one + //locked member holds in place: it stays with the group + m_driver_held = driver_item + && !ItemGroups::groupOf(driver_item).isNull() + && !m_moved_content.items().contains(driver_item); + if (m_driver_held && m_status_bar) { + m_status_bar->showMessage(QObject::tr( + "Ce groupe ne peut pas être déplacé : " + "la position d'un de ses éléments est verrouillée.")); + } + //Remove element text and text group, if the parent element is selected. const auto element_text{m_moved_content.m_element_texts}; for(const auto &deti : element_text) { @@ -109,6 +123,17 @@ int ElementsMover::beginMovement(Diagram *diagram, QGraphicsItem *driver_item) return(m_moved_content.count()); } +/** + @brief ElementsMover::holds + @return true if @a item is the item the user drags and it must not move: + it is in a group that a locked member keeps in place (#1146). Each item + that drives a movement asks before moving itself. +*/ +bool ElementsMover::holds(const QGraphicsItem *item) const +{ + return m_driver_held && item && item == m_movement_driver; +} + /** @brief ElementsMover::continueMovement Add a move to the current movement. @@ -255,7 +280,8 @@ void ElementsMover::endMovement() m_movement_running = false; m_moved_content.clear(); - if (m_status_bar) { + //Keep saying why a held group did not move + if (m_status_bar && !m_driver_held) { m_status_bar->clearMessage(); } } diff --git a/sources/elementsmover.h b/sources/elementsmover.h index a8fd1857d..2074a88eb 100644 --- a/sources/elementsmover.h +++ b/sources/elementsmover.h @@ -52,6 +52,7 @@ class ElementsMover { int beginMovement(Diagram *, QGraphicsItem * = nullptr); void continueMovement(const QPointF &); void endMovement(); + bool holds(const QGraphicsItem *item) const; // attributes private: @@ -59,6 +60,7 @@ class ElementsMover { QPointF m_current_movement; Diagram *m_diagram{nullptr}; QGraphicsItem *m_movement_driver{nullptr}; + bool m_driver_held{false}; DiagramContent m_moved_content; QPointer m_status_bar; diff --git a/sources/qetgraphicsitem/diagramtextitem.cpp b/sources/qetgraphicsitem/diagramtextitem.cpp index 771a3d42d..6e1a7aa02 100644 --- a/sources/qetgraphicsitem/diagramtextitem.cpp +++ b/sources/qetgraphicsitem/diagramtextitem.cpp @@ -366,6 +366,12 @@ void DiagramTextItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) { if(diagram_ && m_first_move) diagram_->elementsMover().beginMovement(diagram_, this); + if (diagram_ && diagram_->elementsMover().holds(this)) { + m_first_move = false; + event->accept(); + return; + } + QPointF old_pos = pos(); //Set the actual pos diff --git a/sources/qetgraphicsitem/qetgraphicsitem.cpp b/sources/qetgraphicsitem/qetgraphicsitem.cpp index 505c4cc37..631ee8d85 100644 --- a/sources/qetgraphicsitem/qetgraphicsitem.cpp +++ b/sources/qetgraphicsitem/qetgraphicsitem.cpp @@ -144,6 +144,11 @@ void QetGraphicsItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event) //It's the first movement, we signal it to parent diagram diagram()->elementsMover().beginMovement(diagram(), this); } + if (diagram() && diagram()->elementsMover().holds(this)) { + m_first_move = false; + event->accept(); + return; + } //we apply the mouse movement QPointF old_pos = pos();