Bug 150074 – Split TypesettingFeatures into kerning and ligatures bools

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

[Myles C. Maxfield]

Split TypesettingFeatures into kerning and ligatures bools

[Myles C. Maxfield]

Created attachment 262958 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 262958 [details]
Patch

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

> Source/WebCore/platform/graphics/FontCascade.cpp:108
> +    , m_enableKerning(0)
> +    , m_enableLigatures(0)

false, not 0.

> Source/WebCore/platform/graphics/FontCascade.h:310
> +        return kerning == FontCascadeDescription::Kerning::Normal
> +            || (kerning == FontCascadeDescription::Kerning::Auto
> +                && (textRenderingMode == GeometricPrecision
> +                    || textRenderingMode == OptimizeLegibility
> +                    || (textRenderingMode == AutoTextRendering && s_defaultKerning)));

My eyes! Can you factor this into some more readable inline functions?

> Source/WebCore/platform/graphics/FontCascade.h:321
> +        return ligatures == FontVariantLigatures::Yes
> +            || (ligatures == FontVariantLigatures::Normal
> +                && (textRenderingMode == GeometricPrecision
> +                    || textRenderingMode == OptimizeLegibility
> +                    || (textRenderingMode == AutoTextRendering && s_defaultLigatures)));

Ditto.

> Source/WebCore/platform/graphics/FontDescription.h:195
> -    enum Kerning { AutoKerning, NormalKerning, NoneKerning };
> +    enum class Kerning { Auto, Normal, None };

Call sites would be more readable if you move this outside the class.

> Source/WebCore/platform/graphics/WidthIterator.h:92
> +    bool m_enableKerning;
> +    bool m_enableLigatures;

No initializers?

> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:478
> +    ProviderInfo info = { characters, length, getCFStringAttributes(false, false, platformData().orientation()) };

Boolean trap here.

[Myles C. Maxfield]

Created attachment 263005 [details]
Patch for committing

[Myles C. Maxfield]

Created attachment 263007 [details]
Patch for committing

[Build Bot]

Comment on attachment 263007 [details]
Patch for committing

Attachment 263007 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/280259

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 263011 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 263007 [details]
Patch for committing

Attachment 263007 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/280283

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 263012 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Myles C. Maxfield]

Created attachment 263013 [details]
Patch for committing

[Myles C. Maxfield]

Created attachment 263024 [details]
Patch for committing

[Myles C. Maxfield]

Committed r191014: <http://trac.webkit.org/changeset/191014>

[Radar WebKit Bug Importer]

<rdar://problem/23767727>