Closed Bug 1109677 Opened 7 years ago Closed 7 years ago

Various APZ test failures when event-regions enabled

Categories

(Core :: Panning and Zooming, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files, 1 obsolete file)

Turning on event-regions causes the APZ gtests to fail (see for example https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d8f1c56b29a2). These need to be resolved before we can turn on event regions for non-B2G platforms.
Assignee: nobody → bugmail.mozilla
The new event-regions code ignores scrollinfo layers so I just converted all the scrollinfo layers in the tests to thebes/painted layers.
Attachment #8534616 - Flags: review?(botond)
Missed a hunk
Attachment #8534616 - Attachment is obsolete: true
Attachment #8534616 - Flags: review?(botond)
Attachment #8534620 - Flags: review?(botond)
Since the new code relies on the event regions and clip rect rather than the composition bounds and visible region we need to make sure we update those properties correctly.
Attachment #8534621 - Flags: review?(botond)
Attachment #8534613 - Flags: review?(botond) → review+
Attachment #8534620 - Flags: review?(botond) → review+
Comment on attachment 8534621 [details] [diff] [review]
Part 3 - Populate hit regions properly

Review of attachment 8534621 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/tests/gtest/TestAsyncPanZoomController.cpp
@@ +1690,5 @@
> +      nsIntRect scrollRect = LayerIntRect::ToUntyped(RoundedToInt(aScrollableRect * metrics.LayersPixelsPerCSSPixel()));
> +      er.mHitRegion = nsIntRegion(nsIntRect(layerBound.TopLeft(), scrollRect.Size()));
> +      aLayer->SetEventRegions(er);
> +      aLayer->SetClipRect(&layerBound);
> +    }

Why do we want the event regions to have the size of the scrollable rect?
The scrollable rect is a more accurate representation of the actual size of the layer. Using the analogy of the composition bounds being the "hole" through which you view the scrollable rect, the scrollable rect represents the bounds of the layer that is moving.

When we create the layer tree using CreateLayerTree, we pass in an array of visible-regions, and those populate the initial hit regions because there we assume the layers are non-scrollable. Once we call SetScrollableFrameMetrics we turn that visible region into the composition bounds (just above the hunk you referenced) and therefore the scrollable rect corresponds to the hit region.
Comment on attachment 8534621 [details] [diff] [review]
Part 3 - Populate hit regions properly

Review of attachment 8534621 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/tests/gtest/TestAsyncPanZoomController.cpp
@@ +1687,5 @@
>      aLayer->SetFrameMetrics(metrics);
> +    if (!aScrollableRect.IsEqualEdges(CSSRect(-1, -1, -1, -1))) {
> +      EventRegions er = aLayer->GetEventRegions();
> +      nsIntRect scrollRect = LayerIntRect::ToUntyped(RoundedToInt(aScrollableRect * metrics.LayersPixelsPerCSSPixel()));
> +      er.mHitRegion = nsIntRegion(nsIntRect(layerBound.TopLeft(), scrollRect.Size()));

Maybe add a comment here that we're trying to mimic how Layout calculates an event region?
Attachment #8534621 - Flags: review?(botond) → review+
https://hg.mozilla.org/mozilla-central/rev/c29cfb6929e5
https://hg.mozilla.org/mozilla-central/rev/86f93a3f1ef8
https://hg.mozilla.org/mozilla-central/rev/c64034bd2b1e
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.