[webkit-reviews] review granted: [Bug 217832] [LFC][IFC] Introduce layout bounds based vertical alignment : [Attachment 411664] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted zalan <zalan at apple.com>'s request
for review:
Bug 217832: [LFC][IFC] Introduce layout bounds based vertical alignment
https://bugs.webkit.org/show_bug.cgi?id=217832

Attachment 411664: Patch

https://bugs.webkit.org/attachment.cgi?id=411664&action=review




--- Comment #4 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 411664
  --> https://bugs.webkit.org/attachment.cgi?id=411664
Patch

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

>
Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp:411
>      auto lineBoxLogicalHeight = lineBox.logicalHeight();
> -    auto lineLogicalHeight = InlineLayoutUnit { };
> -    if (rootStyle.lineHeight().isNegative()) {
> -	   // Negative line height value means the line height is driven by the
content.
> -	   auto usedLineSpacing = [&] {
> -	       auto logicalTopWithLineSpacing = InlineLayoutUnit { };
> -	       auto logicalBottomWithLineSpacing = lineBoxLogicalHeight;
> -	       for (auto& inlineLevelBox : lineBox.inlineLevelBoxList()) {
> -		   if (auto lineSpacing = inlineLevelBox->lineSpacing()) {
> -		       // FIXME: check if line spacing is distributed evenly.
> -		       logicalTopWithLineSpacing =
std::min(logicalTopWithLineSpacing, inlineLevelBox->logicalTop() - *lineSpacing
/ 2);
> -		       logicalBottomWithLineSpacing =
std::max(logicalBottomWithLineSpacing, inlineLevelBox->logicalBottom() +
*lineSpacing / 2);
> -		   }
> -	       }
> -	       return -logicalTopWithLineSpacing +
(logicalBottomWithLineSpacing - lineBoxLogicalHeight);
> -	   };
> -	   lineLogicalHeight = lineBox.logicalHeight() + usedLineSpacing();
> -    } else
> -	   lineLogicalHeight = rootStyle.computedLineHeight();
> -
> +    auto lineLogicalHeight = lineBox.logicalHeight();
>      auto logicalRect = InlineRect { lineContent.logicalTopLeft,
lineContent.lineLogicalWidth, lineLogicalHeight};
>      // Inline tree height is all integer based.
>      auto lineBoxOffset = floorf((lineLogicalHeight - lineBoxLogicalHeight) /
2);

lineBoxOffset == 0?