Summary: | Split TypesettingFeatures into kerning and ligatures bools | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||||||||||
Component: | New Bugs | Assignee: | 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
Myles C. Maxfield
2015-10-12 18:37:35 PDT
Created attachment 262958 [details]
Patch
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. Created attachment 263005 [details]
Patch for committing
Created attachment 263007 [details]
Patch for committing
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. 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 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. 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
Created attachment 263013 [details]
Patch for committing
Created attachment 263024 [details]
Patch for committing
Committed r191014: <http://trac.webkit.org/changeset/191014> |