Bug 133040

Summary:

Migrate layout ascents and descents to LayoutUnits instead of ints

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

New Bugs

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

REOPENED ---

Severity:

Normal

CC:

changseok, commit-queue, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, rbuis, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	ews-feeder: commit-queue-

Description Myles C. Maxfield 2014-05-17 19:48:28 PDT

Migrate layout ascents and descents to LayoutUnit instead of ints

Comment 1 Myles C. Maxfield 2014-05-17 20:17:29 PDT

Created attachment 231645 [details]
Patch

Comment 2 zalan 2014-05-17 21:53:15 PDT

Comment on attachment 231645 [details]
Patch

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

> Source/WebCore/rendering/InlineFlowBox.cpp:872
> +    LayoutUnit strokeOverflow = static_cast<int>(ceilf(lineStyle.textStrokeWidth() / 2.0f));

missing FIXME here.
In general, I transition to LayoutUntits from int/float when I change the functionality as well. int type is a straightforward indication that a particular function/class has not been converted yet to subpixel. Functions with integral LayoutUnits tricked me a few times.

Comment 3 Myles C. Maxfield 2014-06-03 22:08:03 PDT

Split out into https://bugs.webkit.org/show_bug.cgi?id=133501

Comment 4 Rob Buis 2021-04-03 01:47:59 PDT

Created attachment 425084 [details]
Patch

Comment 5 Rob Buis 2021-04-05 12:40:32 PDT

Created attachment 425188 [details]
Patch

Comment 6 zalan 2021-04-05 12:53:44 PDT

Comment on attachment 425188 [details]
Patch

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

> Source/WebCore/rendering/RootInlineBox.cpp:819
> +            LayoutUnit usedFontAscent = fontMetrics.ascent(baselineType());

It's up to you but as I mentioned in the previous patch I'd prefer uniform initialization (and mention in the changelog that ascent/descent values are still integral).

Comment 7 Rob Buis 2021-04-06 00:37:22 PDT

Created attachment 425252 [details]
Patch

Comment 8 EWS 2021-04-06 03:49:38 PDT

Committed r275502: <https://commits.webkit.org/r275502>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425252 [details].

Comment 9 Radar WebKit Bug Importer 2021-04-06 03:50:15 PDT

<rdar://problem/76261027>

Comment 10 Rob Buis 2021-04-06 07:41:37 PDT

Comment on attachment 425188 [details]
Patch

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

>> Source/WebCore/rendering/RootInlineBox.cpp:819
>> +            LayoutUnit usedFontAscent = fontMetrics.ascent(baselineType());
> 
> It's up to you but as I mentioned in the previous patch I'd prefer uniform initialization (and mention in the changelog that ascent/descent values are still integral).

Sure, done.

Comment 11 zalan 2021-04-06 07:53:44 PDT

(In reply to Rob Buis from comment #10)
> Comment on attachment 425188 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425188&action=review
> 
> >> Source/WebCore/rendering/RootInlineBox.cpp:819
> >> +            LayoutUnit usedFontAscent = fontMetrics.ascent(baselineType());
> > 
> > It's up to you but as I mentioned in the previous patch I'd prefer uniform initialization (and mention in the changelog that ascent/descent values are still integral).
> 
> Sure, done.
Thanks!

Comment 12 Rob Buis 2021-04-06 08:27:18 PDT

Reopening to attach new patch.

Comment 13 Rob Buis 2021-04-06 08:27:21 PDT

Created attachment 425281 [details]
Patch