Open Bug 1269537 Opened 9 years ago Updated 2 years ago

Treatment of clip rects during compositor hit testing is incorrect

Categories

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

defect

Tracking

()

Tracking Status
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- wontfix

People

(Reporter: botond, Unassigned)

References

Details

(Keywords: correctness, Whiteboard: [gfx-noted])

Bug 1257288 improved the treatment of clip rects during compositor hit testing, but it's still incorrect. Compositor hit testing currently considers Layer::GetClipRect() to apply on top of the async transform of the bottommost ScrollMetadata of the layer (or on top of the layer's CSS transform if there is no scroll metadata). Where Layer::GetClipRect() rect should actually apply is: 1) If the layer is not fixed, then on top of the layer's CSS transform, but below any async transforms. 2) If the layer is fixed, then on top of the async transform of the scroll frame that the layer is fixed with respect to. After bug 1267438 is fixed, layers will also have a scroll clip (distinct from the scroll clips in the scroll metadata). That should unconditionally apply on top of the layer's CSS transform, below any async transforms.
(In reply to Botond Ballo [:botond] from comment #0) > Where Layer::GetClipRect() rect should actually apply is: > > 1) If the layer is not fixed, then on top of the layer's CSS transform, > but below any async transforms. This has been broken since bug 1148582 introduces scroll clips. > 2) If the layer is fixed, then on top of the async transform of the scroll > frame that the layer is fixed with respect to. This has been broken even before that, possibly always.
(In reply to Botond Ballo [:botond] from comment #1) > (In reply to Botond Ballo [:botond] from comment #0) > > Where Layer::GetClipRect() rect should actually apply is: > > > > ... > > > > 2) If the layer is fixed, then on top of the async transform of the scroll > > frame that the layer is fixed with respect to. > > This has been broken even before that, possibly always. Except, of course, that if the layer is fixed with respect to its bottommost metrics, then the two mistakes (applying it on top of the bottommost metrics rather than below, and not having special handling for fixed layers) cancel each other out, and accidentally produce the correct outcome :)
I spent some time trying to construct test cases that expose these bugs. It's fairly straightforward to do so using an artificial layer tree in an APZ gtest, but I'm not sure how realistic those layer trees would be - I would prefer to start from HTML testcases, and then use a layer tree structure similar to the one in the testcases, so that the layer tree structure is realistic. However, creating HTML testcases that reproduce these issues is tricky due to their transient nature (unlike async-scrolling reftests, to trigger a hit-testing bug you need to send an event, such as a scrolling the wheel, while an async transform is in effect). As I think this bug is low-priority to begin with (again, due to the transient nature of the bug and requiring the user to be generating an input event during that transient period), I think it makes more sense to spend time on higher-priority bugs. We can revisit this and bump up its urgency if we come across a real-life testcase where this shows up.
Assignee: botond → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.