Bug 133040 - Migrate layout ascents and descents to LayoutUnits instead of ints
Summary: Migrate layout ascents and descents to LayoutUnits instead of ints
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-05-17 19:48 PDT by Myles C. Maxfield
Modified: 2021-04-06 15:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch (32.05 KB, patch)
2014-05-17 20:17 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (11.23 KB, patch)
2021-04-03 01:47 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (11.31 KB, patch)
2021-04-05 12:40 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (11.37 KB, patch)
2021-04-06 00:37 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (8.64 KB, patch)
2021-04-06 08:27 PDT, Rob Buis
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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