[CSS Exclusions] ExclusionShape bounding box methods should return LayoutRects

Source/core/rendering/exclusions/ExclusionPolygon.cpp

Index: Source/core/rendering/exclusions/ExclusionPolygon.cpp
diff --git a/Source/core/rendering/exclusions/ExclusionPolygon.cpp b/Source/core/rendering/exclusions/ExclusionPolygon.cpp
index b89d1e5d40af2253959bcff7f8505aaf0c2c2892..1157041a79855fb6531f1759b801243710e59baf 100644
--- a/Source/core/rendering/exclusions/ExclusionPolygon.cpp
+++ b/Source/core/rendering/exclusions/ExclusionPolygon.cpp
@@ -347,14 +347,14 @@ static void computeOverlappingEdgeXProjections(const FloatPolygon& polygon, floa
     sortExclusionIntervals(result);
 }
 
-void ExclusionPolygon::getExcludedIntervals(float logicalTop, float logicalHeight, SegmentList& result) const
+void ExclusionPolygon::getExcludedIntervals(LayoutUnit logicalTop, LayoutUnit logicalHeight, SegmentList& result) const
 {
     const FloatPolygon& polygon = shapeMarginBounds();
     if (polygon.isEmpty())
         return;
 
     float y1 = logicalTop;
-    float y2 = y1 + logicalHeight;
+    float y2 = logicalTop + logicalHeight;

[eae, 2013/05/06 15:59:29]

This changes  the precision of the calculations and could theoretically
introduce an off by one errors (logicalTop + logicalHeight).toFloat() and
logicalTop.toFloat() + logicalHeight.toFloat() are not guaranteed to be the
same.

The same applies to the other changes in this file. This might not be a problem
or might even fix some edge cases. It would be nice to see a test using values
where round(y+h) and round(y)+round(h) are different.

Finally, please update the commit message to detail these logic changes as they
are not covered by the existing description.

 
     Vector<ExclusionInterval> y1XIntervals, y2XIntervals;
     computeXIntersections(polygon, y1, true, y1XIntervals);
@@ -375,14 +375,14 @@ void ExclusionPolygon::getExcludedIntervals(float logicalTop, float logicalHeigh
     }
 }
 
-void ExclusionPolygon::getIncludedIntervals(float logicalTop, float logicalHeight, SegmentList& result) const
+void ExclusionPolygon::getIncludedIntervals(LayoutUnit logicalTop, LayoutUnit logicalHeight, SegmentList& result) const
 {
     const FloatPolygon& polygon = shapePaddingBounds();
     if (polygon.isEmpty())
         return;
 
     float y1 = logicalTop;
-    float y2 = y1 + logicalHeight;
+    float y2 = logicalTop + logicalHeight;
 
     Vector<ExclusionInterval> y1XIntervals, y2XIntervals;
     computeXIntersections(polygon, y1, true, y1XIntervals);
@@ -427,24 +427,28 @@ static inline bool aboveOrToTheLeft(const FloatRect& r1, const FloatRect& r2)
     return false;
 }
 
-bool ExclusionPolygon::firstIncludedIntervalLogicalTop(float minLogicalIntervalTop, const FloatSize& minLogicalIntervalSize, float& result) const
+bool ExclusionPolygon::firstIncludedIntervalLogicalTop(LayoutUnit minLogicalIntervalTop, const LayoutSize& minLogicalIntervalSize, LayoutUnit& result) const
 {
+    float minIntervalTop = minLogicalIntervalTop;
+    float minIntervalHeight = minLogicalIntervalSize.height();
+    float minIntervalWidth = minLogicalIntervalSize.width();
+
     const FloatPolygon& polygon = shapePaddingBounds();
     const FloatRect boundingBox = polygon.boundingBox();
-    if (minLogicalIntervalSize.width() > boundingBox.width())
+    if (minIntervalWidth > boundingBox.width())
         return false;
 
-    float minY = std::max(boundingBox.y(), minLogicalIntervalTop);
-    float maxY = minY + minLogicalIntervalSize.height();
+    float minY = std::max(boundingBox.y(), minIntervalTop);
+    float maxY = minY + minIntervalHeight;
 
     if (maxY > boundingBox.maxY())
         return false;
 
     Vector<const FloatPolygonEdge*> edges;
-    polygon.overlappingEdges(minLogicalIntervalTop, boundingBox.maxY(), edges);
+    polygon.overlappingEdges(minIntervalTop, boundingBox.maxY(), edges);
 
-    float dx = minLogicalIntervalSize.width() / 2;
-    float dy = minLogicalIntervalSize.height() / 2;
+    float dx = minIntervalWidth / 2;
+    float dy = minIntervalHeight / 2;
     Vector<OffsetPolygonEdge> offsetEdges;
 
     for (unsigned i = 0; i < edges.size(); ++i) {
@@ -478,7 +482,7 @@ bool ExclusionPolygon::firstIncludedIntervalLogicalTop(float minLogicalIntervalT
                 offsetEdges.append(offsetEdgeBuffer[j]);
     }
 
-    offsetEdges.append(OffsetPolygonEdge(polygon, minLogicalIntervalTop, FloatSize(0, dy)));
+    offsetEdges.append(OffsetPolygonEdge(polygon, minIntervalTop, FloatSize(0, dy)));
 
     FloatPoint offsetEdgesIntersection;
     FloatRect firstFitRect;
@@ -491,7 +495,7 @@ bool ExclusionPolygon::firstIncludedIntervalLogicalTop(float minLogicalIntervalT
                 FloatRect potentialFirstFitRect(potentialFirstFitLocation, minLogicalIntervalSize);
                 if ((offsetEdges[i].basis() == OffsetPolygonEdge::LineTop
                     || offsetEdges[j].basis() == OffsetPolygonEdge::LineTop
-                    || potentialFirstFitLocation.y() >= minLogicalIntervalTop)
+                    || potentialFirstFitLocation.y() >= minIntervalTop)
                     && (!firstFitFound || aboveOrToTheLeft(potentialFirstFitRect, firstFitRect))
                     && polygon.contains(offsetEdgesIntersection)
                     && firstFitRectInPolygon(polygon, potentialFirstFitRect, offsetEdges[i].edgeIndex(), offsetEdges[j].edgeIndex())) {
@@ -503,7 +507,7 @@ bool ExclusionPolygon::firstIncludedIntervalLogicalTop(float minLogicalIntervalT
     }
 
     if (firstFitFound)
-        result = firstFitRect.y();
+        result = LayoutUnit::fromFloatCeil(firstFitRect.y());
     return firstFitFound;
 }