Bug 100049

Summary:

[CSS Exclusions] Implement empty segments for multiple-segment shape-insides

Product:

WebKit

Reporter:

Bear Travis <betravis>

Component:

CSS

Assignee:

Bear Travis <betravis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, esprehn+autocc, giles_joplin

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

89256

Attachments:

Description	Flags
Initial Patch	none
Incorporating feedback	none
Updating patch	hyatt: review+
Factoring out line adjustment code	none

Bear Travis

Reported 2012-10-22 16:26:14 PDT

We at least need several tests to cover these cases

Attachments
Initial Patch (21.68 KB, patch) 2013-04-16 16:43 PDT, Bear Travis	no flags	Details Formatted Diff Diff
Incorporating feedback (22.22 KB, patch) 2013-04-18 13:24 PDT, Bear Travis	no flags	Details Formatted Diff Diff
Updating patch (22.22 KB, patch) 2013-04-18 14:43 PDT, Bear Travis	hyatt: review+	Details Formatted Diff Diff
Factoring out line adjustment code (22.54 KB, patch) 2013-04-19 14:40 PDT, Bear Travis	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Bear Travis

Comment 1 2013-04-16 16:43:19 PDT

Created attachment 198452 [details] Initial Patch

Hans Muller

Comment 2 2013-04-17 16:42:32 PDT

Source/WebCore/ChangeLog > Shape-insides do not require content in every segment. I'm not exactly sure what this means. Some line segments within a shape may be empty because because the interior of the shape does not completely overlap the line? Source/WebCore/rendering/RenderBlockLineLayout.cpp > float minWidth = 0; > for (size_t i = 0; i < wordMeasurements.size(); i++) { > minWidth = wordMeasurements[i].width; > if (minWidth > 0) > break; > } Maybe move this litte loop into an aptly named inline function, so: float minWidth = firstPositiveWidth(wordMeasurements); And I'm hoping that there's no such thing as negative WordMeasurement.width values. Note also: the cosmetic feedback I've been getting about this kind of loop is to use unsigned and ++i, as in: for (unsigned i = 0; i < wordMeasurements.size(); ++i)

Bear Travis

Comment 3 2013-04-18 13:24:50 PDT

Created attachment 198757 [details] Incorporating feedback

Benjamin Poulain

Comment 4 2013-04-18 14:00:49 PDT

Comment on attachment 198757 [details] Incorporating feedback View in context: https://bugs.webkit.org/attachment.cgi?id=198757&action=review Some minor comments bellow. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:1629 > + for (unsigned i = 0; i < wordMeasurements.size(); i++) { unsigned-> size_t i++ -> ++i > Source/WebCore/rendering/RenderBlockLineLayout.cpp:1715 > + ASSERT(minWidth || !wordMeasurements.size()); !wordMeasurements.size() -> wordMeasurements.isEmpty()

Bear Travis

Comment 5 2013-04-18 14:43:46 PDT

Created attachment 198764 [details] Updating patch

Dave Hyatt

Comment 6 2013-04-19 10:48:32 PDT

Comment on attachment 198764 [details] Updating patch View in context: https://bugs.webkit.org/attachment.cgi?id=198764&action=review r=me, although see comment. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:1722 > + if (exclusionShapeInsideInfo && end == resolver.position()) { > + float minWidth = firstPositiveWidth(wordMeasurements); > + ASSERT(minWidth || wordMeasurements.isEmpty()); > + LayoutUnit nextLineHeight; > + if (minWidth > 0 && exclusionShapeInsideInfo->adjustLogicalLineTop(minWidth)) > + nextLineHeight = exclusionShapeInsideInfo->logicalLineTop() - absoluteLogicalTop; > + else > + nextLineHeight = exclusionShapeInsideInfo->shapeLogicalBottom() - absoluteLogicalTop; > + end = restartLayoutRunsAndFloatsInRange(logicalHeight(), nextLineHeight, lastFloatFromPreviousLine, resolver, oldEnd); > continue; I think it would be nice to put this into a helper function, even if it's just a static function that is up with firstPositiveWidth's definition.

Bear Travis

Comment 7 2013-04-19 14:40:17 PDT

Created attachment 198915 [details] Factoring out line adjustment code

Bear Travis

Comment 8 2013-04-19 15:45:21 PDT

Comment on attachment 198915 [details] Factoring out line adjustment code Adding to commit queue.

WebKit Commit Bot

Comment 9 2013-04-19 16:27:12 PDT

Comment on attachment 198915 [details] Factoring out line adjustment code Clearing flags on attachment: 198915 Committed r148781: <http://trac.webkit.org/changeset/148781>

WebKit Commit Bot

Comment 10 2013-04-19 16:27:14 PDT

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.