Bug 1251638 - Don't clamp the displayport to the scrollable rect on the compositor side. r=botond, a=ritu
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 01 Apr 2016 08:02:25 -0400
changeset 323859 7c3e676329cdfd791a889c838ba79620dc794b17
parent 323858 5c1cf0a4ab8d75fea8c7bd41b7224f24fef807bd
child 323860 cc271ba87800178fe3b6b7c9981d814864815cc0
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, ritu
bugs1251638
milestone47.0a2
Bug 1251638 - Don't clamp the displayport to the scrollable rect on the compositor side. r=botond, a=ritu 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: JgBDi0M3Wtt
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
@@ -2791,18 +2791,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();
 }