Bug 1251638 - Don't clamp the displayport to the scrollable rect on the compositor side. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 30 Mar 2016 19:23:08 -0400
changeset 346102 e32336952fa45b2f820b6c388c85f1716782024d
parent 345720 494289c72ba3997183e7b5beaca3e0447ecaf96d
child 346103 09289c4a90eccf653b41723db409c6cfd57b579e
push id14242
push userkgupta@mozilla.com
push dateWed, 30 Mar 2016 23:23:26 +0000
reviewersbotond
bugs1251638
milestone48.0a1
Bug 1251638 - Don't clamp the displayport to the scrollable rect on the compositor side. r?botond The clamping already happens on the content side, in nsLayoutUtils::GetDisplayPort and friends. The clamping there is more accurate as it reflects the latest main- thread information about the size of the page and position of the displayport base rect, which the compositor thread does not have. Since we are not clamping the displayport on the compositor side, it can "slosh around" a bit more and ends up sending a few more repaint requests when scrolling near the edges of the scrollable frame. This causes some gtests to fail because of the "extra" repaint requests. Disabling the velocity bias removes the sloshing around and fixes the test failures. MozReview-Commit-ID: GmYRwtn0ncq
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/test/gtest/TestHitTesting.cpp
gfx/layers/apz/test/gtest/TestPanning.cpp
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2829,18 +2829,18 @@ const ScreenMargin AsyncPanZoomControlle
   float paintFactor = kDefaultEstimatedPaintDurationMs;
   CSSRect displayPort = CSSRect(scrollOffset + (velocity * paintFactor * gfxPrefs::APZVelocityBias()),
                                 displayPortSize);
 
   // Re-center the displayport based on its expansion over the composition size.
   displayPort.MoveBy((compositionSize.width - displayPort.width)/2.0f,
                      (compositionSize.height - displayPort.height)/2.0f);
 
-  // Make sure the displayport remains within the scrollable rect.
-  displayPort = displayPort.MoveInsideAndClamp(scrollableRect) - scrollOffset;
+  // Make the displayport relative to the scroll offset
+  displayPort = displayPort - scrollOffset;
 
   APZC_LOG_FM(aFrameMetrics,
     "Calculated displayport as (%f %f %f %f) from velocity %s paint time %f metrics",
     displayPort.x, displayPort.y, displayPort.width, displayPort.height,
     ToString(aVelocity).c_str(), paintFactor);
 
   CSSMargin cssMargins;
   cssMargins.left = -displayPort.x;
--- a/gfx/layers/apz/test/gtest/TestHitTesting.cpp
+++ b/gfx/layers/apz/test/gtest/TestHitTesting.cpp
@@ -157,16 +157,18 @@ TEST_F(APZHitTestingTester, HitTesting1)
   hit = GetTargetAPZC(ScreenPoint(-1000, 10));
   EXPECT_EQ(nullAPZC, hit.get());
   EXPECT_EQ(ScreenToParentLayerMatrix4x4(), transformToApzc);
   EXPECT_EQ(ParentLayerToScreenMatrix4x4(), transformToGecko);
 }
 
 // A more involved hit testing test that involves css and async transforms.
 TEST_F(APZHitTestingTester, HitTesting2) {
+  SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
+
   CreateHitTesting2LayerTree();
   ScopedLayerTreeRegistration registration(manager, 0, root, mcc);
 
   manager->UpdateHitTestingTree(nullptr, root, false, 0, 0);
 
   // At this point, the following holds (all coordinates in screen pixels):
   // layers[0] has content from (0,0)-(200,200), clipped by composition bounds (0,0)-(100,100)
   // layers[1] has content from (10,10)-(90,90), clipped by composition bounds (10,10)-(50,50)
--- a/gfx/layers/apz/test/gtest/TestPanning.cpp
+++ b/gfx/layers/apz/test/gtest/TestPanning.cpp
@@ -75,45 +75,50 @@ protected:
     EXPECT_EQ(AsyncTransform(), viewTransformOut);
 
     apzc->AssertStateIsReset();
   }
 };
 
 TEST_F(APZCPanningTester, Pan) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, false);
+  SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
   DoPanTest(true, true, mozilla::layers::AllowedTouchBehavior::NONE);
 }
 
 // In the each of the following 4 pan tests we are performing two pan gestures: vertical pan from top
 // to bottom and back - from bottom to top.
 // According to the pointer-events/touch-action spec AUTO and PAN_Y touch-action values allow vertical
 // scrolling while NONE and PAN_X forbid it. The first parameter of DoPanTest method specifies this
 // behavior.
 // However, the events will be marked as consumed even if the behavior in PAN_X, because the user could
 // move their finger horizontally too - APZ has no way of knowing beforehand and so must consume the
 // events.
 TEST_F(APZCPanningTester, PanWithTouchActionAuto) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, true);
+  SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
   DoPanTest(true, true, mozilla::layers::AllowedTouchBehavior::HORIZONTAL_PAN
                       | mozilla::layers::AllowedTouchBehavior::VERTICAL_PAN);
 }
 
 TEST_F(APZCPanningTester, PanWithTouchActionNone) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, true);
+  SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
   DoPanTest(false, false, 0);
 }
 
 TEST_F(APZCPanningTester, PanWithTouchActionPanX) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, true);
+  SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
   DoPanTest(false, true, mozilla::layers::AllowedTouchBehavior::HORIZONTAL_PAN);
 }
 
 TEST_F(APZCPanningTester, PanWithTouchActionPanY) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, true);
+  SCOPED_GFX_PREF(APZVelocityBias, float, 0.0); // Velocity bias can cause extra repaint requests
   DoPanTest(true, true, mozilla::layers::AllowedTouchBehavior::VERTICAL_PAN);
 }
 
 TEST_F(APZCPanningTester, PanWithPreventDefaultAndTouchAction) {
   SCOPED_GFX_PREF(TouchActionEnabled, bool, true);
   DoPanWithPreventDefaultTest();
 }