[CSS Exclusions] ExclusionShape bounding box methods should return LayoutRects

• Component: CSS
• Status: RESOLVED FIXED

[Hans Muller]

During the review of https://codereview.chromium.org/14289003/ it was pointed out that since the ExclusionShape classes are supposed to return values aligned with the LayoutUnit grid, the shapeMarginLogicalBoundingBox() and shapeMarginLogicalBoundingBox() methods should return LayoutRects, not FloatRects.

Also per the review: we should use flooredLayoutPoint() to snap polygon vertices to the LayoutUnit grid in snapVerticesToLayoutUnitGrid().

[Hans Muller]

(In reply to comment #0)
> During the review of https://codereview.chromium.org/14289003/ it was pointed out that since the ExclusionShape classes are supposed to return values aligned with the LayoutUnit grid, the shapeMarginLogicalBoundingBox() and shapeMarginLogicalBoundingBox() methods should return LayoutRects, not FloatRects.

Note also: by just assigning the margin and padding min,max values to a LayoutRect, we should be able to eliminate the need for the internal snapVerticesToLayoutUnitGrid() method altogether.

[Hans Muller]

Created attachment 199738 [details]
Patch

Redefined the ExclusionShape API in terms of LayoutUnits, instead of floats: all of the ExclusionShape methods now have LayoutUnit parameters and return LayoutUnit values. This is more natural, since the callers work exclusively in LayoutUnits. All of the internal ExclusionShape computations are still done with floats.

I was not able to remove the (now streamlined) snapVerticesToLayoutUnitGrid() method, which is applied to the FloatPoint vertices computed for the internal margin and padding FloatPolygons.  Using LayoutUnit vertices for polygons would obviate the snapping step, but I think trying to replace all use of floating point in FloatPolygon, or LayoutPolygon if you like, would at least be rather destabilizing. The computed polygons need to be snapped to the LayoutUnit grid so that requests to layout lines aligned with their bounding boxes top/bottom succeed. That is: so that the vertical extent of the bounding box matches the FloatPolygon's vertical extent.

[Dirk Schulze]

Comment on attachment 199738 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 199738 [details]
Patch

Clearing flags on attachment: 199738

Committed r149226: <http://trac.webkit.org/changeset/149226>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.