Bug 149237

Summary: Create a font which can be used for testing font features
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, jonlee, rniwa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch simon.fraser: review+

Description Myles C. Maxfield 2015-09-16 16:16:17 PDT 
Create a font which can be used for testing font features
Comment 1 Myles C. Maxfield 2015-09-16 16:37:48 PDT 
Created attachment 261336 [details]
Patch
Comment 2 WebKit Commit Bot 2015-09-16 16:40:47 PDT 
Attachment 261336 [details] did not pass style-queue:


ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:9:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:12:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:20:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:27:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:80:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:82:  Use 'WTF::move()' instead of 'std::move()'.  [runtime/wtf_move] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:139:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:149:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:161:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:171:  Extra space after ( in function call  [whitespace/parens] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:173:  Extra space after ( in function call  [whitespace/parens] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:179:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:180:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:189:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:226:  Use 'WTF::move()' instead of 'std::move()'.  [runtime/wtf_move] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:275:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:309:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:429:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:443:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:502:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:505:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:607:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:630:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:653:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:673:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:674:  Omit int when using unsigned  [runtime/unsigned] [1]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:680:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:698:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:716:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:742:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:765:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:767:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
ERROR: Tools/FontWithFeatures/FontWithFeatures/main.cpp:10:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/main.cpp:14:  Streams are highly discouraged.  [readability/streams] [3]
ERROR: Tools/FontWithFeatures/FontWithFeatures/main.cpp:16:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/main.cpp:18:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/main.cpp:64:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/FontWithFeatures/FontWithFeatures/main.cpp:100:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
Total errors found: 38 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Simon Fraser (smfr) 2015-09-16 17:04:40 PDT 
Comment on attachment 261336 [details]
Patch

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

> Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:140
> +std::vector<uint8_t> generateBoxCharString() {
> +    CFFBuilder builder(unitsPerEm, std::make_pair(0.f, 0.f));

WebKit style is brace on new line. Needs fixing all over this file.

> Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:267
> +        bool active = { true };

no =

> Tools/FontWithFeatures/FontWithFeatures/main.cpp:37
> +    CGFloat fontSize = 25;
> +    CGContextSetTextMatrix(context, CGAffineTransformScale(CGAffineTransformIdentity, 1, 1));
> +    CGContextSetTextPosition(context, location.x, location.y);
> +    CFNumberRef featureValue = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &value);
> +    CFTypeRef featureDictionaryKeys[] = { kCTFontOpenTypeFeatureTag, kCTFontOpenTypeFeatureValue };
> +    CFTypeRef featureDictionaryValues[] = { feature, featureValue };
> +    CFDictionaryRef featureDictionary = CFDictionaryCreate(kCFAllocatorDefault, featureDictionaryKeys, featureDictionaryValues, 2, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
> +    CFRelease(featureValue);
> +    CFTypeRef featureSettingsValues[] = { featureDictionary };
> +    CFArrayRef fontFeatureSettings = CFArrayCreate(kCFAllocatorDefault, featureSettingsValues, 1, &kCFTypeArrayCallBacks);
> +    CFRelease(featureDictionary);
> +    CFTypeRef fontDescriptorKeys[] = { kCTFontFeatureSettingsAttribute };
> +    CFTypeRef fontDescriptorValues[] = { fontFeatureSettings };
> +    CFDictionaryRef fontDescriptorAttributes = CFDictionaryCreate(kCFAllocatorDefault, fontDescriptorKeys, fontDescriptorValues, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
> +    CFRelease(fontFeatureSettings);
> +    CTFontDescriptorRef modifiedFontDescriptor = CTFontDescriptorCreateCopyWithAttributes(fontDescriptor, fontDescriptorAttributes);
> +    CFRelease(fontDescriptorAttributes);
> +    CTFontRef font = CTFontCreateWithFontDescriptor(modifiedFontDescriptor, fontSize, nullptr);
> +    CFRelease(modifiedFontDescriptor);

Such a wall of code.
Comment 4 Myles C. Maxfield 2015-09-16 17:33:16 PDT 
Comment on attachment 261336 [details]
Patch

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

> Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:17
> +static const uint16_t unitsPerEm = 1024;

This should be int16_t.

> Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:534
> +        append16(clampTo<int16_t>(unitsPerEm)); // Typographical descender

Might as well make is 1.

> Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:537
> +        append16(clampTo<uint16_t>(unitsPerEm)); // Windows-specific descent

Ditto.

> Tools/FontWithFeatures/FontWithFeatures/FontCreator.cpp:656
> +        append16(clampTo<int16_t>(unitsPerEm)); // descent

Ditto.
Comment 5 Myles C. Maxfield 2015-09-16 18:15:48 PDT 
Committed r189890: <http://trac.webkit.org/changeset/189890>
Comment 6 Ryosuke Niwa 2015-09-16 22:17:11 PDT 
The test added by this patch has been failing on Yosemite: https://bugs.webkit.org/show_bug.cgi?id=149246
Comment 7 Radar WebKit Bug Importer 2015-09-21 08:55:10 PDT 
<rdar://problem/22781974>