Summary: | [CSS Shapes] Complete RasterShape::firstIncludedIntervalLogicalTop() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hans Muller <giles_joplin> | ||||||||
Component: | CSS | Assignee: | Hans Muller <giles_joplin> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, glenn, kondapallykalyan | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 116348 | ||||||||||
Attachments: |
|
Description
Hans Muller
2013-08-15 11:15:58 PDT
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. Created attachment 209202 [details]
Patch
Eliminated unnecessary Vector copies in RasterShape.cpp, where Region::rects() is called.
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 { }. Created attachment 209209 [details]
Patch
Incorporated review feedback.
Comment on attachment 209209 [details] Patch Clearing flags on attachment: 209209 Committed r154349: <http://trac.webkit.org/changeset/154349> All reviewed patches have been landed. Closing bug. |