Bug 150074

Summary: Split TypesettingFeatures into kerning and ligatures bools
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, dino, jonlee, rniwa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149775    
Attachments:
Description Flags
Patch
simon.fraser: review+
Patch for committing
none
Patch for committing
buildbot: commit-queue-
Archive of layout-test-results from ews106 for mac-mavericks-wk2
none
Archive of layout-test-results from ews102 for mac-mavericks
none
Patch for committing
none
Patch for committing none

Description Myles C. Maxfield 2015-10-12 18:37:35 PDT
Split TypesettingFeatures into kerning and ligatures bools
Comment 1 Myles C. Maxfield 2015-10-12 18:48:40 PDT
Created attachment 262958 [details]
Patch
Comment 2 Simon Fraser (smfr) 2015-10-12 18:58:11 PDT
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.
Comment 3 Myles C. Maxfield 2015-10-13 12:12:25 PDT
Created attachment 263005 [details]
Patch for committing
Comment 4 Myles C. Maxfield 2015-10-13 12:24:38 PDT
Created attachment 263007 [details]
Patch for committing
Comment 5 Build Bot 2015-10-13 13:06:16 PDT
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.
Comment 6 Build Bot 2015-10-13 13:06:20 PDT
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
Comment 7 Build Bot 2015-10-13 13:14:08 PDT
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.
Comment 8 Build Bot 2015-10-13 13:14:11 PDT
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
Comment 9 Myles C. Maxfield 2015-10-13 13:16:53 PDT
Created attachment 263013 [details]
Patch for committing
Comment 10 Myles C. Maxfield 2015-10-13 14:55:42 PDT
Created attachment 263024 [details]
Patch for committing
Comment 11 Myles C. Maxfield 2015-10-13 16:41:15 PDT
Committed r191014: <http://trac.webkit.org/changeset/191014>
Comment 12 Radar WebKit Bug Importer 2015-12-04 14:45:27 PST
<rdar://problem/23767727>