Bug 1223639. Use ForceInside to constrain the displayport rect to the scrollable rect instead of intersect. r=botond
authorTimothy Nikkel <tnikkel@gmail.com>
Wed, 11 Nov 2015 16:38:24 -0600
changeset 308458 64a9ebe5f2b9f0b5971e958121db07735f14fe41
parent 308457 51dbf899ae40b9fdc9f8e5ba7712c3694656be60
child 308459 c5780f2a10e8de3171e12f080d99f62a0c6ca1a5
push id7470
push users.kaspari@gmail.com
push dateThu, 12 Nov 2015 12:51:02 +0000
reviewersbotond
bugs1223639, 1191539, 957668
milestone45.0a1
Bug 1223639. Use ForceInside to constrain the displayport rect to the scrollable rect instead of intersect. r=botond ForceInside shifts the rect first, and then clamps if needed. So the displayport doesn't get shrunk unnecessarily. Bug 1191539 fixed this bug by applying ForceInside to the screen rect of the display port, which happens before the incorrect Intersect call. It's better to remove the Intersect call and just do ForceInside once at the end to the final display port rect. Bug 957668 introduced this bug by using Intersect instead of ForceInside when copying the code from AsyncPanZoomController::CalculatePendingDisplayPort when creating the code that computed a displayport rect from displayport margins.
layout/base/nsLayoutUtils.cpp
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -983,20 +983,16 @@ GetDisplayPortFromMarginsData(nsIContent
     // coordinate space, so it doesn't include the local resolution.
     float localRes = presContext->PresShell()->GetResolution();
     parentRes.xScale /= localRes;
     parentRes.yScale /= localRes;
   }
   ScreenRect screenRect = LayoutDeviceRect::FromAppUnits(base, auPerDevPixel)
                         * parentRes;
 
-  nsRect expandedScrollableRect =
-    nsLayoutUtils::CalculateExpandedScrollableRect(frame);
-
-
   // Note on the correctness of applying the alignment in Screen space:
   //   The correct space to apply the alignment in would be Layer space, but
   //   we don't necessarily know the scale to convert to Layer space at this
   //   point because Layout may not yet have chosen the resolution at which to
   //   render (it chooses that in FrameLayerBuilder, but this can be called
   //   during display list building). Therefore, we perform the alignment in
   //   Screen space, which basically assumes that Layout chose to render at
   //   screen resolution; since this is what Layout does most of the time,
@@ -1074,31 +1070,26 @@ GetDisplayPortFromMarginsData(nsIContent
   screenRect += scrollPosScreen;
   float x = alignment.width * floor(screenRect.x / alignment.width);
   float y = alignment.height * floor(screenRect.y / alignment.height);
   float w = alignment.width * ceil(screenRect.width / alignment.width + 1);
   float h = alignment.height * ceil(screenRect.height / alignment.height + 1);
   screenRect = ScreenRect(x, y, w, h);
   screenRect -= scrollPosScreen;
 
-  ScreenRect screenExpScrollableRect =
-    LayoutDeviceRect::FromAppUnits(expandedScrollableRect,
-                                   auPerDevPixel) * res;
-
-  // Make sure the displayport remains within the scrollable rect.
-  screenRect = screenRect.ForceInside(screenExpScrollableRect - scrollPosScreen);
-
   // Convert the aligned rect back into app units.
   nsRect result = LayoutDeviceRect::ToAppUnits(screenRect / res, auPerDevPixel);
 
   // Expand it for the low-res buffer if needed
   result = ApplyRectMultiplier(result, aMultiplier);
 
-  // Finally, clamp it to the expanded scrollable rect.
-  result = expandedScrollableRect.Intersect(result + scrollPos) - scrollPos;
+  // Make sure the displayport remains within the scrollable rect.
+  nsRect expandedScrollableRect =
+    nsLayoutUtils::CalculateExpandedScrollableRect(frame);
+  result = result.ForceInside(expandedScrollableRect - scrollPos);
 
   return result;
 }
 
 static bool
 GetDisplayPortImpl(nsIContent* aContent, nsRect *aResult, float aMultiplier)
 {
   DisplayPortPropertyData* rectData =