Search code examples
c++qtqgraphicsview

Why are the QGraphicsItem's children not getting mouse clicks anymore if the item is selected?

I am using the SizeGripItem from here (with a minimal modification to add a signal) on a subclass of QGraphicsRectItem defined as such:

Header

//! RectItem that sends a boxChanged signal whenever it is moved or resized via the handles.
class QSignalingBoxItem : public QObject, public QGraphicsRectItem
{
    Q_OBJECT

private:
    void setSelected_(bool selected);

protected:
    QVariant itemChange(GraphicsItemChange change, const QVariant & value);

public:
    QSignalingBoxItem(QRectF rect, QGraphicsItem * parent = nullptr);
    ~QSignalingBoxItem();

signals:
    void boxChanged(QRectF newBox);
};

.cpp

namespace
{
    class BoxResizer : public SizeGripItem::Resizer
    {
    public:
        virtual void operator()(QGraphicsItem* item, const QRectF& rect)
        {
            QSignalingBoxItem* rectItem =
                dynamic_cast<QSignalingBoxItem*>(item);

            if (rectItem)
            {
                rectItem->setRect(rect);
            }
        }
    };
}

QSignalingBoxItem::QSignalingBoxItem(QRectF rect, QGraphicsItem * parent) : QGraphicsRectItem(parent)
{
    setFlags(QGraphicsItem::ItemIsMovable | 
             QGraphicsItem::ItemIsSelectable | 
             QGraphicsItem::ItemSendsScenePositionChanges);
}

QVariant QSignalingBoxItem::itemChange(GraphicsItemChange change, const QVariant & value)
{
    switch (change)
    {
    case QGraphicsItem::ItemSelectedHasChanged:
        setSelected_(value.toBool());
        break;
    case QGraphicsItem::ItemScenePositionHasChanged:
        emit boxChanged(rect());
        break;
    default:
        break;
    }
    return value;
}

void QSignalingBoxItem::setSelected_(bool selected)
{
    //TODO: Test that it works as expected
    if (selected)
    {
        auto sz = new SizeGripItem(new BoxResizer, this);
        connect(sz, &SizeGripItem::resized, [&]() {emit boxChanged(rect()); });
    }
    else
    {
        // Get the child
        // If it's a SizeGripItem, delete it.
        if (childItems().length() > 0)
        {
            foreach(QGraphicsItem* item, childItems())
            {
                auto sz = dynamic_cast<SizeGripItem*>(item);
                if (sz)
                {
                    delete sz;
                    break;
                }
            }
        }
    }
}

If I apply the SizeGripItem in the constructor, the class works as expected. However, I need the handles to be visible and working only if the item is selected. When I run the code above, though, the handles show but any click on the handles moves the whole box (as if i clicked in the middle of the box) instead of resizing it.

Running with a debugger, I see that itemChange doesn't even get called. Why is this the case? And how do I need to modify the class to make this work when the item is selected?

Edit

This is how the class is then used. I have an extension of a QGraphicsView with its private m_scene where I override mouse{Press,Move,Release}Events to generate a box via "click & drag". Setting aside the rest of the class, since the only part of it that touches QSignalingBoxItems are those three functions, I'm adding those. If more is needed, I'll add as necessary.

void QSampleEditor::mousePressEvent(QMouseEvent * mouseEvent)
{
    QImageView::mousePressEvent(mouseEvent);
    if (mouseEvent->buttons() == Qt::RightButton && m_currentSample && m_activePixmap && m_activePixmap->isUnderMouse())
    {
        m_dragStart = mapToScene(mouseEvent->pos());
        m_currentBox = new QSignalingBoxItem({ m_dragStart, QSize(0,0) });
        m_scene.addItem(m_currentBox);
        mouseEvent->accept();
    }
}

void QSampleEditor::mouseMoveEvent(QMouseEvent * mouseEvent)
{
    QImageView::mouseMoveEvent(mouseEvent);
    if (m_currentBox)
    {
        m_currentBox->setRect(rectFromTwoPoints(m_dragStart, mapToScene(mouseEvent->pos())));
        mouseEvent->accept();
    }
}

void QSampleEditor::mouseReleaseEvent(QMouseEvent * mouseEvent)
{
    //TODO: Add checks that the sample is still there.

    QImageView::mouseReleaseEvent(mouseEvent);
    if (m_currentBox)
    {
        // Add to the StringSample
        auto new_char = new CharacterSample;
        connect(m_currentBox, &QSignalingBoxItem::boxChanged, new_char, &CharacterSample::boxChanged);
        new_char->boxChanged(m_currentBox->rect());
        m_currentSample->addCharacter(new_char);
        m_currentBox = nullptr;
        mouseEvent->accept();
    }
}
Solution

  • When a given item has any of its movable ancestor items selected, the eldest selected ancestor receives the event instead - even if the item's position overlaps the child items. This is handled in QGraphicsItem::mouseMoveEvent - the move event is not handled if a movable ancestor exists. The eldest movable selected ancestor receives the event and uses it to move itself, but the descendant items ignore it (here - the handles!).

    General notes about the code (yours and the one you're reusing):

    1. The type names beginning with Q are reserved for Qt. You're not supposed to use such names unless you've put Qt in a namespace.

    2. The SizeGripItem should be flagged to have no contents - because its paint method is a no-op.

    3. Pass non-numeric, non-pointer method arguments via const reference unless the method needs a copy to modify internally.

    4. The SizeGripItem needs to have either a Resizer or emit a signal, not both - these two options are mutually exclusive.

      In fact, the Resizer is a Qt 4 vestige where slots were verbose and it wasn't possible to connect to a lambda. It is completely unnecessary in Qt 5, where a signal can be connected to any functor, including a functor of the Resizer type - thus making the QObject::connect implicitly backwards-compatible with explicit use of Resizer (!).

    The following solutions can be proposed:

    1. The trivial solution - this suppresses the primary cause of the problem: make the managed item non-movable. Handles will then work. It has nothing to do with selection status. It's the movability that induces the problem.
    2. Make the SizeGripItem movable itself. The SignalingBoxItem cannot be movable.
    3. Reimplement SizeGripItem::HandleItem's mouseMoveEvent to accept the relevant events and react to them. The SignalingBoxItem remains movable.
    4. Have SizeGripItem be an event filter for its HandleItems and process the relevant events as in solution #2. The SignalingBoxItem remains movable.
    5. Make the SizeGripItem be a sibling of the SignalingBoxItem. The SignalingBoxItem then can be movable, without impending the use of SizeGripItem's handles. Of course no ancestors of SignalingBoxItem can be movable then.

    The following is a complete example of about 240 lines, implementing solutions 1-3. Every solution has been delineated in a conditional block, and the example compiles with any subset of them enabled. With no solution selected, the original problem remains. The enabled solutions can be selected at runtime.

    First, let's start with the SizeGripItem:

    // https://github.com/KubaO/stackoverflown/tree/master/questions/graphicsscene-children-51596611
#include <QtWidgets>
#include <array>
#define SOLUTION(s) ((!!(s)) << (s))
#define HAS_SOLUTION(s) (!!(SOLUTIONS & SOLUTION(s)))
#define SOLUTIONS (SOLUTION(1) | SOLUTION(2) | SOLUTION(3))

class SizeGripItem : public QGraphicsObject {
   Q_OBJECT
   enum { kMoveInHandle, kInitialPos, kPressPos };
   struct HandleItem : QGraphicsRectItem {
      HandleItem() : QGraphicsRectItem(-4, -4, 8, 8) {
         setBrush(Qt::lightGray);
         setFlags(ItemIsMovable | ItemSendsGeometryChanges);
      }
      SizeGripItem *parent() const { return static_cast<SizeGripItem *>(parentItem()); }
      QVariant itemChange(GraphicsItemChange change, const QVariant &value) override {
         if (change == ItemPositionHasChanged) parent()->handleMoved(this);
         return value;
      }
#if HAS_SOLUTION(2)
      bool sceneEvent(QEvent *event) override {
         return (data(kMoveInHandle).toBool() && hasSelectedMovableAncestor(this) &&
                 processMove(this, event)) ||
                QGraphicsRectItem::sceneEvent(event);
      }
#endif
   };
#if HAS_SOLUTION(2) || HAS_SOLUTION(3)
   static bool processMove(QGraphicsItem *item, QEvent *ev) {
      auto mev = static_cast<QGraphicsSceneMouseEvent *>(ev);
      if (ev->type() == QEvent::GraphicsSceneMousePress &&
          mev->button() == Qt::LeftButton) {
         item->setData(kInitialPos, item->pos());
         item->setData(kPressPos, item->mapToParent(mev->pos()));
         return true;
      } else if (ev->type() == QEvent::GraphicsSceneMouseMove &&
                 mev->buttons() == Qt::LeftButton) {
         auto delta = item->mapToParent(mev->pos()) - item->data(kPressPos).toPointF();
         item->setPos(item->data(kInitialPos).toPointF() + delta);
         return true;
      }
      return false;
   }
   static bool hasSelectedMovableAncestor(const QGraphicsItem *item) {
      auto *p = item->parentItem();
      return p && ((p->isSelected() && (p->flags() & QGraphicsItem::ItemIsMovable)) ||
                   hasSelectedMovableAncestor(p));
   }
#endif
   std::array<HandleItem, 4> handles_;
   QRectF rect_;
   void updateHandleItemPositions() {
      static auto get = {&QRectF::topLeft, &QRectF::topRight, &QRectF::bottomLeft,
                         &QRectF::bottomRight};
      for (auto &h : handles_) h.setPos((rect_.*get.begin()[index(&h)])());
   }
   int index(HandleItem *handle) const { return handle - &handles_[0]; }
   void handleMoved(HandleItem *handle) {
      static auto set = {&QRectF::setTopLeft, &QRectF::setTopRight,
                         &QRectF::setBottomLeft, &QRectF::setBottomRight};
      auto rect = rect_;
      (rect.*set.begin()[index(handle)])(handle->pos());
      setRect(mapRectToParent(rect.normalized()));
   }

  public:
   SizeGripItem(QGraphicsItem *parent = {}) : QGraphicsObject(parent) {
      for (auto &h : handles_) h.setParentItem(this);
      setFlags(ItemHasNoContents);
   }
   QVariant itemChange(GraphicsItemChange change, const QVariant &value) override {
      if (change == QGraphicsItem::ItemPositionHasChanged) resize();
      return value;
   }
   void paint(QPainter *, const QStyleOptionGraphicsItem *, QWidget *) override {}
   QRectF boundingRect() const override { return rect_; }
   void setRect(const QRectF &rect) {
      rect_ = mapRectFromParent(rect);
      resize();
      updateHandleItemPositions();
   }
   void resize() { emit rectChanged(mapRectToParent(rect_), parentItem()); }
   Q_SIGNAL void rectChanged(const QRectF &, QGraphicsItem *);
#if SOLUTIONS
   void selectSolution(int i) {
#if HAS_SOLUTION(1)
      setFlag(ItemIsMovable, i == 1);
      setFlag(ItemSendsGeometryChanges, i == 1);
      if (i != 1) {
         auto rect = mapRectToParent(rect_);
         setPos({});  // reset position if we're leaving the movable mode
         setRect(rect);
      }
      i--;
#endif
      for (auto &h : handles_) {
         int ii = i;
#if HAS_SOLUTION(2)
         h.setData(kMoveInHandle, ii-- == 1);
#endif
#if HAS_SOLUTION(3)
         if (ii == 1)
            h.installSceneEventFilter(this);
         else
            h.removeSceneEventFilter(this);
#endif
      }
   }
#endif
#if HAS_SOLUTION(3)
   bool sceneEventFilter(QGraphicsItem *item, QEvent *ev) override {
      if (hasSelectedMovableAncestor(item)) return processMove(item, ev);
      return false;
   }
#endif
};

    Then, the SignalingBoxItem:

    class SignalingBoxItem : public QObject, public QGraphicsRectItem {
   Q_OBJECT
   SizeGripItem m_sizeGrip{this};
   QVariant itemChange(GraphicsItemChange change, const QVariant &value) override {
      if (change == QGraphicsItem::ItemSelectedHasChanged)
         m_sizeGrip.setVisible(value.toBool());
      else if (change == QGraphicsItem::ItemScenePositionHasChanged)
         emitRectChanged();
      return value;
   }
   void emitRectChanged() { emit rectChanged(mapRectToScene(rect())); }
   void setRectImpl(const QRectF &rect) {
      QGraphicsRectItem::setRect(rect);
      emitRectChanged();
   }

  public:
   SignalingBoxItem(const QRectF &rect = {}, QGraphicsItem *parent = {})
       : QGraphicsRectItem(rect, parent) {
      setFlags(ItemIsMovable | ItemIsSelectable | ItemSendsScenePositionChanges);
      m_sizeGrip.hide();
      connect(&m_sizeGrip, &SizeGripItem::rectChanged, this,
              &SignalingBoxItem::setRectImpl);
   }
   void setRect(const QRectF &rect) {
      setSelected(false);
      m_sizeGrip.setRect(rect);
      setRectImpl(rect);
   }
   Q_SIGNAL void rectChanged(const QRectF &);  // Rectangle in scene coordinates
#if SOLUTIONS
   void selectSolution(int index) {
      setFlag(ItemIsMovable, !HAS_SOLUTION(1) || index != 1);
      m_sizeGrip.selectSolution(index);
   }
#endif
};

    The SampleEditor:

    class SampleEditor : public QGraphicsView {
   Q_OBJECT
   bool m_activeDrag = false;
   SignalingBoxItem m_box;
   QPointF m_dragStart;

  public:
   SampleEditor(QGraphicsScene *scene) : QGraphicsView(scene) {
      scene->addItem(&m_box);
      connect(&m_box, &SignalingBoxItem::rectChanged, this, &SampleEditor::rectChanged);
   }
   Q_SIGNAL void rectChanged(const QRectF &);
   void mousePressEvent(QMouseEvent *event) override {
      QGraphicsView::mousePressEvent(event);
      if (event->button() == Qt::RightButton) {
         m_dragStart = m_box.mapFromScene(mapToScene(event->pos()));
         m_activeDrag = true;
         m_box.show();
         m_box.setRect({m_dragStart, m_dragStart});
         event->accept();
      }
   }
   void mouseMoveEvent(QMouseEvent *event) override {
      QGraphicsView::mouseMoveEvent(event);
      if (m_activeDrag) {
         m_box.setRect({m_dragStart, m_box.mapFromScene(mapToScene(event->pos()))});
         event->accept();
      }
   }
   void mouseReleaseEvent(QMouseEvent *event) override {
      QGraphicsView::mouseReleaseEvent(event);
      if (m_activeDrag && event->button() == Qt::RightButton) {
         event->accept();
         m_activeDrag = false;
      }
   }
   void resizeEvent(QResizeEvent *event) override {
      QGraphicsView::resizeEvent(event);
      scene()->setSceneRect(contentsRect());
   }
#if SOLUTIONS
   void selectSolution(int index) { m_box.selectSolution(index); }
#endif
};

    And, finally, the demo code:

    int main(int argc, char *argv[]) {
   QApplication a(argc, argv);
   QWidget ui;
   QGridLayout layout{&ui};
   QGraphicsScene scene;
   SampleEditor editor(&scene);
   QComboBox sel;
   QLabel status;
   layout.addWidget(&editor, 0, 0, 1, 2);
   layout.addWidget(&sel, 1, 0);
   layout.addWidget(&status, 1, 1);
   sel.addItems({
      "Original (Movable SignalingBoxItem)",
#if HAS_SOLUTION(1)
          "Movable SizeGripItem",
#endif
#if HAS_SOLUTION(2)
          "Reimplemented HandleItem",
#endif
#if HAS_SOLUTION(3)
          "Filtering SizeGripItem",
#endif
   });
   sel.setCurrentIndex(-1);
#if SOLUTIONS
   QObject::connect(&sel, QOverload<int>::of(&QComboBox::currentIndexChanged),
                    [&](int index) { editor.selectSolution(index); });
#endif
   QObject::connect(&editor, &SampleEditor::rectChanged, &status,
                    [&](const QRectF &rect) {
                       QString s;
                       QDebug(&s) << rect;
                       status.setText(s);
                    });
   sel.setCurrentIndex((sel.count() > 1) ? 1 : 0);
   ui.setMinimumSize(640, 480);
   ui.show();
   return a.exec();
}
#include "main.moc"