Bug 119849 – [CSS Shapes] Complete RasterShape::firstIncludedIntervalLogicalTop()

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Hans Muller]

Complete the implementation of RasterShape::firstIncludedIntervalLogicalTop(). Currently the first layout position isn't computed, it's assumed that layout can begin at the top of the shape.

A detailed description of the algorithm to be used here can be found in http://hansmuller-webkit.blogspot.com/2013/08/first-fit-location-for-image-shapes.html.

[Hans Muller]

Created attachment 209132 [details]
Patch

Completed the implementation of RasterShape::firstIncludedIntervalLogicalTop(). The method now computes first logical top location where a line segment can be laid out within a RasterShape, i.e. a shape derived from an image valued URL resource. A detailed description of the algorithm can be found in http://hansmuller-webkit.blogspot.com/2013/08/first-fit-location-for-image-shapes.html.

The new tests exposed a bug in the existing getIncludedIntervals() method. A shape with a vertical gap that spans the entire line now causes the method to short circuit and return an empty interval list.

[Hans Muller]

Created attachment 209202 [details]
Patch

Eliminated unnecessary Vector copies in RasterShape.cpp, where Region::rects() is called.

[Alexandru Chiculita]

Comment on attachment 209202 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=209202&action=review

r=me

> Source/WebCore/rendering/shapes/RasterShape.cpp:53
> +    for (int y = minY; y <= bounds().height() - minSize.height(); y++) {

nit: might be useful to explain why we combine absolute position minY with relative position from bounds().height(). Or just use maxY.

> Source/WebCore/rendering/shapes/RasterShape.cpp:101
> +        if (lineY > currentLineMaxY) // We've encountered a vertical gap in lineRects, there are no included intervals.

nit: put the comment inside the if statement. Use { }.

[Hans Muller]

Created attachment 209209 [details]
Patch

Incorporated review feedback.

[WebKit Commit Bot]

Comment on attachment 209209 [details]
Patch

Clearing flags on attachment: 209209

Committed r154349: <http://trac.webkit.org/changeset/154349>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.