Bug 175236 – Update font selection algorithm to match latest CSS spec

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

Update font selection algorithm to match latest CSS spec

[Myles C. Maxfield]

Created attachment 317330 [details]
Needs tests

[Myles C. Maxfield]

Created attachment 317361 [details]
WIP

[Myles C. Maxfield]

Created attachment 317664 [details]
Patch

[Build Bot]

Attachment 317664 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:10:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 317701 [details]
Patch

[Darin Adler]

Comment on attachment 317701 [details]
Patch

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

> Source/WebCore/platform/graphics/FontSelectionAlgorithm.h:220
> +static inline FontSelectionValue lowerWeightSearchThreshold()

All these functions that return FontSelectionValue could instead return const FontSelectionValue&. Unclear to me how OK it is that they copy every time they are called.

> LayoutTests/ChangeLog:9
> +        * fast/text/variations/font-selection-font-weight-expected.html: Added.
> +        * fast/text/variations/font-selection-font-weight.html: Added.

Note that reference tests only compare the top 800x600; is this test fitting or spilling off the bottom?

[Myles C. Maxfield]

Comment on attachment 317701 [details]
Patch

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

>> Source/WebCore/platform/graphics/FontSelectionAlgorithm.h:220
>> +static inline FontSelectionValue lowerWeightSearchThreshold()
> 
> All these functions that return FontSelectionValue could instead return const FontSelectionValue&. Unclear to me how OK it is that they copy every time they are called.

A FontSelectionValue is 2 bytes, so it is a better choice than a reference.

>> LayoutTests/ChangeLog:9
>> +        * fast/text/variations/font-selection-font-weight.html: Added.
> 
> Note that reference tests only compare the top 800x600; is this test fitting or spilling off the bottom?

It fits because all the frames are 48x48 and are inline-blocks.

[WebKit Commit Bot]

Comment on attachment 317701 [details]
Patch

Clearing flags on attachment: 317701

Committed r221630: <http://trac.webkit.org/changeset/221630>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34694258>