Bug 166667 – [Cocoa] Font variations are inspected even when nobody is using variations

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

[Myles C. Maxfield]

[Cocoa] Font variations are inspected even when nobody is using variations

[Myles C. Maxfield]

Created attachment 297961 [details]
Patch

[Myles C. Maxfield]

Created attachment 297962 [details]
Patch

[Myles C. Maxfield]

Created attachment 297964 [details]
Patch

[Myles C. Maxfield]

<rdar://problem/29848883>

[Said Abou-Hallawa]

Comment on attachment 297964 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:466
> +            if (iterator != defaultValues.end()) {

Can't we use:

if (iterator == defaultValues.end())
    continue;

So we do not have to indent the rest of the loop.

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:467
> +                float valueToApply = std::max(std::min(newVariation.value(), iterator->value.maximumValue), iterator->value.minimumValue);

Can't we use clampTo(newVariation.value(), iterator->value.minimumValue, iterator->value.maximumValue) instead?

[Myles C. Maxfield]

Comment on attachment 297964 [details]
Patch

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

>> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:466
>> +            if (iterator != defaultValues.end()) {
> 
> Can't we use:
> 
> if (iterator == defaultValues.end())
>     continue;
> 
> So we do not have to indent the rest of the loop.

👍

>> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:467
>> +                float valueToApply = std::max(std::min(newVariation.value(), iterator->value.maximumValue), iterator->value.minimumValue);
> 
> Can't we use clampTo(newVariation.value(), iterator->value.minimumValue, iterator->value.maximumValue) instead?

👍

[Myles C. Maxfield]

The patch at https://bugs.webkit.org/show_bug.cgi?id=166672 fixes this.

*** This bug has been marked as a duplicate of bug 166672 ***