Overall, the Qt library has done a fantastic job of maintaining relatively simple interfaces with well-named functions that do what they say they’re going to do [unlike MFC for example] and of having objects react the way you would expect them to without much modification. I’ve used it for several cross-platform Windows/Mac OS X/Linux applications and I can usually bend it to my will fairly easily because they’ve provided the right hooks.
One exception I’ve run into recently – and an oversight that is really kind of surprising – is using rubber band selections on a QGraphicsView. What would be natural – and what most people would expect – is that if the user is holding down the “modify selection” key – Control on Windows, Command on Mac OS X – then:
- clicking a selected object will deselect it without modifying the rest of the selection
- clicking an unselected object will select it without modifying the rest of the selection
- clicking and dragging will give the user a way to select several objects to add to the current selection (known as rubber band or drag selection).
But with QGraphicsView – by design – if you have some objects selected (as pictured above), there’s no way to extend the selection by using “rubber band selection” to add to it. Clicking the mouse clears the current selection regardless of whether the user is holding the “modify selection” key. This is a UX design bug. It is counter-intuitive and confusing for users.
Not only is the default behaviour confusing, there’s no way to correct it without either inheriting from QGraphicsView and writing a new rubber band selection system or hacking the Qt source. I chose to fix the problem by doing the latter. Fortunately this turned out to be easier than I originally thought it would be…
The general solution is as follows [my changes marked with // [ASM] Multi-selection]:
- Add a QList<QGraphicsItems *> member to QGraphicsViewPrivate in qgraphicsview_p.h to keep track of any current selection.
12345678910#ifndef QT_NO_RUBBERBANDQRect rubberBandRect;QRegion rubberBandRegion(const QWidget *widget, const QRect &rect) const;bool rubberBanding;Qt::ItemSelectionMode rubberBandSelectionMode;// [ASM] Multi-selectionQList<QGraphicsItem *> rubberBandExtendSelection;// [ASM] Multi-selection#endifint handScrollMotions;
- In QGraphicsView::mousePressEvent() in qgraphicsview.cpp, if we’ve determined that the user is “rubber banding”, check the modifier keys to see if the Control/Command key is down. If it is, save the current selection to our new private QList<QGraphicsItems *> member on QGraphicsViewPrivate. If the modifier key isn’t down, then clear the selection as we used to.
1234567891011121314151617181920#ifndef QT_NO_RUBBERBANDif (d->dragMode == QGraphicsView::RubberBandDrag && !d->rubberBanding) {if (d->sceneInteractionAllowed) {// Rubberbanding is only allowed in interactive mode.event->accept();d->rubberBanding = true;d->rubberBandRect = QRect();if (d->scene) {// [ASM] Multi-selectionbool multiSelect = (event->modifiers() & Qt::ControlModifier) != 0;if ( multiSelect )d->rubberBandExtendSelection = d->scene->selectedItems();elsed->scene->clearSelection();// [ASM] Multi-selection}}} else#endif
- Add a parameter to QGraphicsScene::setSelectionArea() in qgraphicsscene.h to pass in a pointer to our current selection.
123456789QList selectedItems() const;QPainterPath selectionArea() const;void setSelectionArea(const QPainterPath &path); // ### obsoletevoid setSelectionArea(const QPainterPath &path, const QTransform &deviceTransform);void setSelectionArea(const QPainterPath &path, Qt::ItemSelectionMode mode); // ### obsolete// [ASM] Multi-selection// Add parameter to determine if we should extend the selection or reset itvoid setSelectionArea(const QPainterPath &path, Qt::ItemSelectionMode mode, const QTransform &deviceTransform, QList<QGraphicsItem *> *inExtendSelection = NULL);// [ASM] Multi-selection
- As the user drags the selection box, call QGraphicsScene::setSelectionArea() with the new parameter in QGraphicsView::mouseMoveEvent() in qgraphicsview.cpp.
123456789101112// Set the new selection areaQPainterPath selectionArea;selectionArea.addPolygon(mapToScene(d->rubberBandRect));selectionArea.closeSubpath();if (d->scene){// [ASM] Multi-selectiond->scene->setSelectionArea(selectionArea, d->rubberBandSelectionMode,viewportTransform(), &(d->rubberBandExtendSelection));// [ASM] Multi-selection}return;
- In the modified QGraphicsScene::setSelectionArea(), check if we passed in a selection list and, if we did, ensure that the items remain selected.
123456789101112131415161718void QGraphicsScene::setSelectionArea(const QPainterPath &path, Qt::ItemSelectionMode mode,const QTransform &deviceTransform, QList<QGraphicsItem *> *inExtendSelection) // [ASM] Multi-selection{...// Unselect all items outside path.foreach (QGraphicsItem *item, unselectItems) {item->setSelected(false);changed = true;}// [ASM] Multi-selectionif ( (inExtendSelection != NULL) && !inExtendSelection->isEmpty() ){foreach (QGraphicsItem *item, *inExtendSelection) {item->setSelected(true);}}// [ASM] Multi-selection
- When the mouse is released, clear our new selection list in QGraphicsView::mouseReleaseEvent().
1234567891011121314151617#ifndef QT_NO_RUBBERBANDif (d->dragMode == QGraphicsView::RubberBandDrag && d->ceneInteractionAllowed && !event->buttons()) {if (d->rubberBanding) {if (d->viewportUpdateMode != QGraphicsView::NoViewportUpdate){if (d->viewportUpdateMode != FullViewportUpdate)viewport()->update(d->rubberBandRegion(viewport(), d->rubberBandRect));elsed->updateAll();}d->rubberBanding = false;d->rubberBandRect = QRect();}// [ASM] Multi-selectiond->rubberBandExtendSelection.clear();// [ASM] Multi-selection} else#endif
- There is one additional case where we need to check keyboard modifiers – QGraphicsScenePrivate::mousePressEventHandler(). Previously, this would clear the selection if our drag mode was not QGraphicsView::ScrollHandDrag, so we add a check for our modifiers and clear the selection if it is not down.
12345678910111213bool dontClearSelection = view && view->dragMode() == QGraphicsView::ScrollHandDrag;// [ASM] Multi-selectionbool multiSelect = (mouseEvent->modifiers() & Qt::ControlModifier) != 0;dontClearSelection |= multiSelect;// [ASM] Multi-selectionif (!dontClearSelection) {// Clear the selection if the originating view isn't in scroll// hand drag mode. The view will clear the selection if no drag// happened.q->clearSelection();}
That’s it. Now the rubber band selection works as expected when the “modify selection” key is down.
If you find this useful, or find any problems with it, please let me know!
Patch file: QGraphicsView_Multi_Select.patch
Notes:
- QGraphicsItem checks the “extend selection” key properly, allowing the user to select/deselect an item by clicking it, so no modifications were necessary there.
- When I modified the QGraphicsScene::setSelectionArea() function to add the current selection list parameter, I used a pointer to the QList<QGraphicsItem*>. This was so I could default it to NULL which means the change would not affect any other code currently using this function. The other option would be to add a new override and factor the code, but that seemed overkill for this.
- I applied this to Qt 4.8.x from git e678ed1. I haven’t looked at other versions, but it might apply to others, or at least give you a head start if you want to implement something like this on other versions of Qt.
- This bug fix has only been tested with my code for my specific case. I can’t see how what I did would affect anything else, but no guarantees!
Qt5 has the same issue. I’m a bit surprised that no one besides you (and now me) talk about this issue. This makes the selection in QGraphicsView unusable in such a way that one needs to either patch Qt like you did, or one has to rewrite the entire selection handling again.
If I find the time to apply your patch to Qt5, and if it works, we should even consider submitting this patch to Qt5 in a binary, source and behavior compatible way…
I absolutely agree. It’s a surprising oversight.
I have submitted patches in the past, but it’s a complicated process relative to other projects I contribute to. Since I don’t do it frequently, I need to relearn it every single time, so I probably don’t contribute as often as I would otherwise.
I am in the process of moving my work to Qt 5, so I’m sure I’ll run into this again eventually and want a more permanent fix…
I cleaned this up and submitted a change request via Gerrit for Qt5 and added you as a reviewer.
Hopefully it’ll go through…
Thanks for the excellent patch! 🙂 I’m glad this is finally fixed with upcoming versions of Qt.
Is this patch compatible in the sense that it could be added to Qt 4 (and Qt 5) without breaking existing applications?
Do you mean safe in the ABI sense or safe in the functionality sense?
On the ABI side, I had to add a parameter to QGraphicsScene::setSelectionArea(), so I think this causes ABI incompatibility you would have to work around (duplicate the function) if that’s an issue.
On the functionality side, I have only used it on Qt 4, but yes I believe it’s safe. As I mentioned, I actually see this as a bug fix and I tried to keep the changes as minimal as possible.
I haven’t tried it on Qt 5 yet and haven’t looked at Qt 5 to see what they might have changed.
My software ships with a modified Qt 4 and I haven’t had any issues with this fix on Mac or Windows. I don’t see anything that would prevent it working on Linux or any other platform either.
If you do try it, I’d appreciate any feedback you might have.
Thanks for stopping by!