Bug 98676

Summary: [CSS Exclusions] shape-outside on floats for polygon shapes
Product: WebKit Reporter: Bem Jones-Bey <bjonesbe>
Component: CSSAssignee: Bem Jones-Bey <bjonesbe>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, esprehn+autocc, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://dev.w3.org/csswg/css3-exclusions/#supported-basic-shapes
Bug Depends on:    
Bug Blocks: 98664    
Attachments:
Description Flags
Patch
none
Updated Patch
none
Updated Patch
none
Updated Patch none

Bem Jones-Bey
Reported 2012-10-08 11:42:16 PDT
This bug tracks the implementation of shape-outside on floats for polygon shape outsides.
Attachments
Patch (35.19 KB, patch)
2013-03-01 11:12 PST, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Updated Patch (35.35 KB, patch)
2013-03-01 13:40 PST, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Updated Patch (35.53 KB, patch)
2013-03-01 14:36 PST, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Updated Patch (36.65 KB, patch)
2013-03-04 21:20 PST, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Bem Jones-Bey
Comment 1 2013-03-01 11:12:07 PST
Created attachment 190995 [details] Patch Initial fix
WebKit Review Bot
Comment 2 2013-03-01 11:55:09 PST
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 3 2013-03-01 12:09:33 PST
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
Comment 4 2013-03-01 13:40:48 PST
Created attachment 191027 [details] Updated Patch Maybe this will make the Linux Chromium bots happy?
Bem Jones-Bey
Comment 5 2013-03-01 14:36:44 PST
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 6 2013-03-04 14:52:59 PST
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
Comment 7 2013-03-04 21:20:16 PST
Created attachment 191394 [details] Updated Patch Fix writing mode issues, add explanatory comments, change method name to hopefully make more sense.
Dave Hyatt
Comment 8 2013-03-05 09:47:34 PST
Comment on attachment 191394 [details] Updated Patch r=me
WebKit Review Bot
Comment 9 2013-03-05 10:21:41 PST
Comment on attachment 191394 [details] Updated Patch Clearing flags on attachment: 191394 Committed r144776: <http://trac.webkit.org/changeset/144776>
WebKit Review Bot
Comment 10 2013-03-05 10:21:44 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.