Bug 191445

Summary: [LFC][IFC] Move some code from InlineFormattingContext::Line to InlineFormattingContext/Geometry
Product: WebKit Reporter: alan baradlay <zalan>
Component: Layout and RenderingAssignee: alan baradlay <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, koivisto, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch koivisto: review+

alan baradlay
Reported 2018-11-08 16:53:24 PST
The idea here is Line should not need to deal with all the post processing activities like aligning the runs (there will be more)
Attachments
Patch (21.03 KB, patch)
2018-11-08 18:58 PST, alan baradlay 		no flags
DetailsFormatted DiffDiff
Patch (21.30 KB, patch)
2018-11-08 21:21 PST, alan baradlay 		koivisto: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
alan baradlay
Comment 1 2018-11-08 18:58:36 PST
Created attachment 354302 [details] Patch
alan baradlay
Comment 2 2018-11-08 21:21:45 PST
Created attachment 354311 [details] Patch
Antti Koivisto
Comment 3 2018-11-09 05:22:27 PST
Comment on attachment 354311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354311&action=review > Source/WebCore/layout/inlineformatting/InlineFormattingContext.h:87 > InlineFormattingState& m_formattingState; Maybe the InlineFormattingState can be passed to the functions that need it? It seems bit silly to have this backpointer in every Line instance. > Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp:111 > + expansionBehavior ^= AllowTrailingExpansion; > + expansionBehavior |= ForbidTrailingExpansion; Should modernize this to use OptionSet at some point.
Antti Koivisto
Comment 4 2018-11-09 05:23:04 PST
(neither comment has much to do with this patch)
alan baradlay
Comment 5 2018-11-09 07:29:06 PST
Committed r238028: <https://trac.webkit.org/changeset/238028>
Radar WebKit Bug Importer
Comment 6 2018-11-09 07:30:24 PST
<rdar://problem/45943491>
Antti Koivisto
Comment 7 2018-11-09 08:53:19 PST
Comment on attachment 354311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354311&action=review >> Source/WebCore/layout/inlineformatting/InlineFormattingContext.h:87 >> InlineFormattingState& m_formattingState; > > Maybe the InlineFormattingState can be passed to the functions that need it? It seems bit silly to have this backpointer in every Line instance. Actually Line seems to be a stack object so this comment doesn't make much sense.
alan baradlay
Comment 8 2018-11-09 08:54:29 PST
(In reply to Antti Koivisto from comment #7) > Comment on attachment 354311 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=354311&action=review > > >> Source/WebCore/layout/inlineformatting/InlineFormattingContext.h:87 > >> InlineFormattingState& m_formattingState; > > > > Maybe the InlineFormattingState can be passed to the functions that need it? It seems bit silly to have this backpointer in every Line instance. > > Actually Line seems to be a stack object so this comment doesn't make much > sense. It is indeed a stack object and we only construct one per formatting context.
Note You need to log in before you can comment on or make changes to this bug.