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 <noreply@anthropic.com>
This commit is contained in:
ispyisail
2026-09-29 23:23:32 +13:00
parent cbbb9fd0ac
commit 2dbaa69186
6 changed files with 91 additions and 2 deletions
+48 -1
View File
@@ -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<Element *> 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<Conductor *> 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<QUuid> 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)
+3
View File
@@ -100,6 +100,9 @@ class DiagramContent
DiagramContent& operator+=(const DiagramContent& other);
bool potentialIsManaged(QList<Conductor *>conductors);
bool hasTextEditing();
private:
int removePinnedGroups();
};
QDebug &operator<<(QDebug, DiagramContent &);
#endif
+27 -1
View File
@@ -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();
}
}
+2
View File
@@ -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<QStatusBar> m_status_bar;
@@ -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
@@ -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();