Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use custom font for stable version and copyright #1799

Merged
merged 2 commits into from
Jan 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/UnitTests/CoreText/data/MetadataTest-Regular.ttf
Git LFS file not shown
55 changes: 31 additions & 24 deletions tests/unittests/CoreText/CTFontTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,40 @@
#import <TestFramework.h>
#import <Foundation/Foundation.h>
#import <CoreText/CoreText.h>
#import <Starboard/SmartTypes.h>
#import <vector>

static NSURL* __GetURLFromPathRelativeToModuleDirectory(NSString* relativePath) {
static char fullPath[_MAX_PATH];
static int unused = [](char* path) { return GetModuleFileNameA(NULL, path, _MAX_PATH); }(fullPath);
return [NSURL fileURLWithPath:[[@(fullPath) stringByDeletingLastPathComponent] stringByAppendingPathComponent:relativePath]];
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will probably make @bbowman very unhappy.

class FontCopyName : public ::testing::TestWithParam<::testing::tuple<CFStringRef, const NSString*>> {
public:
FontCopyName() : _font(nullptr) {
}

protected:
virtual void SetUp() {
const CFStringRef fontName = static_cast<CFStringRef>(@"Segoe UI");
_font = CTFontCreateWithName(fontName, 0.0, NULL);
EXPECT_TRUE_MSG(_font != nil, "Failed: Font is nil.");
auto fontName = woc::MakeAutoCF<CFStringRef>(CFSTR("Metadata Test"));
_testFileURL = __GetURLFromPathRelativeToModuleDirectory(@"/data/MetadataTest-Regular.ttf");
CFErrorRef error = nullptr;
EXPECT_TRUE(CTFontManagerRegisterFontsForURL((__bridge CFURLRef)_testFileURL.get(), kCTFontManagerScopeSession, &error));
EXPECT_EQ(nullptr, error);

_font = woc::MakeAutoCF<CTFontRef>(CTFontCreateWithName(fontName, 20, nullptr));
ASSERT_NE(nullptr, _font);
}

virtual void TearDown() {
if (_font) {
CFRelease(_font);
}
CFErrorRef error = nullptr;
EXPECT_TRUE(CTFontManagerUnregisterFontsForURL((__bridge CFURLRef)_testFileURL.get(), kCTFontManagerScopeSession, &error));
EXPECT_EQ(nullptr, error);
}

CTFontRef _font;
woc::AutoCF<CTFontRef> _font;
StrongId<NSURL> _testFileURL;
};

TEST_P(FontCopyName, VerifyProperties) {
Expand All @@ -46,35 +59,28 @@ virtual void TearDown() {
CFRelease(propertyValue);
}

const NSString* c_copyrightName = @"© 2015 Microsoft Corporation. All Rights Reserved.";
const NSString* c_familyName = @"Segoe UI";
const NSString* c_familyName = @"Metadata Test";
const NSString* c_subFamilyName = @"Regular";
const NSString* c_styleName = @"Regular";
const NSString* c_uniqueName = @"Segoe UI Regular";
const NSString* c_fullName = @"Segoe UI";

// version name for Segoe UI font is different on ARM than on x86 platform, so we are using different version names for both platforms.
#ifdef _M_ARM
const NSString* c_versionName = @"Version 5.53; sf";
#else
const NSString* c_versionName = @"Version 5.53";
#endif
const NSString* c_uniqueName = @"Metadata Test Regular";
const NSString* c_fullName = @"Metadata Test";

const NSString* c_postscriptName = @"SegoeUI";
const NSString* c_trademarkName = @"Segoe is a trademark of the Microsoft group of companies.";
const NSString* c_postscriptName = @"MetadataTest-Regular";
const NSString* c_trademarkName = @"MetadataTest is not a trademark.";
const NSString* c_manufacturerName = @"Microsoft Corporation";
const NSString* c_designerName = nullptr;
const NSString* c_descriptionName = nullptr;
const NSString* c_vendorURLName = @"http://www.microsoft.com/typography/fonts/";
const NSString* c_vendorURLName = @"https://developer.microsoft.com/en-us/windows/bridges/ios";
const NSString* c_designerURLName = nullptr;
const NSString* c_licenseURLName = @"http://www.microsoft.com/typography/fonts/";
const NSString* c_licenseURLName = @"https://developer.microsoft.com/en-us/windows/bridges/ios";
const NSString* c_sampleTextName = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confused why you took this approach - why not just swap out the constants above for the ones in WinObjC.ttf, and reuse this parameterized test instead of adding a new one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(if it's that WinObjC.ttf currently lacks these other metadata, is there a way we could add them to WinObjC.ttf?)


In reply to: 97684882 [](ancestors = 97684882)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how difficult it would be to add the metadata, as that would make this much cleaner. @DHowett-MSFT ?

const NSString* c_copyrightName = @"Copyright (c) Microsoft";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aww, these moved around w.r.t. the true original. the diff could look so much prettier if Version was below fullName and copyright was above familyName! :P

const NSString* c_versionName = @"Version 1.01 ";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How strange that it leave a space at the end; I was even cautious to have it not do that.

const NSString* c_postscriptCIDName = nullptr;

INSTANTIATE_TEST_CASE_P(CTFont,
FontCopyName,
::testing::Values(::testing::make_tuple(kCTFontCopyrightNameKey, c_copyrightName),
::testing::make_tuple(kCTFontFamilyNameKey, c_familyName),
::testing::Values(::testing::make_tuple(kCTFontFamilyNameKey, c_familyName),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could revert this to the original completely. no point in taking a diff on an unnecessary reorganization

::testing::make_tuple(kCTFontSubFamilyNameKey, c_subFamilyName),
::testing::make_tuple(kCTFontStyleNameKey, c_styleName),
::testing::make_tuple(kCTFontUniqueNameKey, c_uniqueName),
Expand All @@ -88,6 +94,7 @@ virtual void TearDown() {
::testing::make_tuple(kCTFontDesignerURLNameKey, c_designerURLName),
::testing::make_tuple(kCTFontLicenseURLNameKey, c_licenseURLName),
::testing::make_tuple(kCTFontSampleTextNameKey, c_sampleTextName),
::testing::make_tuple(kCTFontCopyrightNameKey, c_copyrightName),
::testing::make_tuple(kCTFontVersionNameKey, c_versionName),
::testing::make_tuple(kCTFontPostScriptCIDNameKey, c_postscriptCIDName)));

Expand Down