Closed
Bug 928833
Opened 11 years ago
Closed 10 years ago
[meta] Add touch-sensitive regions to the layer tree for APZC use
Categories
(Core :: Panning and Zooming, defect)
Core
Panning and Zooming
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: kats, Assigned: kats)
References
Details
(Keywords: meta)
Attachments
(2 files)
2.72 KB,
text/plain
|
Details | |
1.07 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
There are a few features that we want to support in the APZC code that currently don't work or work poorly. For all of these things (see list of dep bugs) we need to have more precise hit-testing in the APZC code, that allows for non-rectangular regions of layers. Currently the APZC code assumes an entire layer is scrollable or not, but in reality a layer is composed of potentially many content elements, some of which may be scrollable and others may not (this is particularly true with the touch-action CSS property). Also things like opacity, rounded corners and layer flattening can affect which areas are sensitive to touches and which are not.
After some discussion with roc, nrc, and BenWa we came up with the outline of a plan to support all of these use cases. The plan is to have each layer store a set of nsIntRegions that are touch-sensitive, along with the action that the region corresponds to. We need at least four such regions:
1) one for pointer-events: none (i.e. pass the touch input down to the next layer).
2) one for pan-x
3) one for pan-y
4) one for pan x/y (touch-action: auto)
Any point on the layer outside these four regions is implicitly "touch-action: none" and corresponds to no panning behaviour in the APZC. For the more complex regions that cannot be represented by an nsIntRegion, we can default to the touch-action: none case, and have gecko-based hit detection on the main thread. We will need an API for gecko to kick off scrolling on the APZC if it turns out that the complex hit detection determines that content needs to scroll.
I'm attaching my original notes of the meeting where we came up with this plan, as it has some additional context and rationale.
Depends on: 945203
Updated•11 years ago
|
Blocks: parent-process-apz
Comment 1•11 years ago
|
||
Hi guys,
can we discuss here what tasks are remaining to complete making touch-action work on apzc side only?
As I understand the work that has been done at 945203 is creating only hitRegion and dispatchToContent regions per layer. But we will need to split hitRegions into the 4 types that mentioned in the meeting notes (pan-x/pan-y/...) so that we could know in apzc what gestures are allowed without consulting with the content side.
Given this i assume that there are two big tasks remaining:
1. Extend logic which is done at 945203 by creating touch-action specific regions when building the display list and attach these regions to layers.
2. Perform hit testing through the built regions in the APZCTreeManager/AsyncPanZoomController classes when touchstart events are coming to decide what gesture are allowed.
Could you let me know please if my understanding is correct?
Flags: needinfo?(roc)
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Comment 2•11 years ago
|
||
That sounds correct, yes. Note that even without (1) implemented, we should be able to put in some plumbing in the APZ code to fix hit-detection (bug 918288). I had started work on that but ran into bug 977831 and bug 980544 which need fixing before we can proceed. I've asked Timothy to take a look at those.
Flags: needinfo?(bugmail.mozilla)
Comment 3•11 years ago
|
||
Thanks,
now the situation looks more clear to me.
Robert, could you clarify please - how hard would it be to implement the point [1]? I might be wrong but as I understand we need to add a few more regions to layers (besides hitRegion and DispatchToContent) each of which would mean the region with according touch-action value.
And collecting of such regions will be very similar to the collecting of DispatchToContent regions but with a trickier cases due to the non-usual touch-action value computing logic.
(E.g. for panning with need to traverse elements from the closest scrolling element to current and collect all touch-behavior restrictions).
[1] Extend logic which is done at 945203 by creating touch-action specific regions when building the display list and attach these regions to layers.
(In reply to Nick Lebedev [:nl] from comment #3)
> Robert, could you clarify please - how hard would it be to implement the
> point [1]? I might be wrong but as I understand we need to add a few more
> regions to layers (besides hitRegion and DispatchToContent) each of which
> would mean the region with according touch-action value.
Yes, I think that's right.
> And collecting of such regions will be very similar to the collecting of
> DispatchToContent regions but with a trickier cases due to the non-usual
> touch-action value computing logic.
> (E.g. for panning with need to traverse elements from the closest scrolling
> element to current and collect all touch-behavior restrictions).
Yes. I find the behavior of the touch-action property very confusing :-(
> [1] Extend logic which is done at 945203 by creating touch-action specific
> regions when building the display list and attach these regions to layers.
Yes.
Flags: needinfo?(roc)
Assignee | ||
Updated•10 years ago
|
No longer blocks: parent-process-apz
Assignee | ||
Comment 5•10 years ago
|
||
Stealing this bug and repurposing it to actually turn on the event regions code. The APZ code to use the event regions is in bug 918288 and dependencies. Once that's all landed I'd like to wait a day or to to let it bake, and then land this to switch over to the event-regions codepaths. That way if we uncover serious problems we can flip the pref back to disable it.
Assignee: nobody → bugmail.mozilla
Assignee | ||
Comment 6•10 years ago
|
||
I split out bug 1101627 and bug 1101628 for the touch-action support that was meant to be a part of this bug.
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8525373 [details] [diff] [review]
Enable event regions
Nightly's on 37, so let's get this rolling.
Attachment #8525373 -
Flags: review?(botond)
Updated•10 years ago
|
Attachment #8525373 -
Flags: review?(botond) → review+
Assignee | ||
Comment 8•10 years ago
|
||
Comment 9•10 years ago
|
||
Backed out in https://hg.mozilla.org/integration/b2g-inbound/rev/1a5ef4b0dd44 for mass debug mochitest hangs and a smattering of opt reftest failures.
Assignee | ||
Comment 10•10 years ago
|
||
Relanded as https://hg.mozilla.org/integration/mozilla-inbound/rev/8564f04a6f14 after dealing with all the failures.
Comment 11•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•10 years ago
|
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•