[webkit-reviews] review granted: [Bug 133040] Migrate layout ascents and descents to LayoutUnits instead of ints : [Attachment 434530] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 29 10:36:46 PDT 2021

Darin Adler <darin at apple.com> has granted Rob Buis <rbuis at igalia.com>'s request
for review:
Bug 133040: Migrate layout ascents and descents to LayoutUnits instead of ints

Attachment 434530: Patch


--- Comment #17 from Darin Adler <darin at apple.com> ---
Comment on attachment 434530
  --> https://bugs.webkit.org/attachment.cgi?id=434530

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

> Source/WebCore/ChangeLog:8
> +	   Migrate GlyphOverflow to LayoutUnits instead of ints.

This is missing rationale.

Why are we doing this if it has no detectable effect? Does it make some future
change possible? Does it make something more efficient? Does it make code more
elegant? If it does have a good effect, then why no tests?

> Source/WebCore/rendering/LegacyInlineFlowBox.cpp:936
> +    LayoutUnit topGlyphEdge = glyphOverflow ? (isFlippedLine ?
glyphOverflow->bottom : glyphOverflow->top) : 0_lu;

Is it critical to use LayoutUnit rather than auto in these places? I ask
because "staying as a LayoutUnit" is one kind of thing. Converting to
LayoutUnit is another. Maybe this is conversion to LayoutUnit?

More information about the webkit-reviews mailing list