Closed Bug 1524229 Opened 6 years ago Closed 5 years ago

Hit-test pointer events against RefLayers (Fission on non-WR)

Categories

(Core :: Panning and Zooming, enhancement, P3)

enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1559259

People

(Reporter: hsivonen, Unassigned)

References

Details

(Whiteboard: [fission-event-backlog])

When doing APZ hit testing for a pointer (mouse/touch/stylus) event, find the deepest RefLayer that is hit (in addition to currently finding the deepest scrollable).

If there is such RefLayer (i.e. the event coordinates fall onto Web content and not onto chrome), attach the LayersId of the child of the RefLayer to the event using the new field from bug 1524226.

Multiply together the tranform matrix of RefLayer and the transform matrices of its ancestors and attach the resulting matrix to the event using the new field from bug 1524226.

It's worth noting that RefLayers (and layers in general) are specific to non-WebRender. WebRender has its own compositor hit testing implementation (we enter one or the other here [1]); we'll probably want to use a separate bug to track the equivalent work for WebRender hit testing.

(Jeff also suggested that it might make sense to do the WR implementation first, given the planned "soft" dependency of Fission on WR.)

[1] https://searchfox.org/mozilla-central/rev/e00ea598e52bbb35f8c45abf9c2eade17962bb5e/gfx/layers/apz/src/APZCTreeManager.cpp#2458

If layers in general are non-WR-specific, what happens to the process-top-level LayersId? Does WR still maintain LayersIds for compat or do we need something totally different under WR?:

Flags: needinfo?(botond)

WR still maintains layers IDs for compat. One of the things the WR hit test returns is a "pipeline ID", which is just a layers ID by another name (see [1]).

(What I said in bug 1524226 comment 4 still applies: the layers ID we get at [1] is not necessarily the one we want for Fission purposes. Rather, we'd need to modify the impementation of the wr->HitTest() call just above [2] to (also) give us the layers ID we want.)

[1] https://searchfox.org/mozilla-central/rev/03ebbdab952409640c6857d835d3040bf6f9e2db/gfx/layers/apz/src/APZCTreeManager.cpp#2511
[2] https://searchfox.org/mozilla-central/rev/03ebbdab952409640c6857d835d3040bf6f9e2db/gfx/layers/apz/src/APZCTreeManager.cpp#2505

Flags: needinfo?(botond)

(Had a response written up but Botond beat me to it... but I have a different take on the bug 1524226 comment 4 thing. Posting my full written response below, which is partly redundant now.)

We still use LayersId on the C++ side with WR, and they work the same as with non-WR. The roughly-equivalent notion inside WR is a PipelineId, and we have some conversion functions such as https://searchfox.org/mozilla-central/rev/03ebbdab952409640c6857d835d3040bf6f9e2db/gfx/webrender_bindings/WebRenderTypes.h#206 to map between the two.

In terms of hit-testing, when we use WR to do the hit-test, the pipeline id/layers id comes back at https://searchfox.org/mozilla-central/rev/490ab7f9b84570573a49d7fa018673ce0d5ddf22/gfx/layers/apz/src/APZCTreeManager.cpp#2511 and that's probably what we want to use. I think that the scenario described at https://bugzilla.mozilla.org/show_bug.cgi?id=1524226#c4 would not need any special handling for this bug (the scrollId on the next line should point to the nearest enclosing scrollframe, which will be outside the iframe).

Once the iframe goes OOP we'll likely get a NULL_SCROLL_ID for the scrollId and we'll need to do a walk up the tree to find the actual scroll target. But the LayersId should still be correct, and doing the walk up the tree is something we'll need to handle more generally as part of the APZ-fission handoff fixups.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)

I think that the scenario described at https://bugzilla.mozilla.org/show_bug.cgi?id=1524226#c4 would not need any special handling for this bug (the scrollId on the next line should point to the nearest enclosing scrollframe, which will be outside the iframe).

Once the iframe goes OOP we'll likely get a NULL_SCROLL_ID for the scrollId and we'll need to do a walk up the tree to find the actual scroll target. But the LayersId should still be correct, and doing the walk up the tree is something we'll need to handle more generally as part of the APZ-fission handoff fixups.

Ok, I see: the way things are currently implemented, wr->HitTest() will give us the right layers id for Fission, and not necessarily the right one for scrolling.

We have some options for how to fix that:

  • leave the impl of wr->HitTest() alone and use the hit-testing tree to get the right answer for scrolling; or
  • modify the impl of wr->HitTest() to give us both layers ids
Whiteboard: [fission-event-m1]

WebRender version of this bug is now bug 1528725.

Let's put this into a later milestone now that we're targeting WR for the first milestone.

Whiteboard: [fission-event-m1] → [fission-event-m2]

Moving to backlog, since we aren't supporting non-WR scenarios for now.

Whiteboard: [fission-event-m2] → [fission-event-backlog]
Summary: Hit-test pointer events against RefLayers → Hit-test pointer events against RefLayers (Fission on non-WR)

A newer bug has a patch, so duplicating forward.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.