Floored and truncated rounded confused.
authorallan.jensen@nokia.com <allan.jensen@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 12:20:30 +0000 (12:20 +0000)
committerallan.jensen@nokia.com <allan.jensen@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 12:20:30 +0000 (12:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93475

Reviewed by Levi Weintraub.

Source/WebCore:

Fix the common mistake of confusing truncating (round towards zero) and
flooring (round down). They are only identical for positive numbers,
not for negative numbers.

This patch fixes instances of misimplemented or misused floor in the
layout and geometric code. And also uses the new correct implementations
to clean up the code for enclosingRect.

* platform/FractionalLayoutUnit.h:
(WebCore::FractionalLayoutUnit::fromFloatFloor):
(FractionalLayoutUnit):
(WebCore::FractionalLayoutUnit::ceil):
(WebCore::FractionalLayoutUnit::floor):
* platform/graphics/FloatPoint.h:
(WebCore::FloatPoint::FloatPoint):
(WebCore::roundedIntPoint):
(WebCore::flooredIntPoint):
(WebCore::ceiledIntPoint):
(WebCore::flooredIntSize):
* platform/graphics/FloatRect.cpp:
(WebCore::enclosingIntRect):
(WebCore::enclosedIntRect):
* platform/graphics/FloatSize.h:
(WebCore::roundedIntSize):
(WebCore::flooredIntSize):
(WebCore::flooredIntPoint):
* platform/graphics/FractionalLayoutPoint.h:
(WebCore::flooredIntPoint):
(WebCore::flooredFractionalLayoutPoint):
(WebCore::ceiledFractionalLayoutPoint):
* platform/graphics/FractionalLayoutRect.cpp:
(WebCore::enclosingIntRect):
(WebCore::enclosingFractionalLayoutRect):
* platform/graphics/FractionalLayoutSize.h:
(WebCore::flooredIntSize):
* platform/graphics/IntRect.cpp:
(WebCore::IntRect::IntRect):
* rendering/LayoutTypes.h:
(WebCore::flooredLayoutPoint):
(WebCore::floorToInt):
(WebCore::isIntegerValue):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::convertToLayerCoords):

LayoutTests:

* platform/chromium/TestExpectations:
* platform/mac/TestExpectations:
* platform/qt/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@125167 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/chromium/TestExpectations
LayoutTests/platform/mac/TestExpectations
LayoutTests/platform/qt/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/FractionalLayoutUnit.h
Source/WebCore/platform/graphics/FloatPoint.h
Source/WebCore/platform/graphics/FloatRect.cpp
Source/WebCore/platform/graphics/FloatSize.h
Source/WebCore/platform/graphics/FractionalLayoutPoint.h
Source/WebCore/platform/graphics/FractionalLayoutRect.cpp
Source/WebCore/platform/graphics/FractionalLayoutSize.h
Source/WebCore/platform/graphics/IntRect.cpp
Source/WebCore/rendering/LayoutTypes.h
Source/WebCore/rendering/RenderLayer.cpp

index a06ecc0..bbf8f1f 100644 (file)
@@ -1,3 +1,14 @@
+2012-08-09  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
+
+        Floored and truncated rounded confused.
+        https://bugs.webkit.org/show_bug.cgi?id=93475
+
+        Reviewed by Levi Weintraub.
+
+        * platform/chromium/TestExpectations:
+        * platform/mac/TestExpectations:
+        * platform/qt/TestExpectations:
+
 2012-08-09  Pavel Feldman  <pfeldman@chromium.org>
 
         Web Inspector: improve large array logging experience
index 6743960..c053866 100644 (file)
@@ -3489,3 +3489,6 @@ BUGWK93568 DEBUG : http/tests/misc/window-dot-stop.html = PASS TEXT
 
 BUGWK93586 : fast/forms/autocomplete-off-with-default-value-does-not-clear.html = TIMEOUT
 
+// Tests requiring rebaseling afer bug 93475
+BUGWK93475 : fast/block/float/overhanging-tall-block.html = TEXT
+BUGWK93475 : fast/html/details-writing-mode.html = IMAGE
index da76d44..24227c0 100644 (file)
@@ -335,3 +335,6 @@ BUGWK93538 : fast/forms/basic-selects.html = TEXT
 
 // (r124484) inspector/device-orientation-success.html failing on Mac ports
 BUGWK93552 : inspector/device-orientation-success.html = TEXT
+
+// Tests requiring rebaseling afer bug 93475
+BUGWK93475 : fast/block/float/overhanging-tall-block.html = TEXT
index a4d4a1f..299c739 100644 (file)
@@ -137,3 +137,6 @@ BUGWK93148 : tables/mozilla_expected_failures/marvin/table_overflow_hidden_tr.ht
 BUGWK93148 : fast/css/nested-layers-with-hover.html = TEXT
 
 BUGWK93247 DEBUG : fast/lists/list-marker-remove-crash.html = CRASH
+
+// Test requiring rebaseling afer bug 93475
+BUGWK93475 : fast/block/float/overhanging-tall-block.html = TEXT
index e14cda1..582af2f 100644 (file)
@@ -1,3 +1,54 @@
+2012-08-09  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
+
+        Floored and truncated rounded confused.
+        https://bugs.webkit.org/show_bug.cgi?id=93475
+
+        Reviewed by Levi Weintraub.
+
+        Fix the common mistake of confusing truncating (round towards zero) and
+        flooring (round down). They are only identical for positive numbers,
+        not for negative numbers.
+
+        This patch fixes instances of misimplemented or misused floor in the
+        layout and geometric code. And also uses the new correct implementations
+        to clean up the code for enclosingRect.
+
+        * platform/FractionalLayoutUnit.h:
+        (WebCore::FractionalLayoutUnit::fromFloatFloor):
+        (FractionalLayoutUnit):
+        (WebCore::FractionalLayoutUnit::ceil):
+        (WebCore::FractionalLayoutUnit::floor):
+        * platform/graphics/FloatPoint.h:
+        (WebCore::FloatPoint::FloatPoint):
+        (WebCore::roundedIntPoint):
+        (WebCore::flooredIntPoint):
+        (WebCore::ceiledIntPoint):
+        (WebCore::flooredIntSize):
+        * platform/graphics/FloatRect.cpp:
+        (WebCore::enclosingIntRect):
+        (WebCore::enclosedIntRect):
+        * platform/graphics/FloatSize.h:
+        (WebCore::roundedIntSize):
+        (WebCore::flooredIntSize):
+        (WebCore::flooredIntPoint):
+        * platform/graphics/FractionalLayoutPoint.h:
+        (WebCore::flooredIntPoint):
+        (WebCore::flooredFractionalLayoutPoint):
+        (WebCore::ceiledFractionalLayoutPoint):
+        * platform/graphics/FractionalLayoutRect.cpp:
+        (WebCore::enclosingIntRect):
+        (WebCore::enclosingFractionalLayoutRect):
+        * platform/graphics/FractionalLayoutSize.h:
+        (WebCore::flooredIntSize):
+        * platform/graphics/IntRect.cpp:
+        (WebCore::IntRect::IntRect):
+        * rendering/LayoutTypes.h:
+        (WebCore::flooredLayoutPoint):
+        (WebCore::floorToInt):
+        (WebCore::isIntegerValue):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::convertToLayerCoords):
+
 2012-08-07  Andrey Kosyakov  <caseq@chromium.org>
 
         Web Inspector: use WebInspector.ProgressIndicator in AdvancedSearchController
index 5944035..d81aef2 100644 (file)
@@ -95,6 +95,14 @@ public:
         return v;
     }
 
+    static FractionalLayoutUnit fromFloatFloor(float value)
+    {
+        REPORT_OVERFLOW(isInBounds(value));
+        FractionalLayoutUnit v;
+        v.m_value = floorf(value * kFixedPointDenominator);
+        return v;
+    }
+
     static FractionalLayoutUnit fromFloatRound(float value)
     {
         if (value >= 0)
@@ -146,9 +154,9 @@ public:
 #endif
     {
 #if ENABLE(SUBPIXEL_LAYOUT)
-        if (m_value > 0)
+        if (m_value >= 0)
             return (m_value + kFixedPointDenominator - 1) / kFixedPointDenominator;
-        return (m_value - kFixedPointDenominator + 1) / kFixedPointDenominator;
+        return toInt();
 #else
         return m_value;
 #endif
@@ -166,7 +174,13 @@ public:
 
     int floor() const
     {
-        return toInt();
+#if ENABLE(SUBPIXEL_LAYOUT)
+        if (m_value >= 0)
+            return toInt();
+        return (m_value - kFixedPointDenominator + 1) / kFixedPointDenominator;
+#else
+        return m_value;
+#endif
     }
 
     static float epsilon() { return 1.0f / kFixedPointDenominator; }
index a601a9d..90e6c4b 100644 (file)
@@ -77,6 +77,7 @@ public:
     FloatPoint(float x, float y) : m_x(x), m_y(y) { }
     FloatPoint(const IntPoint&);
     FloatPoint(const FractionalLayoutPoint&);
+    explicit FloatPoint(const FloatSize& size) : m_x(size.width()), m_y(size.height()) { }
 
     static FloatPoint zero() { return FloatPoint(); }
 
@@ -243,17 +244,22 @@ inline float operator*(const FloatPoint& a, const FloatPoint& b)
 
 inline IntPoint roundedIntPoint(const FloatPoint& p)
 {
-    return IntPoint(static_cast<int>(roundf(p.x())), static_cast<int>(roundf(p.y())));
+    return IntPoint(clampToInteger(roundf(p.x())), clampToInteger(roundf(p.y())));
 }
 
 inline IntPoint flooredIntPoint(const FloatPoint& p)
 {
-    return IntPoint(static_cast<int>(p.x()), static_cast<int>(p.y()));
+    return IntPoint(clampToInteger(floorf(p.x())), clampToInteger(floorf(p.y())));
+}
+
+inline IntPoint ceiledIntPoint(const FloatPoint& p)
+{
+    return IntPoint(clampToInteger(ceilf(p.x())), clampToInteger(ceilf(p.y())));
 }
 
 inline IntSize flooredIntSize(const FloatPoint& p)
 {
-    return IntSize(static_cast<int>(p.x()), static_cast<int>(p.y()));
+    return IntSize(clampToInteger(floorf(p.x())), clampToInteger(floorf(p.y())));
 }
 
 float findSlope(const FloatPoint& p1, const FloatPoint& p2, float& c);
index b43fc74..f31c6cc 100644 (file)
@@ -214,26 +214,20 @@ void FloatRect::fitToPoints(const FloatPoint& p0, const FloatPoint& p1, const Fl
 
 IntRect enclosingIntRect(const FloatRect& rect)
 {
-    float left = floorf(rect.x());
-    float top = floorf(rect.y());
-    float width = ceilf(rect.maxX()) - left;
-    float height = ceilf(rect.maxY()) - top;
-    
-    return IntRect(clampToInteger(left), clampToInteger(top), 
-                   clampToInteger(width), clampToInteger(height));
+    IntPoint location = flooredIntPoint(rect.minXMinYCorner());
+    IntPoint maxPoint = ceiledIntPoint(rect.maxXMaxYCorner());
+
+    return IntRect(location, maxPoint - location);
 }
 
 IntRect enclosedIntRect(const FloatRect& rect)
 {
-    int x = clampToInteger(ceilf(rect.x()));
-    int y = clampToInteger(ceilf(rect.y()));
-    float maxX = clampToInteger(floorf(rect.maxX()));
-    float maxY = clampToInteger(floorf(rect.maxY()));
-    // A rect of width 0 should not become a rect of width -1 due to ceil/floor.
-    int width = max(clampToInteger(maxX - x), 0);
-    int height = max(clampToInteger(maxY - y), 0);
-
-    return IntRect(x, y, width, height);
+    IntPoint location = ceiledIntPoint(rect.minXMinYCorner());
+    IntPoint maxPoint = flooredIntPoint(rect.maxXMaxYCorner());
+    IntSize size = maxPoint - location;
+    size.clampNegativeToZero();
+
+    return IntRect(location, size);
 }
 
 IntRect roundedIntRect(const FloatRect& rect)
index 5f01fbb..b1d72a5 100644 (file)
@@ -186,12 +186,12 @@ inline bool operator!=(const FloatSize& a, const FloatSize& b)
 
 inline IntSize roundedIntSize(const FloatSize& p)
 {
-    return IntSize(static_cast<int>(roundf(p.width())), static_cast<int>(roundf(p.height())));
+    return IntSize(clampToInteger(roundf(p.width())), clampToInteger(roundf(p.height())));
 }
 
 inline IntSize flooredIntSize(const FloatSize& p)
 {
-    return IntSize(static_cast<int>(p.width()), static_cast<int>(p.height()));
+    return IntSize(clampToInteger(floorf(p.width())), clampToInteger(floorf(p.height())));
 }
 
 inline IntSize expandedIntSize(const FloatSize& p)
@@ -201,7 +201,7 @@ inline IntSize expandedIntSize(const FloatSize& p)
 
 inline IntPoint flooredIntPoint(const FloatSize& p)
 {
-    return IntPoint(static_cast<int>(p.width()), static_cast<int>(p.height()));
+    return IntPoint(clampToInteger(floorf(p.width())), clampToInteger(floorf(p.height())));
 }
 
 } // namespace WebCore
index 26550b9..fc830e6 100644 (file)
@@ -159,7 +159,7 @@ inline FractionalLayoutSize toSize(const FractionalLayoutPoint& a)
 
 inline IntPoint flooredIntPoint(const FractionalLayoutPoint& point)
 {
-    return IntPoint(point.x().toInt(), point.y().toInt());
+    return IntPoint(point.x().floor(), point.y().floor());
 }
 
 inline IntPoint roundedIntPoint(const FractionalLayoutPoint& point)
@@ -177,6 +177,16 @@ inline IntPoint ceiledIntPoint(const FractionalLayoutPoint& point)
     return IntPoint(point.x().ceil(), point.y().ceil());
 }
 
+inline FractionalLayoutPoint flooredFractionalLayoutPoint(const FloatPoint& p)
+{
+    return FractionalLayoutPoint(FractionalLayoutUnit::fromFloatFloor(p.x()), FractionalLayoutUnit::fromFloatFloor(p.y()));
+}
+
+inline FractionalLayoutPoint ceiledFractionalLayoutPoint(const FloatPoint& p)
+{
+    return FractionalLayoutPoint(FractionalLayoutUnit::fromFloatCeil(p.x()), FractionalLayoutUnit::fromFloatCeil(p.y()));
+}
+
 } // namespace WebCore
 
 #endif // FractionalLayoutPoint_h
index 7c92556..76c0026 100644 (file)
@@ -128,21 +128,19 @@ FractionalLayoutRect unionRect(const Vector<FractionalLayoutRect>& rects)
 
 IntRect enclosingIntRect(const FractionalLayoutRect& rect)
 {
-    float x = floorf(rect.x().toFloat());
-    float y = floorf(rect.y().toFloat());
-    float width = ceilf(rect.maxX().toFloat()) - x;
-    float height = ceilf(rect.maxY().toFloat()) - y;
+    IntPoint location = flooredIntPoint(rect.minXMinYCorner());
+    IntPoint maxPoint = ceiledIntPoint(rect.maxXMaxYCorner());
 
-    return IntRect(clampToInteger(x), clampToInteger(y), 
-                   clampToInteger(width), clampToInteger(height));
+    return IntRect(location, maxPoint - location);
 }
 
 FractionalLayoutRect enclosingFractionalLayoutRect(const FloatRect& rect)
 {
 #if ENABLE(SUBPIXEL_LAYOUT)
-    return FractionalLayoutRect(rect.x(), rect.y(),
-                     rect.maxX() - rect.x() + FractionalLayoutUnit::epsilon(),
-                     rect.maxY() - rect.y() + FractionalLayoutUnit::epsilon());
+    FractionalLayoutPoint location = flooredFractionalLayoutPoint(rect.minXMinYCorner());
+    FractionalLayoutPoint maxPoint = ceiledFractionalLayoutPoint(rect.maxXMaxYCorner());
+
+    return FractionalLayoutRect(location, maxPoint - location);
 #else
     return enclosingIntRect(rect);
 #endif
index 4ed97e3..1071d8a 100644 (file)
@@ -151,7 +151,7 @@ inline bool operator!=(const FractionalLayoutSize& a, const FractionalLayoutSize
 
 inline IntSize flooredIntSize(const FractionalLayoutSize& s)
 {
-    return IntSize(s.width().toInt(), s.height().toInt());
+    return IntSize(s.width().floor(), s.height().floor());
 }
 
 inline IntSize roundedIntSize(const FractionalLayoutSize& s)
index dfa2f91..fd1e509 100644 (file)
@@ -36,14 +36,14 @@ using std::min;
 namespace WebCore {
 
 IntRect::IntRect(const FloatRect& r)
-    : m_location(IntPoint(static_cast<int>(r.x()), static_cast<int>(r.y())))
-    , m_size(IntSize(static_cast<int>(r.width()), static_cast<int>(r.height())))
+    : m_location(clampToInteger(r.x()), clampToInteger(r.y()))
+    , m_size(clampToInteger(r.width()), clampToInteger(r.height()))
 {
 }
 
 IntRect::IntRect(const FractionalLayoutRect& r)
-    : m_location(flooredIntPoint(r.location()))
-    , m_size(flooredIntSize(r.size()))
+    : m_location(r.x(), r.y())
+    , m_size(r.width(), r.height())
 {
 }
 
index 3b8ff53..51967f8 100644 (file)
@@ -91,17 +91,12 @@ inline LayoutPoint roundedLayoutPoint(const FloatPoint& p)
 
 inline LayoutPoint flooredLayoutPoint(const FloatPoint& p)
 {
-    return LayoutPoint(p.x(), p.y());
+    return flooredFractionalLayoutPoint(p);
 }
 
 inline LayoutPoint flooredLayoutPoint(const FloatSize& s)
 {
-    return LayoutPoint(s.width(), s.height());
-}
-
-inline LayoutSize flooredLayoutSize(const FloatPoint& p)
-{
-    return LayoutSize(p.x(), p.y());
+    return flooredLayoutPoint(FloatPoint(s));
 }
 
 inline int roundToInt(LayoutUnit value)
@@ -111,7 +106,7 @@ inline int roundToInt(LayoutUnit value)
 
 inline int floorToInt(LayoutUnit value)
 {
-    return value.toInt();
+    return value.floor();
 }
 
 inline LayoutUnit roundedLayoutUnit(float value)
@@ -164,7 +159,7 @@ inline IntRect pixelSnappedIntRect(LayoutPoint location, LayoutSize size)
 
 inline bool isIntegerValue(const LayoutUnit value)
 {
-    return value.floor() == value;
+    return value.toInt() == value;
 }
 
 } // namespace WebCore
index ef2a8e8..373beab 100644 (file)
@@ -1457,7 +1457,7 @@ void RenderLayer::convertToLayerCoords(const RenderLayer* ancestorLayer, LayoutP
         // If the fixed layer's container is the root, just add in the offset of the view. We can obtain this by calling
         // localToAbsolute() on the RenderView.
         FloatPoint absPos = renderer()->localToAbsolute(FloatPoint(), true);
-        location += flooredLayoutSize(absPos);
+        location += LayoutSize(absPos.x(), absPos.y());
         return;
     }