Bug 206861

Summary:

[LFC][IFC] Decouple Display::LineBox and Layout::LineBoxBuilder

Product:

WebKit

Reporter:

zalan <zalan>

Component:

Layout and Rendering

Assignee:

zalan <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, commit-queue, koivisto, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Description zalan 2020-01-27 21:15:59 PST

ssia

Comment 1 Radar WebKit Bug Importer 2020-01-27 21:16:21 PST

<rdar://problem/58945284>

Comment 2 zalan 2020-01-27 21:18:55 PST

Created attachment 388964 [details]
Patch

Comment 3 zalan 2020-01-28 07:30:04 PST

Created attachment 388991 [details]
Patch

Comment 4 Antti Koivisto 2020-01-28 08:18:28 PST

Comment on attachment 388991 [details]
Patch

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

> Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:544
> +    inlineContent.lineBoxes.append(Display::LineBox { lineBox.logicalRect(), lineBox.scrollableOverflow(), lineInkOverflow, { baseline.ascent(), baseline.descent() }, lineBox.baselineOffset(), lineBox.isConsideredEmpty() });

Could have some sort of helper for the conversion.

Type name Display::LineBox is probably not needed.

> Source/WebCore/layout/inlineformatting/InlineLineBox.h:36
> +namespace Layout {
> +
> +class LineBox {

Could this be like LineBoxBuilder or something?

Comment 5 zalan 2020-01-28 08:43:56 PST

(In reply to Antti Koivisto from comment #4)
> Comment on attachment 388991 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388991&action=review
> 
> > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:544
> > +    inlineContent.lineBoxes.append(Display::LineBox { lineBox.logicalRect(), lineBox.scrollableOverflow(), lineInkOverflow, { baseline.ascent(), baseline.descent() }, lineBox.baselineOffset(), lineBox.isConsideredEmpty() });
> 
> Could have some sort of helper for the conversion.
This will eventually turn into a logical -> physical conversion (horizontal flip/vertical transform). I am not yet sure the exact setup for this so I prefer to not do it in this patch.  

> 
> Type name Display::LineBox is probably not needed.
> 
> > Source/WebCore/layout/inlineformatting/InlineLineBox.h:36
> > +namespace Layout {
> > +
> > +class LineBox {
> 
> Could this be like LineBoxBuilder or something?
Sure.

Comment 6 zalan 2020-01-28 08:51:04 PST

Created attachment 389007 [details]
Patch

Comment 7 zalan 2020-01-28 08:51:57 PST

Created attachment 389009 [details]
Patch

Comment 8 WebKit Commit Bot 2020-01-28 10:26:07 PST

Comment on attachment 389009 [details]
Patch

Clearing flags on attachment: 389009

Committed r255257: <https://trac.webkit.org/changeset/255257>

Comment 9 WebKit Commit Bot 2020-01-28 10:26:09 PST

All reviewed patches have been landed.  Closing bug.