Rebuild the hit-tester as part of the scene build. #2797
Conversation
|
(This is needed for bugzilla https://bugzilla.mozilla.org/show_bug.cgi?id=1465892) |
|
@bors-servo r+ |
|
📌 Commit 64ef4e5 has been approved by |
Rebuild the hit-tester as part of the scene build. In order for Gecko/APZ to get useful results from hit-testing, the hit-test tree needs to stay in sync with the scene and clip-scroll tree. This patch accomplishes this by rebuilding the hit-tester at the same time we rebuild or swap in these other structures, rather than at render time. r? @mrobinson <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2797) <!-- Reviewable:end -->
|
@bors-servo r- @kats raised a good point, which is that this change makes it so that the hit tester is not updated during scrolling. |
|
☀️ Test successful - status-appveyor, status-taskcluster |
|
So I spent more time thinking about this and realized that in addition to the previous problem (hit tester not getting updated during scrolling), there are other problems with this patch. Fundamentally, the problem I'm trying to solve here is that there is a period of time after a scene swap but before the next render where the hit_tester in WR has internal state that does not correspond to the APZ state. The original patch I had for this PR attempted to fix that by updating the hit-tester right after the scene swap. However, the "APZ state" incorporates not just the display list, but async scrolling offsets and scrollbar transforms. These are applied at here and here respectively, which are during the render phase. So the hit-tester really only has an internally-correct state if we update it after a render (which is what we're doing now), or if we extract those bits from the rendering step and have the ability to do them independently of a full render (which seems like it would be really convoluted). That leaves us with one of two possible solutions: (1) Start respecting the I'm leaning heavily towards approach (2). I had actually wanted to do this earlier but I ran into issues where Gecko would get confused as to how many frames were inflight and would get stuck always waiting for a non-existent frame. However I have a better solution now that seems to work. I'll put up the PR for that but if anybody prefers approach (1) I'm happy to discuss it further. (Edited for clarity) |
|
I also prefer (2). Looks good to me! |
|
☔ The latest upstream changes (presumably #2776) made this pull request unmergeable. Please resolve the merge conflicts. |
The render ensures that the hit-tester is rebuilt using the most recent display list (which is installed into gecko's APZ code at the same time as the scene swap), along with the corresponding async scroll offsets and scrollbar thumb transforms (which are sampled from gecko's APZ step and used to update the clip-scroll tree during a render). The render step is triggered via a new argument to update_document, because the existing generate_frame flag on the transaction has semantic meaning (in that it triggers a callback) that we don't want to interfere with. In addition, we can remove the obsolete `render_on_hittest` flag.
|
rebased ^ This is ready for proper review (in case it wasn't clear) |
|
@bors-servo r+ |
|
📌 Commit 91e81d1 has been approved by |
Rebuild the hit-tester as part of the scene build. In order for Gecko/APZ to get useful results from hit-testing, the hit-test tree needs to stay in sync with the scene and clip-scroll tree. This patch accomplishes this by rebuilding the hit-tester at the same time we rebuild or swap in these other structures, rather than at render time. r? @mrobinson <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2797) <!-- Reviewable:end -->
|
☀️ Test successful - status-appveyor, status-taskcluster |
In order for Gecko/APZ to get useful results from hit-testing, the
hit-test tree needs to stay in sync with the scene and clip-scroll tree.
This patch accomplishes this by rebuilding the hit-tester at the
same time we rebuild or swap in these other structures, rather than at
render time.
r? @mrobinson
This change is