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
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

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