Image valued shapes are represented internally as a vector of above threshold horizontal "runs"; just the start,end column for the above threshold pixels on a row. The vector's size is currently the image's height plus the user supplied shapeMargin length. The limiting case for the run-vector's size is just one above threshold pixel one the first or last image row. In that case the shapeMargin can be limited to the shape's reference box height, since the shape is clipped to the reference box after it's expanded by shapeMargin. Currently shapeMargin is incorrectly clamped to the image size in RasterShape::marginIntervals() int marginBoundaryRadius = std::min(clampToInteger(ceil(shapeMargin())), std::max(m_imageSize.width(), m_imageSize.height()));
Note also: once this is fixed, the m_imageSize member can be removed.
Created attachment 227304 [details] Patch Corrected the way the maximum shape-margin value was computed for image valued shapes. The limiting case is an image with just one above threshold pixel. The largest possible value for shapeMargin is the radius of a circle that encloses the size of the shape-outside element's margin-box, since the shape-outside's boundary is clipped to the margin-box. The radius is just sqrt(2) * the margin-box's largest dimension. Also cleaned up up a few poorly named parameters.
Comment on attachment 227304 [details] Patch You're missing ChangeLogs. Looks ok otherwise though.
Created attachment 227345 [details] Patch Added the ChangeLogs. Sorry about that.
Comment on attachment 227345 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227345&action=review > Source/WebCore/rendering/shapes/Shape.h:71 > - static PassOwnPtr<Shape> createRasterShape(Image*, float threshold, const LayoutRect& drawRect, const LayoutRect& clipRect, WritingMode, Length margin, Length padding); > + static PassOwnPtr<Shape> createRasterShape(Image*, float threshold, const LayoutRect& imageRect, const LayoutRect& marginRect, WritingMode, Length margin, Length padding); Interesting that Shape.cpp already had that change.
(In reply to comment #5) > (From update of attachment 227345 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=227345&action=review > > > Source/WebCore/rendering/shapes/Shape.h:71 > > - static PassOwnPtr<Shape> createRasterShape(Image*, float threshold, const LayoutRect& drawRect, const LayoutRect& clipRect, WritingMode, Length margin, Length padding); > > + static PassOwnPtr<Shape> createRasterShape(Image*, float threshold, const LayoutRect& imageRect, const LayoutRect& marginRect, WritingMode, Length margin, Length padding); > > Interesting that Shape.cpp already had that change. Yes. I had trouble deciding what to call this parameter and the code reflected as much. It's the rect that the overall shape is clipped to (so clipRect) and it's defined by the shape element's margin-box (so marginRect), which can be a little confusing since it's shape-margin that can grow the shape into the margin-box. The names still aren't great, but now at least they're consistent.
Comment on attachment 227345 [details] Patch Clearing flags on attachment: 227345 Committed r166019: <http://trac.webkit.org/changeset/166019>
All reviewed patches have been landed. Closing bug.