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

Re-Implementation of reverse string via CFString #2479

Merged
merged 4 commits into from
Apr 20, 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
37 changes: 34 additions & 3 deletions Frameworks/CoreText/CTLine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,46 @@

#import <CoreText/CTLine.h>
#import <StubReturn.h>
#import "NSStringInternal.h"
#import "CoreTextInternal.h"
#import "CGContextInternal.h"
#import "DWriteWrapper_CoreText.h"
#import <CoreText/CTTypesetter.h>
#import <CoreFoundation/CFString.h>

#include <memory>
#import <algorithm>
#import <numeric>
#import <cwchar>
#import <vector>

static CFStringRef __CTCreateReversedString(CFStringRef string) {
Copy link
Contributor

@aballway aballway Apr 11, 2017

Choose a reason for hiding this comment

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

how is this change being tested? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnitTests


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

if (string == nullptr) {
return nullptr;
}

CFIndex length = CFStringGetLength(string);
CFIndex usedBufLen;
CFStringGetBytes(string, CFRangeMake(0, length), kCFStringEncodingUTF16, 0, false, nullptr, length, &usedBufLen);
Copy link
Contributor

@ms-jihua ms-jihua Apr 11, 2017

Choose a reason for hiding this comment

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

Seems kind of expensive for this purpose (unless CFStringGetBytes has a fastpath here?)

I wonder if CFStringGetMaximumSizeForEncoding() would be faster/would still work? #ByDesign

Copy link
Contributor Author

@msft-Jeyaram msft-Jeyaram Apr 11, 2017

Choose a reason for hiding this comment

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

Yes it does take a faster way than if you provide a buffer.
Further improvements can be addressed by a bug to improve core text. The CR is mainly to remove Foundation dependency and what we have now is better than the earlier Foundation/copy based implementation.
#ByDesign


if (length < 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: move this above CFStringGetBytes call

return CFStringCreateCopy(kCFAllocatorDefault, string);
}

CFIndex bufLen = (usedBufLen / sizeof(UniChar));
std::unique_ptr<UniChar[]> characters(new UniChar[bufLen + 1]);
characters[bufLen] = L'\0';

CFStringGetBytes(
string, CFRangeMake(0, length), kCFStringEncodingUTF16, 0, false, reinterpret_cast<UInt8*>(characters.get()), usedBufLen, nullptr);

wchar_t* result = _wcsrev(reinterpret_cast<wchar_t*>(characters.get()));
if (result == nullptr) {
return nullptr;
}

return CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, characters.release(), bufLen, nullptr);
}

static NSMutableAttributedString* _getTruncatedStringFromSourceLine(CTLineRef line,
CTLineTruncationType truncationType,
double widthToExtract);
Expand Down Expand Up @@ -187,8 +217,9 @@ CTLineRef CTLineCreateTruncatedLine(CTLineRef sourceLine, double width, CTLineTr
CFDictionaryRef attribs = CTRunGetAttributes(run);

if (truncationType == kCTLineTruncationStart) {
NSString* reverse = [runString _reverseString];
NSAttributedString* string = [[NSAttributedString alloc] initWithString:reverse attributes:(NSDictionary*)attribs];
auto reverse = woc::MakeStrongCF<CFStringRef>(__CTCreateReversedString(static_cast<CFStringRef>(runString)));
NSAttributedString* string =
[[NSAttributedString alloc] initWithString:static_cast<NSString*>(reverse.get()) attributes:(NSDictionary*)attribs];
[ret insertAttributedString:string atIndex:0];
[string release];
} else if (truncationType == kCTLineTruncationEnd) {
Expand Down
4 changes: 2 additions & 2 deletions Frameworks/CoreText/CTUtilities.mm
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//******************************************************************************
//
// Copyright (c) 2016 Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
Expand All @@ -23,4 +23,4 @@
*/
uint32_t CTGetCoreTextVersion() {
return kCTVersionNumber10_5;
}
}
23 changes: 1 addition & 22 deletions Frameworks/Foundation/NSString.mm
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//******************************************************************************
//
// Copyright (c) 2015 Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
Expand Down Expand Up @@ -2200,25 +2200,4 @@ - (int)_versionStringCompare:(NSString*)compStrAddr {

return result;
}

- (NSString*)_reverseString {
NSUInteger length = [self length];
if (length < 2) {
return self;
}

std::vector<char> characters(length + 1);
[self getCString:&characters[0] maxLength:length];
for (int i = 0; i < length / 2; ++i) {
char character = characters[length - i - 1];
characters[length - i - 1] = characters[i];
characters[i] = character;
}
characters[length] = '\0';

NSString* ret = [[[NSString alloc] initWithCString:&characters[0]] autorelease];

return ret;
}

@end
2 changes: 1 addition & 1 deletion Frameworks/include/CFFoundationInternal.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//******************************************************************************
//
// Copyright (c) 2015 Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

These clean-ups, though nice, seem unrelated to this PR. Just revert these files?

//
// This code is licensed under the MIT License (MIT).
//
Expand Down
3 changes: 1 addition & 2 deletions Frameworks/include/NSStringInternal.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//******************************************************************************
//
// Copyright (c) 2015 Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
Expand All @@ -22,7 +22,6 @@

@interface NSString ()
- (int)_versionStringCompare:(NSString*)compStrAddr;
- (NSString*)_reverseString;
@end

@interface NSString (WinObjCHSTRINGAdditions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@
<ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\WindowsOnly\NSObjectInternalTests.mm" />
<ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\WindowsOnly\NSPointerFunctionsInternalTests.mm" />
<ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\WindowsOnly\NSRecursiveLockInternalTests.mm" />
<ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\WindowsOnly\NSStringInternalTests.mm" />
<ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\WindowsOnly\NSPointerArrayInternalTests.mm" />
<ClangCompile Include="$(StarboardBasePath)\tests\unittests\Foundation\WindowsOnly\ArchivalInternalTests.mm" />
</ItemGroup>
Expand Down
7 changes: 3 additions & 4 deletions include/CoreText/CTUtilities.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//******************************************************************************
//
// Copyright (c) 2016 Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft. All rights reserved.
//
// This code is licensed under the MIT License (MIT).
//
Expand All @@ -16,13 +16,12 @@
#pragma once

#import <CoreText/CoreTextExport.h>

#include <stdint.h>
#import <stdint.h>

CORETEXT_EXPORT uint32_t CTGetCoreTextVersion();

#define kCTVersionNumber10_5 0x00020000
#define kCTVersionNumber10_5_2 0x00020001
#define kCTVersionNumber10_5_3 0x00020002
#define kCTVersionNumber10_5_5 0x00020003
#define kCTVersionNumber10_6 0x00030000
#define kCTVersionNumber10_6 0x00030000
45 changes: 0 additions & 45 deletions tests/unittests/Foundation/WindowsOnly/NSStringInternalTests.mm

This file was deleted.