Bug 1831148. Allow nsLayoutUtils::TransformFrameRectToAncestor to use full nscoord range. r=gfx-reviewers,lsalzman

The problem with the previous patch was that a rect spanning (-reallybig,reallybig) would get reduced to fit in an nscoord based (x,y,width,height) rect by clamping each of x, y, width, height individually, which would result in the clamped rect having origin nscoord_min and size nscoord_max so that it would span (nscoord_min,0), which means it does not intersect the positive axis at all, which in most cases is the only part of the coord system that is visible.

We already have a solution for this exact problem in the code base. nsLayoutUtils::RoundGfxRectToAppRect uses ConstrainToCoordValues which reduces the size of the rect equally on both ends so the above rect would span (nscoord_min/2,nscoord_max/2) and have origin nscoord_min/2 and size nscoord_max. However we can't use RoundGfxRectToAppRect here because it has different rounding behaviour (and we have tests that depend on it, so I assume web sites will too). So we create a new function that is like RoundGfxRectToAppRect but has the rounding behaviour that we have in the existing function here

I re-used David's existing test from the previous patch.

Differential Revision: https://phabricator.services.mozilla.com/D269158

Author: tnikkel

layout/base/nsLayoutUtils.cpp

@@ -2185,9 +2185,9 @@ static Rect TransformGfxRectToAncestor(
   }
   const nsIFrame* ancestor = aOutAncestor ? *aOutAncestor : aAncestor.mFrame;
   float factor = ancestor->PresContext()->AppUnitsPerDevPixel();
-  Rect maxBounds =
-      Rect(float(nscoord_MIN) / factor * 0.5, float(nscoord_MIN) / factor * 0.5,
-           float(nscoord_MAX) / factor, float(nscoord_MAX) / factor);
+  Rect maxBounds = Rect(
+      float(nscoord_MIN) / factor, float(nscoord_MIN) / factor,
+      float(nscoord_MAX) / factor * 2.0, float(nscoord_MAX) / factor * 2.0);
   return ctm.TransformAndClipBounds(aRect, maxBounds);
 }
 
@@ -2391,13 +2391,8 @@ nsRect nsLayoutUtils::TransformFrameRectToAncestor(
       aMatrixCache, aStopAtStackingContextAndDisplayPortAndOOFFrame,
       aOutAncestor);
 
-  float destAppUnitsPerDevPixel =
-      aAncestor.mFrame->PresContext()->AppUnitsPerDevPixel();
-  return nsRect(
-      NSFloatPixelsToAppUnits(float(result.x), destAppUnitsPerDevPixel),
-      NSFloatPixelsToAppUnits(float(result.y), destAppUnitsPerDevPixel),
-      NSFloatPixelsToAppUnits(float(result.width), destAppUnitsPerDevPixel),
-      NSFloatPixelsToAppUnits(float(result.height), destAppUnitsPerDevPixel));
+  return ScaleThenRoundGfxRectToAppRect(
+      result, aAncestor.mFrame->PresContext()->AppUnitsPerDevPixel());
 }
 
 LayoutDeviceIntPoint nsLayoutUtils::WidgetToWidgetOffset(nsIWidget* aFrom,

layout/base/nsLayoutUtils.h

@@ -1101,6 +1101,19 @@ class nsLayoutUtils {
   template <typename T>
   static nsRect RoundGfxRectToAppRect(const T& aRect, const float aFactor);
 
+  /**
+   * Like the above but slightly different scale and round behaviour. First
+   * scales, then constrains to nscoord, then rounds each component (x, y,
+   * width, height) individually.
+   *
+   * @param aRect The graphics rect to round outward.
+   * @param aFactor The number of app units per graphics unit.
+   * @return The rounaded rectangle in app space.
+   */
+  template <typename T>
+  static nsRect ScaleThenRoundGfxRectToAppRect(const T& aRect,
+                                               const float aFactor);
+
   /**
    * Returns a subrectangle of aContainedRect that is entirely inside the
    * rounded rect. Complex cases are handled conservatively by returning a
@@ -3274,6 +3287,41 @@ nsRect nsLayoutUtils::RoundGfxRectToAppRect(const T& aRect,
   return retval;
 }
 
+template <typename T>
+nsRect nsLayoutUtils::ScaleThenRoundGfxRectToAppRect(const T& aRect,
+                                                     const float aFactor) {
+  // Get a new Rect whose units are app units by scaling by the specified
+  // factor.
+  T scaledRect = aRect;
+  scaledRect.Scale(aFactor);
+
+  nsRect retval;
+
+  double start = double(scaledRect.x);
+  double size = double(scaledRect.width);
+  // We now need to constrain our results to the max and min values for coords.
+  ConstrainToCoordValues(start, size);
+  // ConstrainToCoordValues ensures converting to nscoord is safe.
+  retval.x = NSToCoordRoundWithClamp(start);
+  retval.width = NSToCoordRoundWithClamp(size);
+
+  start = double(scaledRect.y);
+  size = double(scaledRect.height);
+  ConstrainToCoordValues(start, size);
+  retval.y = NSToCoordRoundWithClamp(start);
+  retval.height = NSToCoordRoundWithClamp(size);
+
+  if (!aRect.Width()) {
+    retval.SetWidth(0);
+  }
+
+  if (!aRect.Height()) {
+    retval.SetHeight(0);
+  }
+
+  return retval;
+}
+
 namespace mozilla {
 
 /**

layout/generic/test/mochitest.toml

@@ -180,6 +180,8 @@ skip-if = ["!debug"] # Bug 1838577
 
 ["test_bug1803209.html"]
 
+["test_bug1831148.html"]
+
 ["test_bug1847329.html"]
 
 ["test_crash_on_mouse_move.html"]

layout/generic/test/test_bug1831148.html

@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+<style>
+#marker {
+  width: 1px;
+  height: 1px;
+}
+#outer {
+  height: 1000px;
+  background: #a0ffff;
+  overflow: scroll;
+}
+
+#inner {
+  height: 16593200px;
+  background: #ffa0a0;
+}
+
+.log {
+  white-space: pre;
+}
+</style>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1831148">Mozilla Bug 1831148</a>
+<div id="marker"></div>
+<div id="outer">
+  <div id="inner"></div>
+</div>
+<script>
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(() => {
+  const interval = 20;
+  const increment = outer.scrollHeight / interval;
+  const offset = marker.getBoundingClientRect().bottom;
+  for (let i = 0; i < interval; i++) {
+    outer.scrollTop = i * increment;
+    console.log(outer.scrollTop);
+    // Shift to account for viewport coordinate shift.
+    const bcrTop = -inner.getBoundingClientRect().top + offset;
+    // Floating point value diverges from scrollTop.
+    isfuzzy(bcrTop, outer.scrollTop, 1, "scrollTop and BCR top match");
+  }
+  SimpleTest.finish();
+});
+</script>