Bug 98676 – [CSS Exclusions] shape-outside on floats for polygon shapes

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

[Bem Jones-Bey]

This bug tracks the implementation of shape-outside on floats for polygon shape outsides.

[Bem Jones-Bey]

Created attachment 190995 [details]
Patch

Initial fix

[WebKit Review Bot]

Comment on attachment 190995 [details]
Patch

Attachment 190995 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16857091

[WebKit Review Bot]

Comment on attachment 190995 [details]
Patch

Attachment 190995 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16882002

[Bem Jones-Bey]

Created attachment 191027 [details]
Updated Patch

Maybe this will make the Linux Chromium bots happy?

[Bem Jones-Bey]

Created attachment 191038 [details]
Updated Patch

The last one had bad luck, lets try again to make the Linux Chromium bots happy?

[Dave Hyatt]

Comment on attachment 191038 [details]
Updated Patch

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

r- for the writing mode issue, since that looks wrong to me.

> Source/WebCore/rendering/ExclusionShapeInfo.h:92
> -    LayoutUnit shapeLogicalRight() const { return computedShape()->shapeLogicalBoundingBox().y() + logicalLeftOffset(); }
> +    LayoutUnit shapeLogicalRight() const { return computedShape()->shapeLogicalBoundingBox().maxX() + logicalLeftOffset(); }

LOL.

> Source/WebCore/rendering/RenderBlock.h:1143
> +        const FloatingObject* lastFloat() const { return m_last; }

I think it is worth a comment explaining what this is.

> Source/WebCore/rendering/RenderBlock.h:1151
> +        mutable const FloatingObject* m_last;

Ditto.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:181
> +        shapeOutsideInfo->computeSegmentsForLine(m_block->logicalHeight() - newFloat->y(), logicalHeightForLine(m_block, m_isFirstLine));

This looks wrong. I suspect you mean m_block->logicalTopForFloat(newFloat) rather than newFloat->y().

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:188
> +        if (shapeOutsideInfo)
> +            newLeft += shapeOutsideInfo->logicalRightOffsetForLine();

This looks confusing to me. The only reason we're getting the right edge of the float is that is where the left edge of the line begins (i.e., after the right edge of the float). 

It reads very confusingly that you are calling a method named logicalRightOffsetForLine. If it's a delta applied to the float's right edge, then this method name seems very strange.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:199
> +#if ENABLE(CSS_EXCLUSIONS)
> +        if (shapeOutsideInfo)
> +            newRight += shapeOutsideInfo->logicalLeftOffsetForLine();
> +#endif

Same here. This seems very confusing.

[Bem Jones-Bey]

Created attachment 191394 [details]
Updated Patch

Fix writing mode issues, add explanatory comments, change method name to hopefully make more sense.

[Dave Hyatt]

Comment on attachment 191394 [details]
Updated Patch

r=me

[WebKit Review Bot]

Comment on attachment 191394 [details]
Updated Patch

Clearing flags on attachment: 191394

Committed r144776: <http://trac.webkit.org/changeset/144776>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.