Summary: | [CSS Exclusions] polygon shape-inside layout fails | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hans Muller <giles_joplin> | ||||||||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Created attachment 197457 [details]
Test case.
Created attachment 198349 [details]
Simplified test case screenshot.
Created attachment 198350 [details]
Simplified test case.
This version of the test case uses the Ahem font and just a few single-character lines of text to demo the problem.
Created attachment 198388 [details]
Patch
Comment on attachment 198388 [details]
Patch
The firstIncludedIntervalLogicalTop() method's implementation relied on optimistic assumptions about floating point accuracy which, in rare cases, caused it to discard first-fit locations based on the intersection of the minLogicalIntervalTop offset edge and a polygon offset edge. Now: we do not verify that first-fit locations based on the intersection of an offset edge and the minLogicalIntervalTop offset edge are below the horizontal minLogicalIntervalTop line. They're essentially below the line "by definition".
Comment on attachment 198388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=198388&action=review r=me. Just some editorial change requests. > Source/WebCore/ChangeLog:18 > + (WebCore::ExclusionPolygon::firstIncludedIntervalLogicalTop): Avoid floating point problems when checking intersections with the offset edge based on minLogicalIntervalTop. Split the comment into two lines. > Source/WebCore/ChangeLog:21 > + (WebCore::OffsetPolygonEdge::basis): Report what the offset edge is "based on": a polygon edge, the top of the line, or a (reflex) vertex. Ditto. > Source/WebCore/rendering/ExclusionPolygon.h:41 > + enum Basis { Edge, Vertex, LineTop }; I'd like to have a new line for each enum item. This is usually used nowadays. Created attachment 198447 [details]
Patch
Made the changes that Dirk suggested.
Comment on attachment 198447 [details] Patch Clearing flags on attachment: 198447 Committed r148582: <http://trac.webkit.org/changeset/148582> All reviewed patches have been landed. Closing bug. |
Created attachment 197456 [details] Test case screenshot. As can be seen in the attached screenshot, there's a large gap in the text where no gap should be.