Closed Bug 1389146 Opened 4 years ago Closed 4 years ago

Implement a performant hit-testing mechanism for APZ (in WebRender)

Categories

(Core :: Graphics: WebRender, enhancement, P1)

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox56 --- unaffected
firefox57 --- unaffected
firefox59 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [wr-mvp] [gfx-noted])

Attachments

(2 files)

We need to implement a hit-testing solution for APZ in WebRender. Although I filed bug 1389143 for getting a basic one in C++ code up and running, I think the performance characteristics of that one will not be good enough to ship. Instead, we will want to do hit-testing inside WebRender and call into that from APZ. The details of this are still a little hazy to me but there will need to be some changes inside WR to implement the hit-test and expose the necessary APIs, and then some changes on the gecko side to invoke those APIs.

The hit-test needs to provide not just the scrollable frame that was hit, but the event region information associated with the point as well. Right now WR has no notion of event regions, so we'll have to plumb that in as well which is a big enough piece of work that I'll file another bug for that.
Priority: P3 → P2
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
Some WIP: https://github.com/staktrace/gecko-dev/commits/hit-test

These patches (totally untested) in theory allow APZ to query WR to do the hit test. I haven't figured out how to hook up the scrollbar special cases yet. The part that I still need to do is actually sending WR the information it wants for hit-testing, which I'll do in bug 1389149. But before I do that I want to do the ScrollingLayersHelper cleanup that I was planning to do for bug 1405359.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Priority: P2 → P1
The code currently in m-c has a "hidden" gfx.webrender.hit-test pref which, if enabled, compares the WR hit-test results to the event-regions hit-test result and prints mismatches out to stderr. This is basically useless now, since the event-regions hit-test results are not correct enough to be used as a baseline, and the only functional change is more slowness.

Instead I'm going to make this pref actually turn on the WR hit-test codepaths and turn off the event regions codepaths. There are still correctness issues with the WR codepath (see other stuff hanging off bug 1421380) but at least this will make the pref do something useful and allow (a) performance comparisons between the two codepaths and (b) testing the new codepath in automated tests.
Also for the record I compared the performance on MotionMark and MazeSolver with both codepaths and it's about the same in both cases. Slightly better with the WR hit-testing code on MazeSolver, but MotionMark is noisy enough that I can't say one way or another.
Comment on attachment 8932597 [details]
Bug 1389146 - Add code to dump the webrender display list, behind a compile guard.

https://reviewboard.mozilla.org/r/203656/#review209122
Attachment #8932597 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8932598 [details]
Bug 1389146 - Enable WR hit-testing when the pref is enabled.

https://reviewboard.mozilla.org/r/203658/#review209504
Attachment #8932598 - Flags: review?(botond) → review+
Comment on attachment 8932598 [details]
Bug 1389146 - Enable WR hit-testing when the pref is enabled.

https://reviewboard.mozilla.org/r/203658/#review209624
Attachment #8932598 - Flags: review?(mstange) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d2900c96fc29
Add code to dump the webrender display list, behind a compile guard. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/d3e60fe1db3a
Enable WR hit-testing when the pref is enabled. r=botond,mstange
https://hg.mozilla.org/mozilla-central/rev/d2900c96fc29
https://hg.mozilla.org/mozilla-central/rev/d3e60fe1db3a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
No longer blocks: 1410778
You need to log in before you can comment on or make changes to this bug.