Support mobile swipe/drag gestures in RDM
Categories
(DevTools :: Responsive Design Mode, enhancement, P3)
Tracking
(firefox79 fixed)
Tracking | Status | |
---|---|---|
firefox79 | --- | fixed |
People
(Reporter: mtigley, Assigned: mtigley)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 4 obsolete files)
This bug looks at supporting dragging gestures in RDM, which will allow the user to simulate proper swiping/scrolling gestures with the mouse.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
Updates to this:
Since Bug 1623941 landed, RDM is now able to perform drag gestures on the page via dragging the pointer on a trackpad or mouse. The next missing piece is having the "fling" animation that occurs on a touchend
event. This should be handled at: https://searchfox.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#1499
Because RDM's touch simulator is synthesizing native touch points to trigger gestures on the platform-level, this should just work. But it seems that PanGestureInput
events are not being triggered so that the state PAN_MOMENTUM
is set on AsyncPanZoomController
: https://searchfox.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp#2698.
So I think what should be happening is that the APZCTreeManager
input queue should be processing a PanGestureInput
at some point (perhaps starting at: https://searchfox.org/mozilla-central/source/gfx/layers/apz/src/InputQueue.cpp#415) before a touchend
is processed. But right now the processing of this PanGestureInput
just isn't happening.
Botond, do you know where the start of dispatching of pan gestures for drag gestures might occur?
Comment 3•4 years ago
|
||
The PanGestureInput
code isn't relevant to this particular scenario; it's used with hardware input devices that can trigger panning directly, such as trackpads.
With touch events, the touchend
event handling should trigger a fling animation, by going into HandleEndOfPan() which then starts a fling animation via this call. Most likely something is bailing out before getting to that point.
Assignee | ||
Comment 4•4 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3)
With touch events, the
touchend
event handling should trigger a fling animation, by going into HandleEndOfPan() which then starts a fling animation via this call. Most likely something is bailing out before getting to that point.
Thanks for the input! I built my original assumption around mState == PAN_MOMENTUM
for touchend
event handling to trigger the HandleEndOfPan() call. But I see there are other states such as PANNING
that can cause it too. I'll continue my investigation by looking at the code responsible for setting those states.
Comment 5•4 years ago
|
||
I had a brief look at this. The issue seems to be that APZ is receiving both mouse and touch events, and the mouse-up causes it to transition to the NOTHING
state, which causes the touch-end to not enter the case for handling the PANNING
state.
Comment 6•4 years ago
|
||
The only reason APZ cares about mouse events is scrollbar dragging. Since scrollbar dragging also works via touch events, perhaps we should make APZ just ignore mouse events when RDM is active.
It occurred to me that perhaps RDM prevent-defaulting the mouse events should already have this effect, but it doesn't because mouse events aren't APZ-aware, which means they don't opt into the mechanism that causes APZ to wait and see if an event was prevent-defaulted and only process it if it wasn't.
Comment 7•4 years ago
|
||
I cooked up a quick patch to make the mousedown event APZ-aware, such that prevent-defaulting it causes APZ to ignore the drag gesture it starts.
With this patch, we get further: we get into the PANNING
case in OnTouchEnd()
, but we still don't trigger a fling animation because the computed velocity is zero. That will require further investigation.
I'm also not sure whether making the mousedown event APZ-aware in general is the best solution, as it can have negative side effects for performance (APZ needs to wait for content more often). We may want a solution more targeted to RDM.
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #7)
I'm also not sure whether making the mousedown event APZ-aware in general is the best solution, as it can have negative side effects for performance (APZ needs to wait for content more often). We may want a solution more targeted to RDM.
Thanks for looking into this a bit, Botond! I agree we should try to make this RDM-specific, especially because of performance losses. I'll work on a patch that does this using your work as guidance.
Comment 9•4 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #7)
With this patch, we get further: we get into the
PANNING
case inOnTouchEnd()
, but we still don't trigger a fling animation because the computed velocity is zero. That will require further investigation.
This turned out to be a Linux-specific issue related to synthesized touch events having incorrect timestamps. The attached patch fixes it. With these two patches, mouse-drags successfully trigger fling animations for me in RDM (tested on Linux).
Assignee | ||
Comment 10•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D77395
Comment 14•4 years ago
|
||
Having a valid timestamp is important because APZ uses it for
velocity calculations.
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 15•4 years ago
|
||
Pushed by mtigley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/58a9b7f50a4b Add an "IsRDMTouchSimulationActive" field to ScrollMetadata. r=botond https://hg.mozilla.org/integration/autoland/rev/a5fe71693070 Have APZ ignore mouse events when RDM is active. r=botond
Comment 16•4 years ago
|
||
Backed out 2 changesets (bug 1621781) for Mochitest failures in gfx/layers/apz/test/mochitest/test_group_mouseevents.html
Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304364512&repo=autoland&lineNumber=7132
Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=a5fe71693070d3fea7a60d09eef4452fddbb5704
Backout:
https://hg.mozilla.org/integration/autoland/rev/b35b66158e96d01893080bc31f8916ac0fbb4ad8
Comment 17•4 years ago
|
||
Looks like the call at https://hg.mozilla.org/integration/autoland/rev/a5fe71693070#l1.20 needs to be inside a scoped block with a RecursiveMutexAutoLock lock(mRecursiveMutex);
to avoid a data race with reading/writing the scroll metadata.
Comment 18•4 years ago
|
||
Right, sorry, should have caught this during review.
To elaborate a bit, mScrollMetadata
can be accessed from multiple threads, and therefore accesses to it need to be guarded by acquiring mRecursiveMutex
, as mentioned here. (The comment is a bit out of date as it uses the word "monitor" when it means "mutex". The type of the mutex used to be RecursiveMonitor
as has since been changed to RecursiveMutex
.)
The "scoped block" that Kats is referring to is a technique that involves using a loose pair of braces like e.g. here to cause the RecursiveMutexAutoLock
to go out of scope (and therefore the mutex to be unlocked) earlier than at the end of the function. This is important to respect lock ordering, as code later in HandleDragEvent()
(in particular, this call) acquires the tree lock (which the lock ordering says cannot be acquired while mRecursiveMutex
is held).
Assignee | ||
Comment 19•4 years ago
|
||
Thank you, Kats and Botond for explaining the reason behind the backout. I've updated the patch accordingly and have a green try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7bacb8aa705ce0dc8880c55b198848b60516e0f.
Comment 20•4 years ago
|
||
Pushed by mtigley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4cbcf8cdb97d Add an "IsRDMTouchSimulationActive" field to ScrollMetadata. r=botond https://hg.mozilla.org/integration/autoland/rev/a6eed456d959 Have APZ ignore mouse events when RDM is active. r=botond
Comment 21•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4cbcf8cdb97d
https://hg.mozilla.org/mozilla-central/rev/a6eed456d959
Comment 22•4 years ago
|
||
This is awesome Micah, thanks for working on this. It is making RDM so much better mobile-like.
Updated•4 years ago
|
Comment 23•4 years ago
|
||
Added mention of drag events under "Enable/Disable touch simulation" in the Controlling Responsive Design Mode section. Also, general copy-editing and clean-up of this article.
Description
•