Bug 1559094 - Restore old behavior for background-size: cover + zero-sized background positioning area. r=dholbert a=ritu
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 23 Jun 2019 10:08:16 +0000
changeset 537068 edc925e0821a1066f0b354a486cfffe34b40e4f2
parent 537067 8fb74fcbc12c83a230f700196e83193e6749d740
child 537069 902d1b155d7bdbaeb3b08e3a2a06721aeb18d4c3
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, ritu
bugs1559094
milestone68.0
Bug 1559094 - Restore old behavior for background-size: cover + zero-sized background positioning area. r=dholbert a=ritu This restores our previous and per-spec behavior. Comparing only ratios was not correct in the case one of the dimensions was zero and thus not scaled. Differential Revision: https://phabricator.services.mozilla.com/D35571
layout/generic/AspectRatio.h
layout/painting/nsImageRenderer.cpp
testing/web-platform/tests/css/css-backgrounds/background-size-cover-003-ref.html
testing/web-platform/tests/css/css-backgrounds/background-size-cover-003.html
--- a/layout/generic/AspectRatio.h
+++ b/layout/generic/AspectRatio.h
@@ -28,16 +28,21 @@ struct AspectRatio {
 
   explicit operator bool() const { return mRatio != 0.0f; }
 
   nscoord ApplyTo(nscoord aCoord) const {
     MOZ_DIAGNOSTIC_ASSERT(*this);
     return NSCoordSaturatingNonnegativeMultiply(aCoord, mRatio);
   }
 
+  float ApplyToFloat(float aFloat) const {
+    MOZ_DIAGNOSTIC_ASSERT(*this);
+    return mRatio * aFloat;
+  }
+
   // Inverts the ratio, in order to get the height / width ratio.
   MOZ_MUST_USE AspectRatio Inverted() const {
     return *this ? AspectRatio(1.0f / mRatio) : *this;
   }
 
   bool operator==(const AspectRatio& aOther) const {
     return mRatio == aOther.mRatio;
   }
--- a/layout/painting/nsImageRenderer.cpp
+++ b/layout/painting/nsImageRenderer.cpp
@@ -334,20 +334,54 @@ nsSize nsImageRenderer::ComputeConcreteS
 /* static */
 nsSize nsImageRenderer::ComputeConstrainedSize(
     const nsSize& aConstrainingSize, const AspectRatio& aIntrinsicRatio,
     FitType aFitType) {
   if (!aIntrinsicRatio) {
     return aConstrainingSize;
   }
 
-  auto constrainingRatio =
-      AspectRatio::FromSize(aConstrainingSize.width, aConstrainingSize.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.)
+  //
+  // We check if the image's aspect ratio is "fatter" than the constraint area
+  // by simply applying the aspect ratio to the constraint area's height, to
+  // produce a "hypothetical width", and we check whether that
+  // aspect-ratio-provided "hypothetical width" is wider than the constraint
+  // area's actual width. If it is, then the aspect ratio is fatter than the
+  // constraint area.
+  //
+  // 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 large 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) == (constrainingRatio < aIntrinsicRatio)) {
+  if ((aFitType == CONTAIN) == (constraintWidth < hypotheticalWidth)) {
     size.width = aConstrainingSize.width;
     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 < AppUnitsPerCSSPixel()) {
       size.height = aConstrainingSize.height;
     }
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-backgrounds/background-size-cover-003-ref.html
@@ -0,0 +1,21 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<style>
+body { margin: 0 }
+.first {
+  width: 100px;
+  height: 50px;
+  background: lime;
+}
+.space {
+  height: 50px;
+}
+.second {
+  width: 100px;
+  height: 100px;
+  background: lime;
+}
+</style>
+<div class="first"></div>
+<div class="space"></div>
+<div class="second"></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-backgrounds/background-size-cover-003.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<title>CSS Test: background-size: cover with zero-sized background positioning area.</title>
+<link rel="help" href="https://drafts.csswg.org/css-backgrounds/#valdef-background-size-cover">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/4049">
+<link rel="help" href=" https://bugzilla.mozilla.org/show_bug.cgi?id=1559094">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<link rel="match" href="background-size-cover-003-ref.html">
+<style>
+body { margin: 0 }
+div {
+  background-size: cover;
+  background-repeat: no-repeat;
+  background-position: top left;
+  background-origin: content-box;
+  background-image: url(/images/green-100x50.png);
+}
+#test1 {
+  height: 0;
+  width: 100px;
+  padding-bottom: 100px;
+}
+
+#test2 {
+  height: 100px;
+  width: 0;
+  padding-right: 100px;
+}
+#test3 {
+  height: 0;
+  width: 0;
+  padding-right: 100px;
+  padding-bottom: 100px;
+}
+</style>
+<div id="test1"></div>
+<div id="test2"></div>
+<div id="test3"></div>