Bug 989802 - Round viewport units to appunits using trunc rather than round so that repeated uses fit within a container. r=roc

viewport-units-rounding-1.html fails without the patch and passes with the patch. viewport-units-rounding-2.html fails with an early version of the patch but not without the patch or with the final version.
2014-05-29 17:43:06 -07:00 · 2014-05-29 17:43:06 -07:00 · 581f9bc30a
--- a/gfx/src/nsCoord.h
+++ b/gfx/src/nsCoord.h
@ -268,6 +268,52 @@ inline nscoord NSToCoordCeilClamped(double aValue)
  return NSToCoordCeil(aValue);
 }

+// The NSToCoordTrunc* functions remove the fractional component of
+// aValue, and are thus equivalent to NSToCoordFloor* for positive
+// values and NSToCoordCeil* for negative values.
+
+inline nscoord NSToCoordTrunc(float aValue)
+{
+  // There's no need to use truncf() since it matches the default
+  // rules for float to integer conversion.
+  return nscoord(aValue);
+}
+
+inline nscoord NSToCoordTrunc(double aValue)
+{
+  // There's no need to use trunc() since it matches the default
+  // rules for float to integer conversion.
+  return nscoord(aValue);
+}
+
+inline nscoord NSToCoordTruncClamped(float aValue)
+{
+#ifndef NS_COORD_IS_FLOAT
+  // Bounds-check before converting out of float, to avoid overflow
+  if (aValue >= nscoord_MAX) {
+    return nscoord_MAX;
+  }
+  if (aValue <= nscoord_MIN) {
+    return nscoord_MIN;
+  }
+#endif
+  return NSToCoordTrunc(aValue);
+}
+
+inline nscoord NSToCoordTruncClamped(double aValue)
+{
+#ifndef NS_COORD_IS_FLOAT
+  // Bounds-check before converting out of double, to avoid overflow
+  if (aValue >= nscoord_MAX) {
+    return nscoord_MAX;
+  }
+  if (aValue <= nscoord_MIN) {
+    return nscoord_MIN;
+  }
+#endif
+  return NSToCoordTrunc(aValue);
+}
+
 /*
 * Int Rounding Functions
 */
--- a/layout/reftests/pixel-rounding/reftest.list
+++ b/layout/reftests/pixel-rounding/reftest.list
@ -174,3 +174,6 @@ skip-if(B2G) random-if(Android) == border-image-width-4.html border-image-width-
 skip-if(B2G) random-if(Android) == border-image-width-9.html border-image-width-0.html # bug 661996 # bug 773482

 == iframe-1.html iframe-1-ref.html
+
+== viewport-units-rounding-1.html viewport-units-rounding-1-ref.html
+== viewport-units-rounding-2.html about:blank
--- a/layout/reftests/pixel-rounding/viewport-units-rounding-1-frame.html
+++ b/layout/reftests/pixel-rounding/viewport-units-rounding-1-frame.html
@ -0,0 +1,51 @@
+<!DOCTYPE HTML>
+<title>subframe for test of viewport units rounding</title>
+<meta charset="UTF-8">
+<style>
+
+html, body { margin: 0; padding: 0; border: none }
+
+body > div {
+  background: red; /* should never show */
+  overflow: hidden; /* block formatting context */
+}
+
+body > div > div {
+  float: left;
+  height: 10px;
+}
+
+body > div.w25 > div { width: 25vw; }
+
+</style>
+
+<div class="w25">
+  <div style="background: fuchsia"></div>
+  <div style="background: aqua"></div>
+  <div style="background: silver"></div>
+  <div style="background: yellow"></div>
+</div>
+
+<!--
+  Really, though, anything that is a multiple of 5vw will always
+  produce a round number of appunits when starting from an integer
+  number of pixels, since 5vw is 1/20 of the viewport.
+
+  So, more interesting:
+-->
+
+<div>
+  <div style="width: 24vw; background: fuchsia"></div>
+  <div style="width: 24vw; background: aqua"></div>
+  <div style="width: 24vw; background: silver"></div>
+  <div style="width: 24vw; background: yellow"></div>
+  <div style="width: 4vw; background: gray"></div>
+</div>
+
+<div>
+  <div style="width: 21vw; background: fuchsia"></div>
+  <div style="width: 21vw; background: aqua"></div>
+  <div style="width: 21vw; background: silver"></div>
+  <div style="width: 21vw; background: yellow"></div>
+  <div style="width: 16vw; background: gray"></div>
+</div>
--- a/layout/reftests/pixel-rounding/viewport-units-rounding-1-ref.html
+++ b/layout/reftests/pixel-rounding/viewport-units-rounding-1-ref.html
@ -0,0 +1,123 @@
+<!DOCTYPE HTML>
+<title>viewport units rounding reference</title>
+<meta charset="UTF-8">
+<style>
+div.contain { height: 50px }
+div.contain > div { height: 10px }
+div.contain > div > div { height: 10px; float: left }
+</style>
+
+<div class="contain" style="width: 200px">
+  <div>
+    <div style="width: 50px; background: fuchsia"></div>
+    <div style="width: 50px; background: aqua"></div>
+    <div style="width: 50px; background: silver"></div>
+    <div style="width: 50px; background: yellow"></div>
+  </div>
+  <div>
+    <div style="width: 48px; background: fuchsia"></div>
+    <div style="width: 48px; background: aqua"></div>
+    <div style="width: 48px; background: silver"></div>
+    <div style="width: 48px; background: yellow"></div>
+    <div style="width: 8px; background: gray"></div>
+  </div>
+  <div>
+    <div style="width: 42px; background: fuchsia"></div>
+    <div style="width: 42px; background: aqua"></div>
+    <div style="width: 42px; background: silver"></div>
+    <div style="width: 42px; background: yellow"></div>
+    <div style="width: 32px; background: gray"></div>
+  </div>
+</div>
+
+<div class="contain" style="width: 201px">
+  <div>
+    <div style="width: 50px; background: fuchsia"></div>
+    <div style="width: 51px; background: aqua"></div>
+    <div style="width: 50px; background: silver"></div>
+    <div style="width: 50px; background: yellow"></div>
+  </div>
+  <div>
+    <div style="width: 48px; background: fuchsia"></div>
+    <div style="width: 48px; background: aqua"></div>
+    <div style="width: 49px; background: silver"></div>
+    <div style="width: 48px; background: yellow"></div>
+    <div style="width: 8px; background: gray"></div>
+  </div>
+  <div>
+    <div style="width: 42px; background: fuchsia"></div>
+    <div style="width: 42px; background: aqua"></div>
+    <div style="width: 43px; background: silver"></div>
+    <div style="width: 42px; background: yellow"></div>
+    <div style="width: 32px; background: gray"></div>
+  </div>
+</div>
+
+<div class="contain" style="width: 202px">
+  <div>
+    <div style="width: 51px; background: fuchsia"></div>
+    <div style="width: 50px; background: aqua"></div>
+    <div style="width: 51px; background: silver"></div>
+    <div style="width: 50px; background: yellow"></div>
+  </div>
+  <div>
+    <div style="width: 48px; background: fuchsia"></div>
+    <div style="width: 49px; background: aqua"></div>
+    <div style="width: 48px; background: silver"></div>
+    <div style="width: 49px; background: yellow"></div>
+    <div style="width: 8px; background: gray"></div>
+  </div>
+  <div>
+    <div style="width: 42px; background: fuchsia"></div>
+    <div style="width: 43px; background: aqua"></div>
+    <div style="width: 42px; background: silver"></div>
+    <div style="width: 43px; background: yellow"></div>
+    <div style="width: 32px; background: gray"></div>
+  </div>
+</div>
+
+<div class="contain" style="width: 203px">
+  <div>
+    <div style="width: 51px; background: fuchsia"></div>
+    <div style="width: 51px; background: aqua"></div>
+    <div style="width: 50px; background: silver"></div>
+    <div style="width: 51px; background: yellow"></div>
+  </div>
+  <div>
+    <div style="width: 49px; background: fuchsia"></div>
+    <div style="width: 48px; background: aqua"></div>
+    <div style="width: 49px; background: silver"></div>
+    <div style="width: 49px; background: yellow"></div>
+    <div style="width: 8px; background: gray"></div>
+  </div>
+  <div>
+    <div style="width: 43px; background: fuchsia"></div>
+    <div style="width: 42px; background: aqua"></div>
+    <div style="width: 43px; background: silver"></div>
+    <div style="width: 42px; background: yellow"></div>
+    <div style="width: 33px; background: gray"></div>
+  </div>
+</div>
+
+<div class="contain" style="width: 204px">
+  <div>
+    <div style="width: 51px; background: fuchsia"></div>
+    <div style="width: 51px; background: aqua"></div>
+    <div style="width: 51px; background: silver"></div>
+    <div style="width: 51px; background: yellow"></div>
+  </div>
+  <div>
+    <div style="width: 49px; background: fuchsia"></div>
+    <div style="width: 49px; background: aqua"></div>
+    <div style="width: 49px; background: silver"></div>
+    <div style="width: 49px; background: yellow"></div>
+    <div style="width: 8px; background: gray"></div>
+  </div>
+  <div>
+    <div style="width: 43px; background: fuchsia"></div>
+    <div style="width: 43px; background: aqua"></div>
+    <div style="width: 43px; background: silver"></div>
+    <div style="width: 42px; background: yellow"></div>
+    <div style="width: 33px; background: gray"></div>
+  </div>
+</div>
--- a/layout/reftests/pixel-rounding/viewport-units-rounding-1.html
+++ b/layout/reftests/pixel-rounding/viewport-units-rounding-1.html
@ -0,0 +1,12 @@
+<!DOCTYPE HTML>
+<title>viewport units rounding</title>
+<meta charset="UTF-8">
+<style>
+iframe { display: block; border: 0; margin: 0; padding: 0; }
+</style>
+
+<iframe scrolling="no" src="viewport-units-rounding-1-frame.html" width="200" height="50" ></iframe>
+<iframe scrolling="no" src="viewport-units-rounding-1-frame.html" width="201" height="50" ></iframe>
+<iframe scrolling="no" src="viewport-units-rounding-1-frame.html" width="202" height="50" ></iframe>
+<iframe scrolling="no" src="viewport-units-rounding-1-frame.html" width="203" height="50" ></iframe>
+<iframe scrolling="no" src="viewport-units-rounding-1-frame.html" width="204" height="50" ></iframe>
--- a/layout/reftests/pixel-rounding/viewport-units-rounding-2.html
+++ b/layout/reftests/pixel-rounding/viewport-units-rounding-2.html
@ -0,0 +1,43 @@
+<!DOCTYPE HTML>
+<title>viewport units rounding test</title>
+<meta charset="UTF-8">
+<style>
+
+body > div > div { line-height: 1px }
+span {
+  display: inline-block;
+  height: 1px;
+  width: var(--w);
+  vertical-align: bottom;
+}
+span:nth-child(1) {
+  background: red;
+}
+span:nth-child(2) {
+  background: white;
+  margin-left: var(--negw);
+}
+
+</style>
+<body>
+<script>
+
+for (var p = 0; p < 20; ++p) {
+  var position = ((20 * p) + (p / 50)) + "px";
+  var container = document.createElement("div");
+  container.style.position = "absolute";
+  container.style.top = "0px";
+  container.style.left = position;
+  for (var w = 0; w < 99; ++w) {
+    var width = (0.5 + w / 100) + "vw";
+    var div = document.createElement("div");
+    div.setAttribute("style", "--w: " + width + "; " +
+                              "--negw: -" + width);
+    div.appendChild(document.createElement("span"));
+    div.appendChild(document.createElement("span"));
+    container.appendChild(div);
+  }
+  document.body.appendChild(container);
+}
+
+</script>
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@ -223,9 +223,21 @@ struct CalcLengthCalcOps : public css::BasicCoordCalcOps,
  }
 };

-static inline nscoord ScaleCoord(const nsCSSValue &aValue, float factor)
+static inline nscoord ScaleCoordRound(const nsCSSValue& aValue, float aFactor)
 {
-  return NSToCoordRoundWithClamp(aValue.GetFloatValue() * factor);
+  return NSToCoordRoundWithClamp(aValue.GetFloatValue() * aFactor);
+}
+
+static inline nscoord ScaleViewportCoordTrunc(const nsCSSValue& aValue,
+                                              nscoord aViewportSize)
+{
+  // For units (like percentages and viewport units) where authors might
+  // repeatedly use a value and expect some multiple of the value to be
+  // smaller than a container, we need to use floor rather than round.
+  // We need to use division by 100.0 rather than multiplication by 0.1f
+  // to avoid introducing error.
+  return NSToCoordTruncClamped(aValue.GetFloatValue() *
+                               aViewportSize / 100.0f);
 }

 already_AddRefed<nsFontMetrics>
@ -351,18 +363,22 @@ static nscoord CalcLengthWith(const nsCSSValue& aValue,
    // for an increased cost to dynamic changes to the viewport size
    // when viewport units are in use.
    case eCSSUnit_ViewportWidth: {
-      return ScaleCoord(aValue, 0.01f * CalcViewportUnitsScale(aPresContext).width);
+      nscoord viewportWidth = CalcViewportUnitsScale(aPresContext).width;
+      return ScaleViewportCoordTrunc(aValue, viewportWidth);
    }
    case eCSSUnit_ViewportHeight: {
-      return ScaleCoord(aValue, 0.01f * CalcViewportUnitsScale(aPresContext).height);
+      nscoord viewportHeight = CalcViewportUnitsScale(aPresContext).height;
+      return ScaleViewportCoordTrunc(aValue, viewportHeight);
    }
    case eCSSUnit_ViewportMin: {
      nsSize vuScale(CalcViewportUnitsScale(aPresContext));
-      return ScaleCoord(aValue, 0.01f * min(vuScale.width, vuScale.height));
+      nscoord viewportMin = min(vuScale.width, vuScale.height);
+      return ScaleViewportCoordTrunc(aValue, viewportMin);
    }
    case eCSSUnit_ViewportMax: {
      nsSize vuScale(CalcViewportUnitsScale(aPresContext));
-      return ScaleCoord(aValue, 0.01f * max(vuScale.width, vuScale.height));
+      nscoord viewportMax = max(vuScale.width, vuScale.height);
+      return ScaleViewportCoordTrunc(aValue, viewportMax);
    }
    // While we could deal with 'rem' units correctly by simply not
    // caching any data that uses them in the rule tree, it's valuable
@ -415,7 +431,7 @@ static nscoord CalcLengthWith(const nsCSSValue& aValue,
        rootFontSize = rootStyleFont->mFont.size;
      }

-      return ScaleCoord(aValue, float(rootFontSize));
+      return ScaleCoordRound(aValue, float(rootFontSize));
    }
    default:
      // Fall through to the code for units that can't be stored in the
@ -436,13 +452,13 @@ static nscoord CalcLengthWith(const nsCSSValue& aValue,
    case eCSSUnit_EM: {
      // CSS2.1 specifies that this unit scales to the computed font
      // size, not the em-width in the font metrics, despite the name.
-      return ScaleCoord(aValue, float(aFontSize));
+      return ScaleCoordRound(aValue, float(aFontSize));
    }
    case eCSSUnit_XHeight: {
      nsRefPtr<nsFontMetrics> fm =
        GetMetricsFor(aPresContext, aStyleContext, styleFont,
                      aFontSize, aUseUserFontSet);
-      return ScaleCoord(aValue, float(fm->XHeight()));
+      return ScaleCoordRound(aValue, float(fm->XHeight()));
    }
    case eCSSUnit_Char: {
      nsRefPtr<nsFontMetrics> fm =
@ -451,7 +467,7 @@ static nscoord CalcLengthWith(const nsCSSValue& aValue,
      gfxFloat zeroWidth = (fm->GetThebesFontGroup()->GetFontAt(0)
                            ->GetMetrics().zeroOrAveCharWidth);

-      return ScaleCoord(aValue, ceil(aPresContext->AppUnitsPerDevPixel() *
+      return ScaleCoordRound(aValue, ceil(aPresContext->AppUnitsPerDevPixel() *
                                          zeroWidth));
    }
    default: