RESOLVED FIXED 191445
[LFC][IFC] Move some code from InlineFormattingContext::Line to InlineFormattingContext/Geometry 
https://bugs.webkit.org/show_bug.cgi?id=191445
Summary [LFC][IFC] Move some code from InlineFormattingContext::Line to InlineFormatt...
zalan
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, zalan 		no flags
DetailsFormatted DiffDiff
Patch (21.30 KB, patch)
2018-11-08 21:21 PST, zalan 		koivisto: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
zalan
Comment 1 2018-11-08 18:58:36 PST
Created attachment 354302 [details] Patch
zalan
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)
zalan
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.
zalan
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.