commit 75370b03ee98d51ae3b18e0852a36e657204aa58
parent 1667ec582b66ed73989fdbc210e69b580b3e04a3
Author: Timothy Nikkel <tnikkel@gmail.com>
Date: Thu, 23 Oct 2025 08:03:20 +0000
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
Diffstat:
4 files changed, 101 insertions(+), 10 deletions(-)
diff --git a/layout/base/nsLayoutUtils.cpp b/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,
diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h
@@ -1102,6 +1102,19 @@ class nsLayoutUtils {
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
* smaller rect than necessary.
@@ -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 {
/**
diff --git a/layout/generic/test/mochitest.toml b/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"]
diff --git a/layout/generic/test/test_bug1831148.html b/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>