- Revision
- 125591
- Author
- [email protected]
- Date
- 2012-08-14 12:45:49 -0700 (Tue, 14 Aug 2012)
Log Message
Accumulating LayoutUnits with floats for determining block preferred width can lead to wrapping
https://bugs.webkit.org/show_bug.cgi?id=93513
Reviewed by Eric Seidel.
Source/WebCore:
Sub-pixel values from floats are subject to small losses in precision when accumulated with
floating point values, as we do in RenderBlock. This patch adds a new method to FractionalLayoutUnit --
ceilToFloat -- which guarantees us a floating point value at least as big as our original LayoutUnit
value, and uses it along with ceiledLayoutUnit to avoid underprovisioning RenderBlock's preferred
widths due to lost precision.
Test: fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html
* platform/FractionalLayoutUnit.h:
(WebCore::FractionalLayoutUnit::ceilToFloat): Returns a float value the same or larger than the
FractionalLayoutUnit value.
(FractionalLayoutUnit):
(WebCore::FractionalLayoutUnit::epsilon): Now returns 0 when sub-pixel is disabled.
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeLogicalLocationForFloat): Adding a LayoutUnit::epsilon fudge factor
for fitting floats. This is probably necessary due to precision being lost elsewhere.
(WebCore::RenderBlock::computeInlinePreferredLogicalWidths): Ensure no precision is lost due to conversion
when accumulating our min/max width with floats.
LayoutTests:
Ensuring floats with sub-pixel values don't unnecessarily wrap.
* fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats-expected.txt: Added.
* fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (125590 => 125591)
--- trunk/LayoutTests/ChangeLog 2012-08-14 19:38:04 UTC (rev 125590)
+++ trunk/LayoutTests/ChangeLog 2012-08-14 19:45:49 UTC (rev 125591)
@@ -1,3 +1,15 @@
+2012-08-14 Levi Weintraub <[email protected]>
+
+ Accumulating LayoutUnits with floats for determining block preferred width can lead to wrapping
+ https://bugs.webkit.org/show_bug.cgi?id=93513
+
+ Reviewed by Eric Seidel.
+
+ Ensuring floats with sub-pixel values don't unnecessarily wrap.
+
+ * fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats-expected.txt: Added.
+ * fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html: Added.
+
2012-08-14 Jer Noble <[email protected]>
Fullscreen/normal volume sliders don't stay in sync
Added: trunk/LayoutTests/fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats-expected.txt (0 => 125591)
--- trunk/LayoutTests/fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats-expected.txt 2012-08-14 19:45:49 UTC (rev 125591)
@@ -0,0 +1,9 @@
+Support
+Member Center
+This tests that we don't cause an assertion failure on relayout of nested positioned elements. This test pass if we don't cause an assertion failure.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.getElementById("support").offsetTop is document.getElementById("membercenter").offsetTop
+
Added: trunk/LayoutTests/fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html (0 => 125591)
--- trunk/LayoutTests/fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html (rev 0)
+++ trunk/LayoutTests/fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html 2012-08-14 19:45:49 UTC (rev 125591)
@@ -0,0 +1,31 @@
+<html>
+<head>
+<script src=""
+<style type="text/css">
+#relativeContainer { position:relative; width:980px; height:58px; margin:18px auto 0; z-index:999; font-size:12px; }
+#relativeContainer ul { position:absolute; right:200px; top:1px; margin:0; }
+#relativeContainer ul li { display:inline; float:left; margin:0 0 0 2.8em; background-color: green;}
+#relativeContainer ul li a { display:block; color:#333; text-decoration:none; padding:.3em 0 .3em 5px; text-shadow: 0 1px 0 #fff; line-height:18px; }
+</style>
+</head>
+<body>
+<div id="relativeContainer">
+ <ul>
+ <li id="support"><a href=""
+ <li id="membercenter"><a href="" Center</a></li>
+ </ul>
+</div>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("This tests that we don't cause an assertion failure on relayout of nested positioned elements. This test pass if we don't cause an assertion failure.");
+
+shouldBe('document.getElementById("support").offsetTop', 'document.getElementById("membercenter").offsetTop');
+
+if (window.testRunner)
+ document.getElementById("relativeContainer").display = 'none';
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (125590 => 125591)
--- trunk/Source/WebCore/ChangeLog 2012-08-14 19:38:04 UTC (rev 125590)
+++ trunk/Source/WebCore/ChangeLog 2012-08-14 19:45:49 UTC (rev 125591)
@@ -1,3 +1,29 @@
+2012-08-14 Levi Weintraub <[email protected]>
+
+ Accumulating LayoutUnits with floats for determining block preferred width can lead to wrapping
+ https://bugs.webkit.org/show_bug.cgi?id=93513
+
+ Reviewed by Eric Seidel.
+
+ Sub-pixel values from floats are subject to small losses in precision when accumulated with
+ floating point values, as we do in RenderBlock. This patch adds a new method to FractionalLayoutUnit --
+ ceilToFloat -- which guarantees us a floating point value at least as big as our original LayoutUnit
+ value, and uses it along with ceiledLayoutUnit to avoid underprovisioning RenderBlock's preferred
+ widths due to lost precision.
+
+ Test: fast/sub-pixel/block-preferred-widths-with-sub-pixel-floats.html
+
+ * platform/FractionalLayoutUnit.h:
+ (WebCore::FractionalLayoutUnit::ceilToFloat): Returns a float value the same or larger than the
+ FractionalLayoutUnit value.
+ (FractionalLayoutUnit):
+ (WebCore::FractionalLayoutUnit::epsilon): Now returns 0 when sub-pixel is disabled.
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::computeLogicalLocationForFloat): Adding a LayoutUnit::epsilon fudge factor
+ for fitting floats. This is probably necessary due to precision being lost elsewhere.
+ (WebCore::RenderBlock::computeInlinePreferredLogicalWidths): Ensure no precision is lost due to conversion
+ when accumulating our min/max width with floats.
+
2012-08-14 Jer Noble <[email protected]>
Fullscreen/normal volume sliders don't stay in sync
Modified: trunk/Source/WebCore/platform/FractionalLayoutUnit.h (125590 => 125591)
--- trunk/Source/WebCore/platform/FractionalLayoutUnit.h 2012-08-14 19:38:04 UTC (rev 125590)
+++ trunk/Source/WebCore/platform/FractionalLayoutUnit.h 2012-08-14 19:45:49 UTC (rev 125591)
@@ -114,10 +114,20 @@
int toInt() const { return m_value / kFixedPointDenominator; }
float toFloat() const { return static_cast<float>(m_value) / kFixedPointDenominator; }
double toDouble() const { return static_cast<double>(m_value) / kFixedPointDenominator; }
+ float ceilToFloat() const
+ {
+ float floatValue = toFloat();
+ if (static_cast<int>(floatValue * kFixedPointDenominator) == m_value)
+ return floatValue;
+ if (floatValue > 0)
+ return nextafterf(floatValue, std::numeric_limits<float>::max());
+ return nextafterf(floatValue, std::numeric_limits<float>::min());
+ }
#else
int toInt() const { return m_value; }
float toFloat() const { return static_cast<float>(m_value); }
double toDouble() const { return static_cast<double>(m_value); }
+ float ceilToFloat() const { return toFloat(); }
#endif
unsigned toUnsigned() const { REPORT_OVERFLOW(m_value >= 0); return toInt(); }
@@ -183,7 +193,11 @@
#endif
}
+#if ENABLE(SUBPIXEL_LAYOUT)
static float epsilon() { return 1.0f / kFixedPointDenominator; }
+#else
+ static int epsilon() { return 0; }
+#endif
static const FractionalLayoutUnit max()
{
FractionalLayoutUnit m;
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (125590 => 125591)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-08-14 19:38:04 UTC (rev 125590)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-08-14 19:45:49 UTC (rev 125591)
@@ -3826,7 +3826,8 @@
LayoutUnit heightRemainingLeft = 1;
LayoutUnit heightRemainingRight = 1;
floatLogicalLeft = logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft);
- while (logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight) - floatLogicalLeft < floatLogicalWidth) {
+ // FIXME: LayoutUnit::epsilon is probably only necessary here due to lost precision elsewhere https://bugs.webkit.org/show_bug.cgi?id=94000
+ while (logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight) - floatLogicalLeft + LayoutUnit::epsilon() < floatLogicalWidth) {
logicalTopOffset += min(heightRemainingLeft, heightRemainingRight);
floatLogicalLeft = logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft);
if (inRenderFlowThread()) {
@@ -3841,7 +3842,8 @@
LayoutUnit heightRemainingLeft = 1;
LayoutUnit heightRemainingRight = 1;
floatLogicalLeft = logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight);
- while (floatLogicalLeft - logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft) < floatLogicalWidth) {
+ // FIXME: LayoutUnit::epsilon is probably only necessary here due to lost precision elsewhere https://bugs.webkit.org/show_bug.cgi?id=94000
+ while (floatLogicalLeft - logicalLeftOffsetForLine(logicalTopOffset, logicalLeftOffset, false, &heightRemainingLeft) + LayoutUnit::epsilon() < floatLogicalWidth) {
logicalTopOffset += min(heightRemainingLeft, heightRemainingRight);
floatLogicalLeft = logicalRightOffsetForLine(logicalTopOffset, logicalRightOffset, false, &heightRemainingRight);
if (inRenderFlowThread()) {
@@ -5751,11 +5753,11 @@
Length startMargin = childStyle->marginStart();
Length endMargin = childStyle->marginEnd();
if (startMargin.isFixed())
- margins += startMargin.value();
+ margins += ceiledLayoutUnit(startMargin.value());
if (endMargin.isFixed())
- margins += endMargin.value();
- childMin += margins;
- childMax += margins;
+ margins += ceiledLayoutUnit(endMargin.value());
+ childMin += margins.ceilToFloat();
+ childMax += margins.ceilToFloat();
}
}
@@ -5763,8 +5765,8 @@
// Case (2). Inline replaced elements and floats.
// Go ahead and terminate the current line as far as
// minwidth is concerned.
- childMin += child->minPreferredLogicalWidth();
- childMax += child->maxPreferredLogicalWidth();
+ childMin += child->minPreferredLogicalWidth().ceilToFloat();
+ childMax += child->maxPreferredLogicalWidth().ceilToFloat();
bool clearPreviousFloat;
if (child->isFloating()) {
@@ -5791,11 +5793,11 @@
LayoutUnit ti = 0;
if (!addedTextIndent) {
ti = textIndent;
- childMin += ti;
- childMax += ti;
+ childMin += ti.ceilToFloat();
+ childMax += ti.ceilToFloat();
if (childMin < 0)
- textIndent = childMin;
+ textIndent = ceiledLayoutUnit(childMin);
else
addedTextIndent = true;
}
@@ -5863,12 +5865,15 @@
trailingSpaceChild = 0;
// Add in text-indent. This is added in only once.
- LayoutUnit ti = 0;
+ float ti = 0;
if (!addedTextIndent) {
- ti = textIndent;
- childMin+=ti; beginMin += ti;
- childMax+=ti; beginMax += ti;
+ ti = textIndent.ceilToFloat();
+ childMin += ti;
+ childMax += ti;
+ beginMin += ti;
+ beginMax += ti;
+
if (childMin < 0)
textIndent = childMin;
else
