Issue #1619 - Convert Intrinsic Ratio to Float

https://bugzilla.mozilla.org/show_bug.cgi?id=1547792

Aspect Ratio handling simplified by using floating point integers:
- Multiplication of value (or inverse value) to a known side for Scaling
- No unequal equal values such as "4/3" vs "8/6" vs "20/15"
- Truly "Empty" aspect ratios, even if one dimension is not 0

4 files changed, 86 insertions, 84 deletions

diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp
index 9a827546f..e6e3b5f6f 100644
--- a/layout/base/nsCSSRendering.cpp
+++ b/layout/base/nsCSSRendering.cpp
@@ -3546,10 +3546,10 @@ ComputeDrawnSizeForBackground(const CSSSizeOrRatio& aIntrinsicSize,
     imageSize.width = ComputeRoundedSize(imageSize.width, aBgPositioningArea.width);
     if (!isRepeatRoundInBothDimensions &&
         aLayerSize.mHeightType == nsStyleImageLayers::Size::DimensionType::eAuto) {
-      // Restore intrinsic rato
-      if (aIntrinsicSize.mRatio.width) {
-        float scale = float(aIntrinsicSize.mRatio.height) / aIntrinsicSize.mRatio.width;
-        imageSize.height = NSCoordSaturatingNonnegativeMultiply(imageSize.width, scale);
+      // Restore intrinsic ratio
+      if (aIntrinsicSize.mRatio) {
+        imageSize.height =
+            aIntrinsicSize.mRatio.Inverted().ApplyTo(imageSize.width);
       }
     }
   }
@@ -3560,10 +3560,9 @@ ComputeDrawnSizeForBackground(const CSSSizeOrRatio& aIntrinsicSize,
     imageSize.height = ComputeRoundedSize(imageSize.height, aBgPositioningArea.height);
     if (!isRepeatRoundInBothDimensions &&
         aLayerSize.mWidthType == nsStyleImageLayers::Size::DimensionType::eAuto) {
-      // Restore intrinsic rato
-      if (aIntrinsicSize.mRatio.height) {
-        float scale = float(aIntrinsicSize.mRatio.width) / aIntrinsicSize.mRatio.height;
-        imageSize.width = NSCoordSaturatingNonnegativeMultiply(imageSize.height, scale);
+      // Restore intrinsic ratio
+      if (aIntrinsicSize.mRatio) {
+        imageSize.width = aIntrinsicSize.mRatio.ApplyTo(imageSize.height);
       }
     }
   }
@@ -5264,17 +5263,11 @@ CSSSizeOrRatio::ComputeConcreteSize() const
     return nsSize(mWidth, mHeight);
   }
   if (mHasWidth) {
-    nscoord height = NSCoordSaturatingNonnegativeMultiply(
-      mWidth,
-      double(mRatio.height) / mRatio.width);
-    return nsSize(mWidth, height);
+    return nsSize(mWidth, mRatio.Inverted().ApplyTo(mWidth));
   }
 
   MOZ_ASSERT(mHasHeight);
-  nscoord width = NSCoordSaturatingNonnegativeMultiply(
-    mHeight,
-    double(mRatio.width) / mRatio.height);
-  return nsSize(width, mHeight);
+  return nsSize(mRatio.ApplyTo(mHeight), mHeight);
 }
 
 CSSSizeOrRatio
@@ -5379,9 +5372,7 @@ nsImageRenderer::ComputeConcreteSize(const CSSSizeOrRatio& aSpecifiedSize,
   if (aSpecifiedSize.mHasWidth) {
     nscoord height;
     if (aIntrinsicSize.HasRatio()) {
-      height = NSCoordSaturatingNonnegativeMultiply(
-        aSpecifiedSize.mWidth,
-        double(aIntrinsicSize.mRatio.height) / aIntrinsicSize.mRatio.width);
+      height = aIntrinsicSize.mRatio.Inverted().ApplyTo(aSpecifiedSize.mWidth);
     } else if (aIntrinsicSize.mHasHeight) {
       height = aIntrinsicSize.mHeight;
     } else {
@@ -5393,9 +5384,7 @@ nsImageRenderer::ComputeConcreteSize(const CSSSizeOrRatio& aSpecifiedSize,
   MOZ_ASSERT(aSpecifiedSize.mHasHeight);
   nscoord width;
   if (aIntrinsicSize.HasRatio()) {
-    width = NSCoordSaturatingNonnegativeMultiply(
-      aSpecifiedSize.mHeight,
-      double(aIntrinsicSize.mRatio.width) / aIntrinsicSize.mRatio.height);
+    width = aIntrinsicSize.mRatio.ApplyTo(aSpecifiedSize.mHeight);
   } else if (aIntrinsicSize.mHasWidth) {
     width = aIntrinsicSize.mWidth;
   } else {
@@ -5406,32 +5395,57 @@ nsImageRenderer::ComputeConcreteSize(const CSSSizeOrRatio& aSpecifiedSize,
 
 /* static */ nsSize
 nsImageRenderer::ComputeConstrainedSize(const nsSize& aConstrainingSize,
-                                        const nsSize& aIntrinsicRatio,
+                                        const AspectRatio& aIntrinsicRatio,
                                         FitType aFitType)
 {
-  if (aIntrinsicRatio.width <= 0 && aIntrinsicRatio.height <= 0) {
+  if (!aIntrinsicRatio) {
     return aConstrainingSize;
   }
 
-  float scaleX = double(aConstrainingSize.width) / aIntrinsicRatio.width;
-  float scaleY = double(aConstrainingSize.height) / aIntrinsicRatio.height;
+  // Suppose we're doing a "contain" fit. If the image's aspect ratio has a
+  // "fatter" shape than the constraint area, then we need to use the
+  // constraint area's full width, and we need to use the aspect ratio to
+  // produce a height. ON the other hand, if the aspect ratio is "skinnier", we
+  // use the constraint area's full height, and we use the aspect ratio to
+  // produce a width. (If instead we're doing a "cover" fit, then it can easily
+  // be seen that we should do precisely the opposite.)
+  //
+  // This is equivalent to the more descriptive alternative:
+  //
+  //   AspectRatio::FromSize(aConstrainingSize) < aIntrinsicRatio
+  //
+  // But gracefully handling the case where one of the two dimensions from
+  // aConstrainingSize is zero. This is easy to prove since:
+  //
+  //   aConstrainingSize.width / aConstrainingSize.height < aIntrinsicRatio
+  //
+  // Is trivially equivalent to:
+  //
+  //   aIntrinsicRatio.width < aIntrinsicRatio * aConstrainingSize.height
+  //
+  // For the cases where height is not zero.
+  //
+  // We use float math here to avoid losing precision for very lareg backgrounds
+  // since we use saturating nscoord math otherwise.
+  const float constraintWidth = float(aConstrainingSize.width);
+  const float hypotheticalWidth =
+      aIntrinsicRatio.ApplyToFloat(aConstrainingSize.height);
+
   nsSize size;
-  if ((aFitType == CONTAIN) == (scaleX < scaleY)) {
+  if ((aFitType == CONTAIN) == (constraintWidth < hypotheticalWidth)) {
     size.width = aConstrainingSize.width;
-    size.height = NSCoordSaturatingNonnegativeMultiply(
-                    aIntrinsicRatio.height, scaleX);
+    size.height = aIntrinsicRatio.Inverted().ApplyTo(aConstrainingSize.width);
     // If we're reducing the size by less than one css pixel, then just use the
     // constraining size.
     if (aFitType == CONTAIN && aConstrainingSize.height - size.height < nsPresContext::AppUnitsPerCSSPixel()) {
       size.height = aConstrainingSize.height;
     }
   } else {
-    size.width = NSCoordSaturatingNonnegativeMultiply(
-                   aIntrinsicRatio.width, scaleY);
+    size.height = aConstrainingSize.height;
+    size.width = aIntrinsicRatio.ApplyTo(aConstrainingSize.height);
     if (aFitType == CONTAIN && aConstrainingSize.width - size.width < nsPresContext::AppUnitsPerCSSPixel()) {
       size.width = aConstrainingSize.width;
     }
-    size.height = aConstrainingSize.height;
   }
   return size;
 }
diff --git a/layout/base/nsCSSRendering.h b/layout/base/nsCSSRendering.h
index 791e9656e..8ebeb7537 100644
--- a/layout/base/nsCSSRendering.h
+++ b/layout/base/nsCSSRendering.h
@@ -17,6 +17,7 @@
 #include "nsLayoutUtils.h"
 #include "nsStyleStruct.h"
 #include "nsIFrame.h"
+#include "mozilla/AspectRatio.h"
 
 class gfxDrawable;
 class nsStyleContext;
@@ -41,8 +42,7 @@ class ImageContainer;
 struct CSSSizeOrRatio
 {
   CSSSizeOrRatio()
-    : mRatio(0, 0)
-    , mHasWidth(false)
+    : mHasWidth(false)
     , mHasHeight(false) {}
 
   bool CanComputeConcreteSize() const
@@ -50,12 +50,12 @@ struct CSSSizeOrRatio
     return mHasWidth + mHasHeight + HasRatio() >= 2;
   }
   bool IsConcrete() const { return mHasWidth && mHasHeight; }
-  bool HasRatio() const { return mRatio.width > 0 && mRatio.height > 0; }
+  bool HasRatio() const { return !!mRatio; }
   bool IsEmpty() const
   {
     return (mHasWidth && mWidth <= 0) ||
            (mHasHeight && mHeight <= 0) ||
-           mRatio.width <= 0 || mRatio.height <= 0; 
+           !mRatio; 
   }
 
   // CanComputeConcreteSize must return true when ComputeConcreteSize is
@@ -67,7 +67,7 @@ struct CSSSizeOrRatio
     mWidth = aWidth;
     mHasWidth = true;
     if (mHasHeight) {
-      mRatio = nsSize(mWidth, mHeight);
+      mRatio = AspectRatio::FromSize(mWidth, mHeight);
     }
   }
   void SetHeight(nscoord aHeight)
@@ -75,7 +75,7 @@ struct CSSSizeOrRatio
     mHeight = aHeight;
     mHasHeight = true;
     if (mHasWidth) {
-      mRatio = nsSize(mWidth, mHeight);
+      mRatio = AspectRatio::FromSize(mWidth, mHeight);
     }
   }
   void SetSize(const nsSize& aSize)
@@ -84,16 +84,16 @@ struct CSSSizeOrRatio
     mHeight = aSize.height;
     mHasWidth = true;
     mHasHeight = true;
-    mRatio = aSize;    
+    mRatio = AspectRatio::FromSize(mWidth, mHeight);    
   }
-  void SetRatio(const nsSize& aRatio)
+  void SetRatio(const AspectRatio& aRatio)
   {
     MOZ_ASSERT(!mHasWidth || !mHasHeight,
                "Probably shouldn't be setting a ratio if we have a concrete size");
     mRatio = aRatio;
   }
 
-  nsSize mRatio;
+  AspectRatio mRatio;
   nscoord mWidth;
   nscoord mHeight;
   bool mHasWidth;
@@ -184,11 +184,9 @@ public:
   /**
    * Compute the size of the rendered image using either the 'cover' or
    * 'contain' constraints (aFitType).
-   * aIntrinsicRatio may be an invalid ratio, that is one or both of its
-   * dimensions can be less than or equal to zero.
    */
   static nsSize ComputeConstrainedSize(const nsSize& aConstrainingSize,
-                                       const nsSize& aIntrinsicRatio,
+                                       const mozilla::AspectRatio& aIntrinsicRatio,
                                        FitType aFitType);
   /**
    * Compute the size of the rendered image (the concrete size) where no cover/
diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp
index fdcdcac78..74dbd63e6 100644
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -3966,7 +3966,7 @@ nsLayoutUtils::GetTextShadowRectsUnion(const nsRect& aTextAndDecorationsRect,
 enum ObjectDimensionType { eWidth, eHeight };
 static nscoord
 ComputeMissingDimension(const nsSize& aDefaultObjectSize,
-                        const nsSize& aIntrinsicRatio,
+                        const AspectRatio& aIntrinsicRatio,
                         const Maybe<nscoord>& aSpecifiedWidth,
                         const Maybe<nscoord>& aSpecifiedHeight,
                         ObjectDimensionType aDimensionToCompute)
@@ -3977,21 +3977,15 @@ ComputeMissingDimension(const nsSize& aDefaultObjectSize,
   // 1. "If the object has an intrinsic aspect ratio, the missing dimension of
   //     the concrete object size is calculated using the intrinsic aspect
   //     ratio and the present dimension."
-  if (aIntrinsicRatio.width > 0 && aIntrinsicRatio.height > 0) {
+  if (aIntrinsicRatio) {
     // Fill in the missing dimension using the intrinsic aspect ratio.
-    nscoord knownDimensionSize;
-    float ratio;
     if (aDimensionToCompute == eWidth) {
-      knownDimensionSize = *aSpecifiedHeight;
-      ratio = aIntrinsicRatio.width / aIntrinsicRatio.height;
-    } else {
-      knownDimensionSize = *aSpecifiedWidth;
-      ratio = aIntrinsicRatio.height / aIntrinsicRatio.width;
+      return aIntrinsicRatio.ApplyTo(*aSpecifiedHeight);
     }
-    return NSCoordSaturatingNonnegativeMultiply(knownDimensionSize, ratio);
+    return aIntrinsicRatio.Inverted().ApplyTo(*aSpecifiedWidth);
   }
 
-  // 2. "Otherwise, if the missing dimension is present in the object’s
+  // 2. "Otherwise, if the missing dimension is present in the object's
   //     intrinsic dimensions, [...]"
   // NOTE: *Skipping* this case, because we already know it's not true -- we're
   // in this function because the missing dimension is *not* present in
@@ -4029,7 +4023,7 @@ ComputeMissingDimension(const nsSize& aDefaultObjectSize,
 static Maybe<nsSize>
 MaybeComputeObjectFitNoneSize(const nsSize& aDefaultObjectSize,
                               const IntrinsicSize& aIntrinsicSize,
-                              const nsSize& aIntrinsicRatio)
+                              const AspectRatio& aIntrinsicRatio)
 {
   // "If the object has an intrinsic height or width, its size is resolved as
   // if its intrinsic dimensions were given as the specified size."
@@ -4076,15 +4070,13 @@ MaybeComputeObjectFitNoneSize(const nsSize& aDefaultObjectSize,
 static nsSize
 ComputeConcreteObjectSize(const nsSize& aConstraintSize,
                           const IntrinsicSize& aIntrinsicSize,
-                          const nsSize& aIntrinsicRatio,
+                          const AspectRatio& aIntrinsicRatio,
                           uint8_t aObjectFit)
 {
   // Handle default behavior (filling the container) w/ fast early return.
   // (Also: if there's no valid intrinsic ratio, then we have the "fill"
   // behavior & just use the constraint size.)
-  if (MOZ_LIKELY(aObjectFit == NS_STYLE_OBJECT_FIT_FILL) ||
-      aIntrinsicRatio.width == 0 ||
-      aIntrinsicRatio.height == 0) {
+  if (MOZ_LIKELY(aObjectFit == NS_STYLE_OBJECT_FIT_FILL) || !aIntrinsicRatio) {
     return aConstraintSize;
   }
 
@@ -4171,7 +4163,7 @@ HasInitialObjectFitAndPosition(const nsStylePosition* aStylePos)
 /* static */ nsRect
 nsLayoutUtils::ComputeObjectDestRect(const nsRect& aConstraintRect,
                                      const IntrinsicSize& aIntrinsicSize,
-                                     const nsSize& aIntrinsicRatio,
+                                     const AspectRatio& aIntrinsicRatio,
                                      const nsStylePosition* aStylePos,
                                      nsPoint* aAnchorPoint)
 {
@@ -5092,10 +5084,12 @@ nsLayoutUtils::IntrinsicForAxis(PhysicalAxis              aAxis,
            styleMinBSize.GetCoordValue() == 0)) ||
         styleMaxBSize.GetUnit() != eStyleUnit_None) {
 
-      nsSize ratio(aFrame->GetIntrinsicRatio());
-      nscoord ratioISize = (horizontalAxis ? ratio.width  : ratio.height);
-      nscoord ratioBSize = (horizontalAxis ? ratio.height : ratio.width);
-      if (ratioBSize != 0) {
+      AspectRatio ratio = aFrame->GetIntrinsicRatio();
+      if (ratio) {
+        // Convert 'ratio' if necessary, so that it's storing ISize/BSize:
+        if (!horizontalAxis) {
+          ratio = ratio.Inverted();
+        }
         AddStateBitToAncestors(aFrame,
             NS_FRAME_DESCENDANT_INTRINSIC_ISIZE_DEPENDS_ON_BSIZE);
 
@@ -5111,14 +5105,14 @@ nsLayoutUtils::IntrinsicForAxis(PhysicalAxis              aAxis,
             (aPercentageBasis.isNothing() &&
              GetPercentBSize(styleBSize, aFrame, horizontalAxis, h))) {
           h = std::max(0, h - bSizeTakenByBoxSizing);
-          result = NSCoordMulDiv(h, ratioISize, ratioBSize);
+          result = ratio.ApplyTo(h);
         }
 
         if (GetDefiniteSize(styleMaxBSize, aFrame, !isInlineAxis, aPercentageBasis, &h) ||
             (aPercentageBasis.isNothing() &&
              GetPercentBSize(styleMaxBSize, aFrame, horizontalAxis, h))) {
           h = std::max(0, h - bSizeTakenByBoxSizing);
-          nscoord maxISize = NSCoordMulDiv(h, ratioISize, ratioBSize);
+          nscoord maxISize = ratio.ApplyTo(h);
           if (maxISize < result) {
             result = maxISize;
           }
@@ -5131,7 +5125,7 @@ nsLayoutUtils::IntrinsicForAxis(PhysicalAxis              aAxis,
             (aPercentageBasis.isNothing() &&
              GetPercentBSize(styleMinBSize, aFrame, horizontalAxis, h))) {
           h = std::max(0, h - bSizeTakenByBoxSizing);
-          nscoord minISize = NSCoordMulDiv(h, ratioISize, ratioBSize);
+          nscoord minISize = ratio.ApplyTo(h);
           if (minISize > result) {
             result = minISize;
           }
@@ -6594,12 +6588,12 @@ nsLayoutUtils::DrawSingleImage(gfxContext&            aContext,
 /* static */ void
 nsLayoutUtils::ComputeSizeForDrawing(imgIContainer *aImage,
                                      CSSIntSize&    aImageSize, /*outparam*/
-                                     nsSize&        aIntrinsicRatio, /*outparam*/
+                                     AspectRatio&   aIntrinsicRatio, /*outparam*/
                                      bool&          aGotWidth,  /*outparam*/
                                      bool&          aGotHeight  /*outparam*/)
 {
   aGotWidth  = NS_SUCCEEDED(aImage->GetWidth(&aImageSize.width));
-  aGotHeight = NS_SUCCEEDED(aImage->GetHeight(&aImageSize.height));
+  aGotHeight = NS_SUCCEEDED(aImage->GetHeight(&aImageSize.height)); 
   bool gotRatio = NS_SUCCEEDED(aImage->GetIntrinsicRatio(&aIntrinsicRatio));
 
   if (!(aGotWidth && aGotHeight) && !gotRatio) {
@@ -6607,7 +6601,7 @@ nsLayoutUtils::ComputeSizeForDrawing(imgIContainer *aImage,
     // decoded) and should return zero size.
     aGotWidth = aGotHeight = true;
     aImageSize = CSSIntSize(0, 0);
-    aIntrinsicRatio = nsSize(0, 0);
+    aIntrinsicRatio = AspectRatio();
   }
 }
 
@@ -6616,7 +6610,7 @@ nsLayoutUtils::ComputeSizeForDrawingWithFallback(imgIContainer* aImage,
                                                  const nsSize&  aFallbackSize)
 {
   CSSIntSize imageSize;
-  nsSize imageRatio;
+  AspectRatio imageRatio;
   bool gotHeight, gotWidth;
   ComputeSizeForDrawing(aImage, imageSize, imageRatio, gotWidth, gotHeight);
 
@@ -6624,19 +6618,13 @@ nsLayoutUtils::ComputeSizeForDrawingWithFallback(imgIContainer* aImage,
   // intrinsic ratio of the image.
   if (gotWidth != gotHeight) {
     if (!gotWidth) {
-      if (imageRatio.height != 0) {
-        imageSize.width =
-          NSCoordSaturatingNonnegativeMultiply(imageSize.height,
-                                               float(imageRatio.width) /
-                                               float(imageRatio.height));
+      if (imageRatio) {
+        imageSize.width = imageRatio.ApplyTo(imageSize.height);
         gotWidth = true;
       }
     } else {
-      if (imageRatio.width != 0) {
-        imageSize.height =
-          NSCoordSaturatingNonnegativeMultiply(imageSize.width,
-                                               float(imageRatio.height) /
-                                               float(imageRatio.width));
+      if (imageRatio) {
+        imageSize.height = imageRatio.Inverted().ApplyTo(imageSize.width);
         gotHeight = true;
       }
     }
diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h
index 36a43e46b..21407511a 100644
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -65,6 +65,7 @@ struct nsStyleImageOrientation;
 struct nsOverflowAreas;
 
 namespace mozilla {
+struct aspectRatio;
 enum class CSSPseudoElementType : uint8_t;
 class EventListenerManager;
 class SVGImageContext;
@@ -126,6 +127,7 @@ enum class RelativeTo {
  */
 class nsLayoutUtils
 {
+  typedef mozilla::AspectRatio AspectRatio;
   typedef mozilla::dom::DOMRectList DOMRectList;
   typedef mozilla::layers::Layer Layer;
   typedef mozilla::ContainerLayerParameters ContainerLayerParameters;
@@ -1217,7 +1219,7 @@ public:
    */
   static nsRect ComputeObjectDestRect(const nsRect& aConstraintRect,
                                       const IntrinsicSize& aIntrinsicSize,
-                                      const nsSize& aIntrinsicRatio,
+                                      const AspectRatio& aIntrinsicRatio,
                                       const nsStylePosition* aStylePos,
                                       nsPoint* aAnchorPoint = nullptr);
 
@@ -1837,7 +1839,7 @@ public:
    */
   static void ComputeSizeForDrawing(imgIContainer* aImage,
                                     CSSIntSize&    aImageSize,
-                                    nsSize&        aIntrinsicRatio,
+                                    AspectRatio&        aIntrinsicRatio,
                                     bool&          aGotWidth,
                                     bool&          aGotHeight);