Add support for touch scrolling in Dolphin
AbandonedPublic

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

Details

Reviewers
ngraham
abalaji
Group Reviewers
Dolphin
Summary

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

Repository
R318 Dolphin
Branch
touchscroll
Lint
No Linters Available
Unit
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

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.

Hi,
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);

to:

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

an in the informationpanelcontent.cpp:

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

to

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.
src/kitemviews/kitemlistcontroller.cpp
73

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
src/kitemviews/kitemlistcontroller.cpp
73

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

elvisangelaccio added inline comments.Dec 3 2018, 8:49 PM
src/kitemviews/kitemlistcontroller.cpp
73

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

More details here: https://medium.com/genymobile/how-c-lambda-expressions-can-improve-your-qt-code-8cd524f4ed9f

abalaji added inline comments.Dec 3 2018, 9:36 PM
src/kitemviews/kitemlistcontroller.cpp
73

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

Hey @ngraham,
I'm terribly sorry, I've been busy with other things. I'll definitely come
back to this is a month or two, after things get a little freed up on my
end. Sorry for making you wait this long, I just saw your message

meven added a subscriber: meven.Feb 18 2020, 4:47 PM

ping @abalaji

Do you think you can have time to do this ?
We might want to let someone else commandeer this work (it means take over).

@abalaji annual ping. :) If you're swamped for time, we can find someone who can finish it up, no problem.

Oh crap, sorry I didn't reply last time. Things are pretty dicey for me at
the moment, so I'm not going to have the time right now unfortunately :(.
If someone could take over this diff, I would appreciate it

No problem.

@meven, you interested?

meven added a comment.Apr 24 2020, 6:14 AM

No problem.

@meven, you interested?

I would, but too many things on my plate right now.

if nobody is working on it, i will try to make it work

<3 <3

Feel free!

niccolove added a subscriber: niccolove.
elvisangelaccio commandeered this revision.Jul 5 2020, 4:27 PM
elvisangelaccio abandoned this revision.
elvisangelaccio edited reviewers, added: abalaji; removed: elvisangelaccio.