[Development] Bug (and fix) in QGItem::itemChange() (?)

Ch'Gans chgans at gna.org
Thu Feb 16 05:52:11 CET 2017


Hi there,

Here is my use-case:
An item has "handles" child items, when the user moves around an handles,
the parent item update it's geometry and position accordingly. Simple
concept to let the user change an item geometry by dragging handles.
Imagine, you're using a drawing application, you select a circle and 5
handles appear on the screen, one in each corner to resize the circle, and
one in the center to move the circle around.

The way i implement it is to let the circle item filter an handle item's
'itemChange()' notification. This work nicely for resizing handles, but
fails for the 'move handle'. Here is the simplified pseudo code:
QVariant HandleItem::itemChange(change, value)
{
 // Move the parent to where the handle wants to go...
 parentItem()->setPos(parentItem()->mapToParent(value);
 // ... and leave the handle at parent's origin
 return QPointF(0, 0);
}

After banging my head for a while, i finally pin-pointed what I think is
the problem:

This is related to how items are moved in
qt5/qtbase/src/widgets/graphicsview/qgraphicsitem.cpp. When a move begins,
the 'initial position' of the item is stored in the scene. This initial
position is simply the value of item->pos(), which is in parent's item CS:

initialPositions = d_ptr->scene->d_func()->movingItemsInitialPositions;
if (initialPositions.isEmpty()) {
  foreach (QGraphicsItem *item, selectedItems)
    initialPositions[item] = item->pos();
  initialPositions[this] = pos();
}
d_ptr->scene->d_func()->movingItemsInitialPositions = initialPositions;

As the move goes on, the item's pos is updated based on this 'initial
position' (expressed in an outdated parent's CS) and the parent position
relative to the item's position:

currentParentPos = item->mapToParent(item->mapFromScene(event->scenePos()));
buttonDownParentPos =
item->mapToParent(item->mapFromScene(event->buttonDownScenePos(Qt::LeftButton)));
[...]
item->setPos(initialPositions.value(item) + currentParentPos -
buttonDownParentPos);

Basically the above 3 lines would be correct only and only if the
initialPositions were expressed in a dynamic way, that is, if they were
independent of the parent's CS since it can change during the move.

The following pseudo-code modifications fixes that:

// 'Capture' time
initialPosition = item->mapToScene(item->mapFromParent(item->pos())); //
Independent of parent CS
[...]
// 'Update' time
initialPosition = item->mapToParent(item->mapFromScene(initialPosition));
// up-to-date w/ changed parent's CS
item->setPos(initialPosition + currentParentPos - buttonDownParentPos);


This modification fixes my use-cases, and doesn't seem to break the "40000
chips" and other demo/example.
The scene's private 'movingItemsInitialPositions' is only set/used by
QGI::mouseMoveEvent() and cleared by QGI::mouseReleaseEvent(), so this is
not an invasive change


So my chain of question is:
- Is changing the item's parent position in an item's itemChange() a
supported feature?
- if it is, then isn't there a bug in qgraphicsitem.ccp as i described
above?
- if positive, is the above fix correct and would you accept this sort of
things if i push that to gerrit?


Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20170216/b332976e/attachment.html>


More information about the Development mailing list