Skip to content
Snippets Groups Projects

qt: qml: implement rubber band selection

Open Fatih Uzunoğlu requested to merge fuzun/vlc:qml-rubberband_selector into master
5 unresolved threads

Qt Quick is oriented towards touch screen, and it does not provide some features that are essential for desktop applications. One of these feature is rubber band selection, which allows multiple selection by dragging views.

This merge request implements a generic rubber band selection. It works by asynchronously checking child items if they are overlapped on the rectangle selection indicator.

It has O(n) complexity because of running through all items. This can be optimized for specific usages, such as list view 1. But this generic approach allows it to be used in other places, such as custom views. I propose it as is, with room for potential improvements.

Note: Obviously, it does not work on touch screen.

Fixes #25568.

simplescreenrecorder-2021-11-22_20.20.42

  1. https://code.woboq.org/qt5/qtbase/src/widgets/itemviews/qlistview.cpp.html#_ZN9QListView12setSelectionERK5QRect6QFlagsIN19QItemSelectionModel13SelectionFlagEE :leftwards_arrow_with_hook:

Edited by Fatih Uzunoğlu

Merge request reports

Members who can merge are allowed to add commits.

Merge request pipeline #439987 passed with warnings

Merge request pipeline passed with warnings for 84e061e4

Ready to merge by members who can write to the target branch.

Merge details

  • The source branch is 5309 commits behind the target branch.
  • 6 commits will be added to master.
  • Source branch will be deleted.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Pierre Lamot
  • Pierre Lamot
  • Pierre Lamot
  • Pierre Lamot
  • Pierre Lamot
    • some feedback on the usability, probably for a future MR.

      • we should probably allow to move the view while selecting when the mouse is at the top/bottom of the view:

      Peek_2021-11-24_11-13

      • selecting in ListView is a bit hard as clicking on the item will trigger the DnD, file browser usually tackle this by making only the name & thumbnail activating the drag&drop or by making the item not DnD in the view margin

      Peek_2021-11-24_11-16

      Peek_2021-11-24_11-21

    • Author Reporter

      Auto scroll should be factored. There exists two auto scrolling based on dragging (in toolbar editor, and in the playlist). Also, they use SmoothedAnimation and it alters the animation based on content size. I thought using velocity instead of duration would correct this behavior, but turns out it just changes the average velocity. I will create a new issue for this, now.

      For the other issue, what comes to my mind is that childAt() can be used to determine the first visible item of the parent (delegate), and if it is the background, don't accept the event so it propagates. I can try to do that if this gets accepted.

    • I will create a new issue for this, now.

      OK

      For the other issue, what comes to my mind is that childAt() can be used to determine the first visible item of the parent (delegate), and if it is the background, don't accept the event so it propagates. I can try to do that if this gets accepted.

      isn't that already what happens? my understanding is that when you click, your mouse event is first intercepted by the child, which starts a drag, otherwise it goes to the parent (so the background) and here it goes to your rubber hander

    • Author Reporter

      Yes that is what happens from the context of the view. But when I think about the delegates specifically, because it has a MouseArea that is filled itself, it handles mouse events itself.

      The question is, should the delegate accept mouse events, if the mouse is over its background (not the view's background)?

      Maybe childAt() to check if the result is control.background can be used, to determine whether the mouse event would be accepted or not.

    • Please register or sign in to reply
  • Fatih Uzunoğlu added 7 commits

    added 7 commits

    • 61a7a7e2 - qt: add ItemOverlapNotifier
    • a22ff5a1 - qt: qml register type ItemOverlapNotifier
    • d4d6c5f2 - qml: add rubberband rectangle selector style properties
    • 89b7550c - qml: add RubberBandOverlapNotifier
    • bb8b3d11 - qml: add RubberBandSelector
    • 41121117 - qml: use RubberBandSelector
    • 7c449c35 - qml: connect context menu signals in medialib views

    Compare with previous version

  • Fatih Uzunoğlu resolved all threads

    resolved all threads

  • Fatih Uzunoğlu changed title from qt: qml: implement item based rubber band rectangular selection to qt: qml: rubber band selection

    changed title from qt: qml: implement item based rubber band rectangular selection to qt: qml: rubber band selection

  • Fatih Uzunoğlu changed title from qt: qml: rubber band selection to qt: qml: implement rubber band selection

    changed title from qt: qml: rubber band selection to qt: qml: implement rubber band selection

  • Pierre Lamot
  • Fatih Uzunoğlu mentioned in issue #26377

    mentioned in issue #26377

  • Fatih Uzunoğlu added 181 commits

    added 181 commits

    • 7c449c35...c8797d1a - 174 commits from branch videolan:master
    • fed788a6 - qt: add ItemOverlapNotifier
    • ccdba3a9 - qt: qml register type ItemOverlapNotifier
    • d520f966 - qml: add rubberband rectangle selector style properties
    • 8ddad711 - qml: add RubberBandOverlapNotifier
    • c5cd3d0a - qml: add RubberBandSelector
    • b6639029 - qml: use RubberBandSelector
    • fbc9e615 - qml: connect context menu signals in medialib views

    Compare with previous version

  • Fatih Uzunoğlu marked this merge request as draft

    marked this merge request as draft

  • Fatih Uzunoğlu added 1854 commits

    added 1854 commits

    • fbc9e615...47514893 - 1847 commits from branch videolan:master
    • bb57a6c0 - qt: add ItemOverlapNotifier
    • c6719ce5 - qt: qml register type ItemOverlapNotifier
    • b958c57c - qml: add rubberband rectangle selector style properties
    • 6e9376b8 - qml: add RubberBandOverlapNotifier
    • 2942f5ad - qml: add RubberBandSelector
    • 9180ef77 - qml: use RubberBandSelector
    • 840f7042 - qml: connect context menu signals in medialib views

    Compare with previous version

  • changed milestone to %Qt redesign (VLC 4.0)

  • Steve Lhomme changed milestone to %4.0

    changed milestone to %4.0

  • Fatih Uzunoğlu added 2638 commits

    added 2638 commits

    • 840f7042...e3b28612 - 2631 commits from branch videolan:master
    • b3db3fff - qt: add ItemOverlapNotifier
    • d70e4509 - qt: qml register type ItemOverlapNotifier
    • 9367875f - qml: add rubberband rectangle selector style properties
    • 44154185 - qml: add RubberBandOverlapNotifier
    • 86f653de - qml: add RubberBandSelector
    • 44f38883 - qml: use RubberBandSelector
    • 06186eb7 - qml: connect context menu signals in medialib views

    Compare with previous version

  • Author Reporter

    I will try a different approach to this soon. It should not take long.

    • Author Reporter

      My plan is to use QTableView to leverage its selection functionality. But I see that the models with the grid views do not make use of the column attribute. I guess this way I need to couple the model with the delegate and try to automatically assign column numbers for the items.

    • would this work with the grid expand functionnality ? or does this only concerns the tableviews?

      I guess this way I need to couple the model with the delegate and try to automatically assign column numbers for the items

      you probably need a proxy model to map the list to a grid

    • Author Reporter

      Did some more research, does not seem a good way to go actually... A proxy model that maps to a grid, at the same time allows splitting might not even be possible. There are more concerns, delegate size might be variable, and the tableview does not support spacing (understandable, given it is a table).

      I could not find a reliable way in Qt Quick world to achieve this, considering a year has passed, I want to either implement this for views that support delayRemove, or close this merge request. I'm in favor of the former, please let me know.

      This will at least be useful for lists, both in medialibrary views, and playlist listview.

    • Please register or sign in to reply
  • Fatih Uzunoğlu added 490 commits

    added 490 commits

    • 06186eb7...f28a4b2b - 484 commits from branch videolan:master
    • 86bd110d - qt: add ItemOverlapNotifier
    • 3523ccc8 - qt: qml register type ItemOverlapNotifier
    • 96a29f40 - qml: add rubberband rectangle selector style properties
    • b9be4294 - qml: add RubberBandOverlapNotifier
    • 4e519368 - qml: add RubberBandSelector
    • 83f65809 - qml: use RubberBandSelector

    Compare with previous version

  • Fatih Uzunoğlu added 5455 commits

    added 5455 commits

    • 83f65809...cb58ece0 - 5445 commits from branch videolan:master
    • eb9513a8 - qml: document required properties from delegate of ExpandGridView
    • 6bf838a9 - qml: minor refactor code in positioning item in ExpandGridView
    • e7357206 - qml: implement 'delayRemove' for delegates of ExpandGridView
    • c50093ec - qml: implement 'reuseItems' for ExpandGridView
    • 236a4a51 - qt: add ItemOverlapNotifier
    • 43640525 - qml: add RubberBandOverlapNotifier
    • 0bfca376 - qml: add RubberBandSelector
    • ee3f50f3 - qt: qml register type ItemOverlapNotifier
    • bd5a4bfa - qml: add rubberband rectangle selector style properties
    • d5a246e1 - qml: use RubberBandSelector

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • Fatih Uzunoğlu marked this merge request as ready

    marked this merge request as ready

  • Fatih Uzunoğlu added 53 commits

    added 53 commits

    • 137565fb...4e47bd35 - 47 commits from branch videolan:master
    • 9b412c87 - qt: add ItemOverlapNotifier
    • d754ddd9 - qml: add RubberBandOverlapNotifier
    • abfdc09c - qml: add RubberBandSelector
    • 5f20ef6d - qt: qml register type ItemOverlapNotifier
    • cde65831 - qml: add rubberband rectangle selector style properties
    • 4f0614e1 - qml: use RubberBandSelector

    Compare with previous version

  • added 6 commits

    • f4147fcf - qt: add ItemOverlapNotifier
    • 79e42d54 - qml: add RubberBandOverlapNotifier
    • fdd3a60f - qml: add RubberBandSelector
    • 6f82ab74 - qt: qml register type ItemOverlapNotifier
    • 191ec147 - qml: add rubberband rectangle selector style properties
    • 414765b5 - qml: use RubberBandSelector

    Compare with previous version

  • added 6 commits

    • e7d97d39 - qt: add ItemOverlapNotifier
    • 0c6cdb7f - qml: add RubberBandOverlapNotifier
    • 76d8404b - qml: add RubberBandSelector
    • f06d1b09 - qt: qml register type ItemOverlapNotifier
    • ef1d78c8 - qml: add rubberband rectangle selector style properties
    • 84e061e4 - qml: use RubberBandSelector

    Compare with previous version

  • Also, #28023 negatively affects the working principle of this feature.

    • Author Reporter

      @chub Regarding today's discussion, I meant that I tried capturing the items as well as their geometry so that it would not be affected by item re-using, but it did not work well. Currently, only the item addresses are captured for identification purposes (whether to inverse toggling).

    • As you're dealing with a grid (or a list) you should be able to know what indices are in your rectangle without having to retain the graphical items.

      for instance let say that your grid has 10x10 elements and no margins/gutter. if your your grid is 100x100 and your selection rectangle as its corners at (5,5) -> (25,25), you know that your selected indices are (0,1,2,10,11,12,20,21,22)

      ListView provides the indexAt that "may" be usable (I don't know how it reacts regarding headers or section delegates)

    • Please register or sign in to reply
  • Please register or sign in to reply
    Loading