Bug 98967 – [CSS Exclusions] Points on the bottom and right edges of an exclusion shape should be classified as "outside"

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

[Hans Muller]

Created attachment 168091 [details]
Diagram: illustrate line inside/outside limits for exclusion shapes

We should a point X,Y to be "inside" an exclusion shape rectangle if

     Y >= rectangleY and Y < rectangleY + rectangleHeight
     X >= rectangleX and X < rectangleX+ rectangleWidth

Polygon vertices or horizontal edges with maximal Y values are similarly not inside the polygon.  Hopefully the diagram makes this clear.

[Hans Muller]

Created attachment 169666 [details]
Patch

Changed the way lines are represented in the ExclusionShapeInsideInfo and ExclusionShape classes so that they're consistent with the rendering code that depends on them.  Lines are now defined by logicalTop, logicalHeight, instead of logicalTop,logicalBottom.  This a clean-up, not a change in functionality. It's already covered by the existing fast/exclusions LayoutTests.

Did not change the way line segements are represented, since what we're using is already consistent with the rest of the rendering code.  

The only material impact from the bug fix is a change to ExclusionShape::lineOverlapsBounds() which used to return 

   return m_lineTop <= shapeBounds.maxY() && m_lineBottom >= shapeBounds.y();

and now returns:

    return m_lineTop < shapeBounds.maxY() && m_lineTop + m_lineHeight >= shapeBounds.y();

This is consistent with the attached diagram: a line whose top is the same as the exclusion shape's top+height is considered outside the shape.    A line whose top equals the shape's top is considered inside.

[Bear Travis]

Comment on attachment 169666 [details]
Patch

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

Overall, looks to be a pretty straightforward change.

> Source/WebCore/rendering/ExclusionShapeInsideInfo.h:100
> +    return m_lineTop < shapeBounds.maxY() && m_lineTop + m_lineHeight >= shapeBounds.y();

Is there a way to add a test for this behavior? Perhaps using a rectangular shape-inside with a line height of 0?

[Hans Muller]

(In reply to comment #2)
> (From update of attachment 169666 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169666&action=review
> 
> Overall, looks to be a pretty straightforward change.
> 
> > Source/WebCore/rendering/ExclusionShapeInsideInfo.h:100
> > +    return m_lineTop < shapeBounds.maxY() && m_lineTop + m_lineHeight >= shapeBounds.y();
> 
> Is there a way to add a test for this behavior? Perhaps using a rectangular shape-inside with a line height of 0?

I think LayoutTests/fast/exclusions/shape-inside/shape-inside-inline-elements.html covers this case.  It creates lines with lineHeight=0.

[Hans Muller]

Created attachment 169948 [details]
Patch

Added the test Bear suggested, just to make sure this particular case is pinned down.

[Dirk Schulze]

Comment on attachment 169948 [details]
Patch

LGTM. r=me

[WebKit Review Bot]

Comment on attachment 169948 [details]
Patch

Clearing flags on attachment: 169948

Committed r132127: <http://trac.webkit.org/changeset/132127>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.