Add support for touch scrolling in Dolphin
Needs ReviewPublic

Authored by abalaji on Apr 12 2018, 8:21 PM.


Group Reviewers

This patch adds support for touch scrolling in Dolphin using QScroller, with touch events sharing some functions with mouse events to behave like clicks. Rubberbands do not trigger during touch scrolling. To prevent accidental item activation during touch scrolling, I check if there was a touch update event with a touch move emitted in between a touch begin and a touch end event, in which case I prevent item activation from happening. Also added the QScroller gesture to the information panel (which is kinda wip right now due to issues).

FEATURE: 385066

Diff Detail

R318 Dolphin
No Linters Available
No Unit Test Coverage
Build Status
Buildable 6013
Build 6031: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
abalaji updated this revision to Diff 32099.Apr 14 2018, 7:40 AM

Simplify some if-else statements

Uh-oh, it looks like most of the diff got wiped out by that update...

abalaji updated this revision to Diff 32133.Apr 14 2018, 5:58 PM

Squash diffs together

Okay, so that seems to have fixed that. Is there a cleaner way to append a diff rather than squashing commits together? Can I just push to a branch or something?

abalaji marked an inline comment as done.Apr 14 2018, 6:01 PM

Thanks! I'll use that from now on. I've just been using git format-patch so far.

abalaji updated this revision to Diff 32137.Apr 14 2018, 6:23 PM

Now using arc

Getting better! The click/tap issue is gone. But now touch-and-drag in an empty area always starts a rubber band selection, and touch-and-drag on an item immediately starts to drag it--though in both cases, the view is scrolled too, if it's scrollable.

Touch-and-hold-for-a-moment-and-then-drag works correctly to drag an item or start a rubber band selection in an empty area.

Hmm, I had to get rid of the touch-hold-drag thing because earlier I was just adapting the existing mouse event handlers with touch, but because multitouch caused issues with that, I've now had to setAcceptTouchEvents(true) on the KItemListView, which means Qt will no longer emit fake mouse events on touch, and that also means no drag and drop because it seems that only works with mouse events. I also disabled rubberbands on touch so I'm seeing any on my end. What kind of touchscreen do you have? Can you verify if touchBeginEvent, touchUpdateEvent, and touchEndEvent are all firing for you?

abalaji updated this revision to Diff 32154.Apr 14 2018, 9:54 PM
  • Touch double click
abalaji updated this revision to Diff 32433.Apr 17 2018, 11:51 PM
  • Minor fixes and cleanup

Sorry for the long hiatus!

Touch double-click is working well now, very nice! But continue to have touch-and-drag issues:

  • When the Breeze theme setting for Windows' drag mode is set to Drag windows from titlebar only, touch-and drag in an empty area starts a rubber band selection, and touch-and-drag on top of a file or folder drags it (this should only happen on touch-and-hold-and-drag).
  • When it's set to "nav Drag windows from all empty areas}, I can't actually drag files and folders at all; they never get picked up when I touch-and-hold-and-drag.
Restricted Application added a subscriber: kfm-devel. · View Herald TranscriptMay 15 2018, 11:09 PM

What's the status here, @abalaji? I and a lot of people would really love to see this in Dolphin! :-)

abalaji added a comment.EditedJun 12 2018, 6:39 PM

Hey @ngraham sorry I've been missing this whole time. I'm back in school and flooded with assignments atm :-(. If you're super interested in this, then I can work on this over the weekend. So at the moment, here's what's working on my config at least:

  • Touch scroll in the KItemList (which covers both the file/folder view as well as the "Places" panel on the left)
  • Opening items on single touch and double touch (respects the single/double click activation policy)

What's not working as well

  • Touch scroll in the information panel on the right with the Breeze widget style with "Drag windows from all empty spaces" enabled. What happens is a drag gets initiated, but because of some reason, the move stops right there and nothing happens when I move my finger around. Same goes for the settings window.

When I pull my finger away, the window stays in the "move" mode. What I mean by this is the cursor stays in the + shape and the window is translucent if that desktop effect is enabled, even if my finger is now off the screen. When I touch the screen again, the window jumps into that spot as if I intended to move the window to that spot. This bug already exists and occurs on the upstream dolphin.

What I had to remove

  • The touch-hold-n-drag to preserve the dragging capabilities from mouse usage.

What happened is, initially I was relying on "fake" mouse events generated by Qt if the widget does not choose to listen to touch events. The problem with this is that we are limited to one touch point, and if the user tries to touch multiple fingers at once, one cannot accurately determine when each touch is released, since a mouse release event is never fired consistently in that scenario, causing a lot of glitches when you tried multitouch gestures for instance.

Now, I've had to switch to native touch gestures, and make the mouse and touch events wrappers for the actual functionality, which is now refactored into separate functions. Qt does not have a touch equivalent for mouse drag gestures, hence dragging cannot work properly, so I had to remove it.

abalaji retitled this revision from Support for touch scrolling in Dolphin to Add support for touch scrolling in Dolphin.Jun 12 2018, 7:00 PM

Thanks for the updates! Hopefully someone else from Dolphin can give you some pointers.

I have one problem with touch on the scrollbar, it does not work proberly.
I have change follow a line in the kitemlistcontainer.cpp :

QScroller::grabGesture(this), QScroller::TouchGesture);


QScroller::grabGesture(this->viewport(), QScroller::TouchGesture);

an in the informationpanelcontent.cpp:

QScroller::grabGesture(m_metaDataArea, QScroller::TouchGesture);


QScroller::grabGesture(m_metaDataArea->viewport(), QScroller::TouchGesture);

and now it works perfect.

abalaji updated this revision to Diff 37018.EditedJul 1 2018, 4:31 PM
  • Fix issues with scroll bar on touch

Thanks @steffenh, I've fixed the issues with the scroll bars.

It would be really nice to have eyes from some more Dolphin folks on this. :)

@broulik, any chance you could help out with the code review for this? My sense is that a lot of people would really love touch support in Dolphin.

acrouthamel added a subscriber: acrouthamel.

How cool is this, how have I not seen this yet?

Hey everyone, I can make some time for myself during this weekend to work on this, feel free to leave comments

Awesome, thank you!

abalaji updated this revision to Diff 42162.Sep 23 2018, 2:58 AM
  • Rebase

FWIW, @davidedmundson just submitted a patch that fixes one of the issues you were running into: D15942: Don't drag windows in empty areas from touch/pen events

abalaji updated this revision to Diff 42940.Oct 5 2018, 6:49 PM
  • Rebase

@abalaji, is this waiting for more UI or code review, or are you still working on some outstanding issues?

abalaji updated this revision to Diff 43096.Oct 8 2018, 2:29 AM
  • Add scrolling to item lists in Configure menu

Hey @ngraham, as you can see I just added scrolling to two list views in the configure menu. Apart from that, there's just one minor issue in the information panel that I'm working on right now: when you touch a clickable thing like a link or a ratings star, then touch scroll, then release the touch point at the same spot on top of the clickable thing, it gets activated. I'm in the process of fixing this. But otherwise, a code review would be appreciated.

Awesome. I'll be able to test again soon. I don't feel qualified to offer code review for this kind of patch though, so hopefully @broulik, @elvisangelaccio, or @anthonyfieroni will be able to do so.

abalaji updated this revision to Diff 43182.Oct 9 2018, 5:22 AM
  • Tidy up diff
abalaji updated this revision to Diff 43183.Oct 9 2018, 5:24 AM
  • Remove unnecessary newline

I don't have a touch screen for actual testing, but I'll have a look asap anyway.

I'll be happy to do all the UI testing if you can do the code review!

abalaji updated this revision to Diff 46337.Nov 27 2018, 7:24 PM
  • Rebase
elvisangelaccio requested changes to this revision.Dec 2 2018, 11:20 AM

Finally had the time to look at this diff.

The patch is surprisingly simpler than I was expecting, but still I'd recommend to split it into two parts:

  1. The code refactoring in KItemListController (i.e. onPress(), onMove(), etc.)
  2. The new support for QScroller and the touch events.

Please add this as receiver, otherwise this connection can lead to crashes.

This revision now requires changes to proceed.Dec 2 2018, 11:20 AM
abalaji added inline comments.Dec 3 2018, 6:32 PM

I'm sorry can you elaborate on this a little bit?

elvisangelaccio added inline comments.Dec 3 2018, 8:49 PM

Basically Qt could call the lambda slot after the object has been already destroyed.

More details here:

abalaji added inline comments.Dec 3 2018, 9:36 PM

Ahh, of course! This makes so much sense. I've experienced weird crashes with connect and lambda's before and never knew this was a thing. Thanks for pointing it out!

abalaji updated this revision to Diff 47557.Dec 14 2018, 11:16 AM
  • Fix leak and warning

@abalaji any update here? This is a really nice feature and I'd really like to see it land in 19.04. :)

Hey @ngraham Im so sorry, I didn't see your message come through. I've been pretty busy the past week because of midterms at my university. I'll split this into two asap

Thank you so much! Don't let me rush you and make your studies suffer, though. :)

FWIW, the 19.04.0 branches were just created today so this is going to have to go into 19.08.0.

Oh rip, looks like I missed the bus. I have it done halfway locally,
perhaps I can get it cleaned up by tomorrow. Any chance for 19.04?

There's always a chance. However being realistic, a big patch like this is probably better to commit right after branching anyway, because then we have four months of pre-release testing and we can ensure it's polished all nice and shiny bright for the next release. :)

Well true, probably for the best to go for 19.08. Sorry, I should have
tried to manage my time better

It's all good my friend! I have faith that it'll get in when it's ready.

Who knows, maybe eventually you'll get to use the gesture code that just landed in Gwenview once @steffenh upstreams it into a framework. :)

Friendly ping! :)

Hey, I'll be free come May, then I'll get back into action :)

@ngraham I'm all settled down and have enough free time in a day to start contributing once again. I'll put up the updated diffs in the coming days

Sweet, thanks for the update!

@ngraham still in the works unfortunately. Found an issue with it actually in one of the info panel widgets, tried to fix that a little. Last worked on it in the weekend, I'll let you know when it's done (hopefully really soon)

Thanks for the update!

Friendly ping! :)

Hey man I'm still working on it, almost done, sorry it's been over a month.
Almost good to go. The plan is to wrap it up this weekend so we can get it
reviewed and merged before the next kde applications release