commit 9a48b65e12256d0428b8f8aeab736dfa5985691f
parent 77ee7bde3d3139025350f36b44eafac283d87c0a
Author: Timothy Nikkel <tnikkel@gmail.com>
Date: Wed, 22 Oct 2025 01:19:39 +0000
Bug 1995177. Refactor and improve nsLayoutUtils::ConstrainToCoordValues. r=gfx-reviewers,lsalzman
This patch does a few things at once, it was hard to separate it out into smaller patches.
Combines two functions that are almost the same into one function.
Stops using std::clamp because how it handles NaN is undefined (this can occur).
Keeps the assert that size is non-negative that was only in one version (the next patch in the series handles the one user of this function that can pass in negative).
Allows for NaN with the "size is non-negative" assert (this can occur).
So this function should fully handle all cases with NaN. Added a comment to that effect.
Forces the inputs/outputs to be double. Because nscoord_max can not be represented exactly as a float (so we'd return float(nscoord_max) which we would then convert to nscoord and it would not have the value nscoord_max).
Adapts the only user of this function to the new interface.
Differential Revision: https://phabricator.services.mozilla.com/D269155
Diffstat:
2 files changed, 41 insertions(+), 47 deletions(-)
diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp
@@ -1670,44 +1670,13 @@ void nsLayoutUtils::GetContainerAndOffsetAtEvent(PresShell* aPresShell,
}
}
-void nsLayoutUtils::ConstrainToCoordValues(float& aStart, float& aSize) {
- MOZ_ASSERT(aSize >= 0);
-
- // Here we try to make sure that the resulting nsRect will continue to cover
- // as much of the area that was covered by the original gfx Rect as possible.
-
- // We clamp the bounds of the rect to {nscoord_MIN,nscoord_MAX} since
- // nsRect::X/Y() and nsRect::XMost/YMost() can't return values outwith this
- // range:
- float end = aStart + aSize;
- aStart = std::clamp(aStart, float(nscoord_MIN), float(nscoord_MAX));
- end = std::clamp(end, float(nscoord_MIN), float(nscoord_MAX));
-
- aSize = end - aStart;
-
- // We must also clamp aSize to {0,nscoord_MAX} since nsRect::Width/Height()
- // can't return a value greater than nscoord_MAX. If aSize is greater than
- // nscoord_MAX then we reduce it to nscoord_MAX while keeping the rect
- // centered:
- if (MOZ_UNLIKELY(std::isnan(aSize))) {
- // Can happen if aStart is -inf and aSize is +inf for example.
- aStart = 0.0f;
- aSize = float(nscoord_MAX);
- } else if (aSize > float(nscoord_MAX)) {
- float excess = aSize - float(nscoord_MAX);
- excess /= 2;
- aStart += excess;
- aSize = float(nscoord_MAX);
- }
-}
-
/**
- * Given a gfxFloat, constrains its value to be between nscoord_MIN and
- * nscoord_MAX.
+ * Given a floating point value, constrains its value to be between nscoord_MIN
+ * and nscoord_MAX.
*
* @param aVal The value to constrain (in/out)
*/
-static void ConstrainToCoordValues(gfxFloat& aVal) {
+static void ConstrainToCoordValues(double& aVal) {
if (aVal <= nscoord_MIN) {
aVal = nscoord_MIN;
} else if (aVal >= nscoord_MAX) {
@@ -1715,28 +1684,42 @@ static void ConstrainToCoordValues(gfxFloat& aVal) {
}
}
-void nsLayoutUtils::ConstrainToCoordValues(gfxFloat& aStart, gfxFloat& aSize) {
- gfxFloat max = aStart + aSize;
+/* static */ void nsLayoutUtils::ConstrainToCoordValues(double& aStart,
+ double& aSize) {
+ MOZ_ASSERT(std::isnan(aSize) || aSize >= 0);
+
+ double max = aStart + aSize;
// Clamp the end points to within nscoord range
::ConstrainToCoordValues(aStart);
::ConstrainToCoordValues(max);
+ // Here we try to make sure that the resulting nsRect will continue to cover
+ // as much of the area that was covered by the original gfx Rect as possible.
+
+ // We must also clamp aSize to {0,nscoord_MAX} since nsRect::Width/Height()
+ // can't return a value greater than nscoord_MAX. If aSize is greater than
+ // nscoord_MAX then we reduce it to nscoord_MAX while keeping the rect
+ // centered:
+
aSize = max - aStart;
// If the width if still greater than the max nscoord, then bring both
// endpoints in by the same amount until it fits.
if (MOZ_UNLIKELY(std::isnan(aSize))) {
// Can happen if aStart is -inf and aSize is +inf for example.
+ // If either aStart or aSize is NaN on entry to this function then the
+ // calculations above this will make the other one NaN too, so this check
+ // ensures that we do not return NaN for either value.
aStart = 0.0f;
aSize = nscoord_MAX;
} else if (aSize > nscoord_MAX) {
- gfxFloat excess = aSize - nscoord_MAX;
+ double excess = aSize - nscoord_MAX;
excess /= 2;
aStart += excess;
aSize = nscoord_MAX;
} else if (aSize < nscoord_MIN) {
- gfxFloat excess = aSize - nscoord_MIN;
+ double excess = aSize - nscoord_MIN;
excess /= 2;
aStart -= excess;
diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h
@@ -3193,8 +3193,9 @@ class nsLayoutUtils {
static bool IsAPZTestLoggingEnabled();
- static void ConstrainToCoordValues(gfxFloat& aStart, gfxFloat& aSize);
- static void ConstrainToCoordValues(float& aStart, float& aSize);
+ // doubles only because nscoord_max and nscoord_min cannot be represented
+ // exactly by floats.
+ static void ConstrainToCoordValues(double& aStart, double& aSize);
};
MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(nsLayoutUtils::PaintFrameFlags)
@@ -3247,21 +3248,31 @@ nsRect nsLayoutUtils::RoundGfxRectToAppRect(const T& aRect,
T scaledRect = aRect;
scaledRect.ScaleRoundOut(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(scaledRect.x, scaledRect.width);
- ConstrainToCoordValues(scaledRect.y, scaledRect.height);
+ ConstrainToCoordValues(start, size);
+ // ConstrainToCoordValues ensures casting to nscoord is safe.
+ retval.x = nscoord(start);
+ retval.width = nscoord(size);
+
+ start = double(scaledRect.y);
+ size = double(scaledRect.height);
+ ConstrainToCoordValues(start, size);
+ retval.y = nscoord(start);
+ retval.height = nscoord(size);
if (!aRect.Width()) {
- scaledRect.SetWidth(0);
+ retval.SetWidth(0);
}
if (!aRect.Height()) {
- scaledRect.SetHeight(0);
+ retval.SetHeight(0);
}
- // Now typecast everything back. This is guaranteed to be safe.
- return nsRect(nscoord(scaledRect.X()), nscoord(scaledRect.Y()),
- nscoord(scaledRect.Width()), nscoord(scaledRect.Height()));
+ return retval;
}
namespace mozilla {
