From b491db621b778f6ef04f07f80f57a383dff29bcf Mon Sep 17 00:00:00 2001 From: Jingtao Huang Date: Thu, 15 Dec 2016 13:55:05 -0800 Subject: [PATCH] Fix some CoreText perf low-hanging fruit - Remove NSLayoutManager __lineHasGlyphsAfterIndex(), which is called multiple times per line - Instead, directly search for the index of the last visible glyph once, compare against this - DWriteWrapper_CoreText - Skip first range of attributes in __DWriteTextLayoutCreate() - was redundant due to the underlaying Format already taking it into account - reserve() ahead of time for glyphRunDescriptionInfo._clusterMap, CTRun->_glyphOrigins, ->_glyphAdvances - A lot of time was being spent in reallocation, resizing - Remove DWriteWrapper _GetUserDefaultLocaleName(), don't wrap in a wstring, directly use a wchar_t buffer - Reduce the number of character buffer copies in _CFStringFromLocalizedString() by 1 - Remove unused/not useful _characters member from CTTypesetter - Misc Related to #1374 --- Frameworks/CoreGraphics/DWriteWrapper.mm | 43 ++++--- Frameworks/CoreText/CTFramesetter.mm | 2 +- Frameworks/CoreText/CTTypesetter.mm | 6 +- Frameworks/CoreText/DWriteWrapper_CoreText.mm | 17 ++- Frameworks/UIKit/NSLayoutManager.mm | 109 ++++++++++-------- .../include/CoreGraphics/DWriteWrapper.h | 4 - Frameworks/include/CoreTextInternal.h | 1 - build/CoreGraphics/dll/CoreGraphics.def | 1 - 8 files changed, 96 insertions(+), 87 deletions(-) diff --git a/Frameworks/CoreGraphics/DWriteWrapper.mm b/Frameworks/CoreGraphics/DWriteWrapper.mm index 6507997786..4f94f3e58a 100644 --- a/Frameworks/CoreGraphics/DWriteWrapper.mm +++ b/Frameworks/CoreGraphics/DWriteWrapper.mm @@ -96,19 +96,6 @@ HRESULT UpdateCollection(const ComPtr& dwriteFactory, const ComP return s_userFontCollection; } -/** - * Helper method to return the user set default locale string. - * - * @return use set locale string as wstring. - */ -std::wstring _GetUserDefaultLocaleName() { - wchar_t localeName[LOCALE_NAME_MAX_LENGTH]; - int defaultLocaleSuccess = GetUserDefaultLocaleName(localeName, LOCALE_NAME_MAX_LENGTH); - - // If the default locale is returned, find that locale name, otherwise use "en-us". - return std::wstring(defaultLocaleSuccess ? localeName : c_defaultUserLanguage); -} - /** * Helper method to convert IDWriteLocalizedStrings object to CFString object. * @@ -123,13 +110,15 @@ CFStringRef _CFStringFromLocalizedString(IDWriteLocalizedStrings* localizedStrin } // Get the default locale for this user. - std::wstring localeName = _GetUserDefaultLocaleName(); + // If the default locale is returned, find that locale name, otherwise use "en-us". + wchar_t localeNameBuf[LOCALE_NAME_MAX_LENGTH]; + int defaultLocaleSuccess = GetUserDefaultLocaleName(localeNameBuf, LOCALE_NAME_MAX_LENGTH); + const wchar_t* localeName = defaultLocaleSuccess ? localeNameBuf : c_defaultUserLanguage; uint32_t index = 0; BOOL exists = FALSE; - // If the default locale is returned, find that locale name, otherwise use "en-us". - RETURN_NULL_IF_FAILED(localizedString->FindLocaleName(localeName.c_str(), &index, &exists)); + RETURN_NULL_IF_FAILED(localizedString->FindLocaleName(localeName, &index, &exists)); if (!exists) { RETURN_NULL_IF_FAILED(localizedString->FindLocaleName(c_defaultUserLanguage, &index, &exists)); } @@ -143,13 +132,15 @@ CFStringRef _CFStringFromLocalizedString(IDWriteLocalizedStrings* localizedStrin uint32_t length = 0; RETURN_NULL_IF_FAILED(localizedString->GetStringLength(index, &length)); - // Get the string. - std::vector wcharString = std::vector(length + 1, 0); - RETURN_NULL_IF_FAILED(localizedString->GetString(index, wcharString.data(), length + 1)); + // Get the string. Use length + 1 here as GetString requires room for the null terminator. + woc::unique_iw wcharString(static_cast(IwMalloc(sizeof(wchar_t) * (length + 1)))); + RETURN_NULL_IF_FAILED(localizedString->GetString(index, wcharString.get(), length + 1)); // Strip out unnecessary null terminator - return (CFStringRef)CFAutorelease( - CFStringCreateWithCharacters(kCFAllocatorDefault, reinterpret_cast(wcharString.data()), length)); + return (CFStringRef)CFAutorelease(CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, + reinterpret_cast(wcharString.release()), + length, + kCFAllocatorDefault)); } /** @@ -280,6 +271,12 @@ HRESULT _DWriteCreateTextFormatWithFontNameAndSize(CFStringRef optionalFontName, userFontInfo = __GetUserFontCollectionHelper()->GetFontPropertiesFromUppercaseFontName(upperFontName); } + // Get the default locale for this user. + // If the default locale is returned, find that locale name, otherwise use "en-us". + wchar_t localeNameBuf[LOCALE_NAME_MAX_LENGTH]; + int defaultLocaleSuccess = GetUserDefaultLocaleName(localeNameBuf, LOCALE_NAME_MAX_LENGTH); + const wchar_t* localeName = defaultLocaleSuccess ? localeNameBuf : c_defaultUserLanguage; + if (info) { RETURN_IF_FAILED( dwriteFactory->CreateTextFormat(reinterpret_cast(Strings::VectorFromCFString(info->familyName.get()).data()), @@ -288,7 +285,7 @@ HRESULT _DWriteCreateTextFormatWithFontNameAndSize(CFStringRef optionalFontName, info->style, info->stretch, fontSize, - _GetUserDefaultLocaleName().data(), + localeName, &textFormat)); } @@ -301,7 +298,7 @@ HRESULT _DWriteCreateTextFormatWithFontNameAndSize(CFStringRef optionalFontName, userFontInfo->style, userFontInfo->stretch, fontSize, - _GetUserDefaultLocaleName().data(), + localeName, &textFormat)); } diff --git a/Frameworks/CoreText/CTFramesetter.mm b/Frameworks/CoreText/CTFramesetter.mm index 4f9f7d6d8a..c902247420 100644 --- a/Frameworks/CoreText/CTFramesetter.mm +++ b/Frameworks/CoreText/CTFramesetter.mm @@ -32,7 +32,7 @@ @implementation _CTFramesetter : NSObject // Call _DWriteWrapper to get _CTLine object list that makes up this frame _CTTypesetter* typesetter = static_cast<_CTTypesetter*>(framesetter->_typesetter); if (range.length == 0L) { - range.length = typesetter->_characters.size() - range.location; + range.length = [typesetter->_string length] - range.location; } StrongId<_CTFrame> ret = _DWriteGetFrame(static_cast(typesetter->_attributedString.get()), range, frameRect); diff --git a/Frameworks/CoreText/CTTypesetter.mm b/Frameworks/CoreText/CTTypesetter.mm index 322a5f871e..dea6ef63ee 100644 --- a/Frameworks/CoreText/CTTypesetter.mm +++ b/Frameworks/CoreText/CTTypesetter.mm @@ -33,10 +33,6 @@ - (instancetype)initWithAttributedString:(NSAttributedString*)str { _attributedString = str; _string = [str string]; - // Measure the string - _characters.resize(str.length); - [_string getCharacters:_characters.data()]; - return self; } @@ -109,7 +105,7 @@ CFIndex CTTypesetterSuggestLineBreak(CTTypesetterRef typesetter, CFIndex startIn CFIndex CTTypesetterSuggestLineBreakWithOffset(CTTypesetterRef ts, CFIndex index, double width, double offset) { _CTTypesetter* typesetter = static_cast<_CTTypesetter*>(ts); _CTFrame* frame = _DWriteGetFrame(static_cast(typesetter->_attributedString.get()), - CFRangeMake(index, typesetter->_characters.size() - index), + CFRangeMake(index, [typesetter->_string length] - index), CGRectMake(offset, 0, width, FLT_MAX)); return ([frame->_lines count] > 0) ? static_cast<_CTLine*>([frame->_lines firstObject])->_strRange.length : 0; } diff --git a/Frameworks/CoreText/DWriteWrapper_CoreText.mm b/Frameworks/CoreText/DWriteWrapper_CoreText.mm index 96ac4407b2..adcce634d8 100644 --- a/Frameworks/CoreText/DWriteWrapper_CoreText.mm +++ b/Frameworks/CoreText/DWriteWrapper_CoreText.mm @@ -265,11 +265,15 @@ static HRESULT __DWriteTextLayoutCreate(CFAttributedStringRef string, CFRange ra uint32_t incompatibleAttributeFlag = 0; CFRange attributeRange; - for (CFIndex currentIndex = range.location; currentIndex < rangeEnd; currentIndex += attributeRange.length) { - CTFontRef font = - static_cast(CFAttributedStringGetAttribute(string, currentIndex, kCTFontAttributeName, &attributeRange)); + // Find the range of the first set of attributes and skip it, since the underlying DWriteTextFormat has already internalized it + // If this first set of attributes lasts the entire range, the below for loop is not executed at all + // attributeRange is populated even if this attribute is not found + CFAttributedStringGetAttribute(string, range.location, kCTFontAttributeName, &attributeRange); - // attributeRange is properly populated even if this attribute is not found + for (CFIndex index = attributeRange.location + attributeRange.length; index < rangeEnd; index += attributeRange.length) { + CTFontRef font = static_cast(CFAttributedStringGetAttribute(string, index, kCTFontAttributeName, &attributeRange)); + + // attributeRange is populated even if this attribute is not found const DWRITE_TEXT_RANGE dwriteRange = { attributeRange.location, attributeRange.length }; if (font) { RETURN_IF_FAILED(__DWriteTextLayoutApplyFont(textLayout, font, dwriteRange)); @@ -282,7 +286,7 @@ static HRESULT __DWriteTextLayoutCreate(CFAttributedStringRef string, CFRange ra } CFNumberRef extraKerningRef = - static_cast(CFAttributedStringGetAttribute(string, currentIndex, kCTKernAttributeName, nullptr)); + static_cast(CFAttributedStringGetAttribute(string, index, kCTKernAttributeName, nullptr)); if (extraKerningRef) { RETURN_IF_FAILED(__DWriteTextLayoutApplyExtraKerning(textLayout, typography, extraKerningRef, dwriteRange)); } else { @@ -329,6 +333,7 @@ HRESULT STDMETHODCALLTYPE DrawGlyphRun(_In_ void* clientDrawingContext, _DWriteGlyphRunDescription glyphRunDescriptionInfo; glyphRunDescriptionInfo._stringLength = glyphRunDescription->stringLength; glyphRunDescriptionInfo._textPosition = glyphRunDescription->textPosition; + glyphRunDescriptionInfo._clusterMap.reserve(glyphRun->glyphCount); std::transform(glyphRunDescription->clusterMap, glyphRunDescription->clusterMap + glyphRun->glyphCount, std::back_inserter(glyphRunDescriptionInfo._clusterMap), @@ -479,6 +484,8 @@ HRESULT STDMETHODCALLTYPE GetPixelsPerDip(_In_opt_ void* clientDrawingContext, _ // TODO:: // This is a temp workaround until we can have actual glyph origins + run->_glyphOrigins.reserve(glyphRunDetails._dwriteGlyphRun[j].glyphCount); + run->_glyphAdvances.reserve(glyphRunDetails._dwriteGlyphRun[j].glyphCount); for (int index = 0; index < glyphRunDetails._dwriteGlyphRun[j].glyphCount; index++) { run->_glyphOrigins.emplace_back(CGPoint{ xPos, yPos }); run->_glyphAdvances.emplace_back(CGSize{ glyphRunDetails._dwriteGlyphRun[j].glyphAdvances[index], 0.0f }); diff --git a/Frameworks/UIKit/NSLayoutManager.mm b/Frameworks/UIKit/NSLayoutManager.mm index f008b6096d..f030fdefd5 100644 --- a/Frameworks/UIKit/NSLayoutManager.mm +++ b/Frameworks/UIKit/NSLayoutManager.mm @@ -34,22 +34,38 @@ @implementation NSLayoutManager { woc::unique_cf _frame; } -// Returns true if any of the characters in line after index have visible glyphs, false otherwise -// Used to determine if a given index is past the visible part of a line for linebreaking -static bool __lineHasGlyphsAfterIndex(CTLineRef line, CFIndex index) { - for (id runRef in static_cast(CTLineGetGlyphRuns(line))) { - CTRunRef run = static_cast(runRef); - CFRange runRange = CTRunGetStringRange(run); - if (runRange.location <= index && index <= runRange.location + runRange.length) { - const CFIndex* indices = CTRunGetStringIndicesPtr(run); - if (std::any_of(indices, - indices + CTRunGetGlyphCount(run), - std::bind(std::greater_equal(), std::placeholders::_1, index))) { - return true; +// Private helper: +// Given a CTLineRef, returns the index of the character in the original string, that maps to the last visible glyph in the line +static CFIndex __CTLineGetStringIndexOfLastVisibleGlyph(CTLineRef line) { + CFArrayRef runs = CTLineGetGlyphRuns(line); + + CFIndex ret = -1; + for (CFIndex i = CFArrayGetCount(runs) - 1; i >= 0; --i) { + CTRunRef run = static_cast(CFArrayGetValueAtIndex(runs, i)); + CFIndex glyphCount = CTRunGetGlyphCount(run); + + // Open question: Can runs ever be non-monotonic with respect to the original string? + // If not, can do an early return here + if (glyphCount > 0) { + const CFIndex* stringIndices = CTRunGetStringIndicesPtr(run); + + // Glyphs within a run can be non-monotonic with respect to the original string + // Find the greatest index within the run's string indices + const CFIndex* maxIndexInRun = std::max_element(stringIndices, stringIndices + glyphCount); + + if (*maxIndexInRun > ret) { + ret = *maxIndexInRun; } } } - return false; + + return ret; +} + +// Private helper for getting the width and height of a line in DIPs +static inline CGSize __CTLineGetBounds(CTLineRef line) { + CGFloat ascent, descent, leading; + return { CTLineGetTypographicBounds(line, &ascent, &descent, &leading), ascent + descent + leading }; } - (void)__layoutAllText { @@ -60,9 +76,6 @@ - (void)__layoutAllText { woc::unique_cf framesetter{ CTFramesetterCreateWithAttributedString( static_cast(_textStorage.get())) }; - NSTextContainer* container = (NSTextContainer*)_textContainers[0]; - CGSize containerSize = container.size; - CGPoint origin = { 0, 0 }; // Creates frame containing all of the text, which allows us to measure the line heights for measuring rectangles and breaks the text // into CTLineRefs for hard line breaks (e.g. '\n') which allows the assumption that once we run out of glyphs in a line to draw we must @@ -72,48 +85,52 @@ - (void)__layoutAllText { _frame.reset(CTFramesetterCreateFrame(framesetter.get(), {}, path.get(), nullptr)); } + NSTextContainer* container = (NSTextContainer*)_textContainers[0]; + const CGSize containerSize = container.size; + CGPoint origin{ 0, 0 }; + for (id lineRef in static_cast(CTFrameGetLines(_frame.get()))) { CTLineRef line = static_cast(lineRef); - // Width of line already drawn, saves us from having to redraw line twice - double drawnWidth = 0.0; + // Maximum height of lines on current horizontal, needed to get next yPos + const CGFloat lineHeight = __CTLineGetBounds(line).height; - CGFloat ascent, descent, leading; + const CFRange lineRange = CTLineGetStringRange(line); + const CFIndex lineEnd = lineRange.location + lineRange.length; // String index of last glyph in line - // Width of entire line - double width = CTLineGetTypographicBounds(line, &ascent, &descent, &leading); + const CFIndex indexOfLastVisibleChar = __CTLineGetStringIndexOfLastVisibleGlyph(line); // Alternate termination point - // Maximum height of lines on current horizontal, needed to get next yPos - CGFloat lineHeight = leading + ascent + descent; + CGRect proposedRect{}; // Proposes a drawing rectangle to [NSTextContainer lineFragmentRectForProposedRect] + proposedRect.size.width = containerSize.width; + proposedRect.size.height = lineHeight; - CFRange lineRange = CTLineGetStringRange(line); + CGRect frameDrawRect{}; // Drawing rectangle actually used in CTFramesetterCreateFrame to layout the text + frameDrawRect.size.height = lineHeight; - // Index of first glyph in line to be drawn - CFIndex stringIndex = lineRange.location; - CFIndex lineEnd = stringIndex + lineRange.length; + double drawnWidth = 0.0; // Width of line already drawn, saves us from having to redraw line twice + CFIndex stringIndex = lineRange.location; // String index of first glyph in line to be drawn while (stringIndex < lineEnd) { if (origin.y > containerSize.height) { // Added as much text as can fit in the frame, can end here return; } - if (!__lineHasGlyphsAfterIndex(line, stringIndex)) { - // Ended up without any visible glyphs to draw - // Caused by a line of only whitespace + if (stringIndex > indexOfLastVisibleChar) { + // Ended up without any visible glyphs to draw, caused by a line of only whitespace origin = { 0.0f, origin.y + lineHeight }; - _totalSize.height += lineHeight; break; } - CGRect remainingRect = {}; - CGRect rect = [container lineFragmentRectForProposedRect:CGRectMake(origin.x, origin.y, containerSize.width, lineHeight) - atIndex:stringIndex - writingDirection:NSWritingDirectionLeftToRight - remainingRect:&remainingRect]; + CGRect remainingRect{}; + proposedRect.origin = origin; + const CGRect rect = [container lineFragmentRectForProposedRect:proposedRect + atIndex:stringIndex + writingDirection:NSWritingDirectionLeftToRight + remainingRect:&remainingRect]; // Approximate how many characters can fit to keep from re-rendering large amounts of text - CFIndex lastIndex = std::min(CTLineGetStringIndexForPosition(line, { drawnWidth + rect.size.width, 0.0f }) + 1, lineEnd); + const CFIndex lastIndex = std::min(CTLineGetStringIndexForPosition(line, { drawnWidth + rect.size.width, 0.0f }) + 1, lineEnd); if (lastIndex == stringIndex) { // Unable to fit any text in the current rect, continue to next @@ -121,13 +138,13 @@ - (void)__layoutAllText { origin = { remainingRect.origin.x, origin.y }; } else { origin = { 0.0f, origin.y + lineHeight }; - _totalSize.height += lineHeight; } continue; } - woc::unique_cf path{ CGPathCreateWithRect(CGRectMake(0, 0, rect.size.width, lineHeight), nullptr) }; + frameDrawRect.size.width = rect.size.width; + woc::unique_cf path{ CGPathCreateWithRect(frameDrawRect, nullptr) }; woc::unique_cf frame{ CTFramesetterCreateFrame(framesetter.get(), { stringIndex, lastIndex - stringIndex }, path.get(), nullptr) }; @@ -135,42 +152,40 @@ - (void)__layoutAllText { // Create line to fit as much text as possible in given rect CTLineRef fitLine = static_cast(CFArrayGetValueAtIndex(CTFrameGetLines(frame.get()), 0)); - CFIndex fitLength = CTLineGetStringRange(fitLine).length; + const CFIndex fitLength = CTLineGetStringRange(fitLine).length; if (fitLength == 0L) { // Failed to fit any text in the current rect, continue to next if (remainingRect.size.width > 0.0f && stringIndex < lineEnd) { origin = { remainingRect.origin.x, origin.y }; } else { origin = { 0.0f, origin.y + lineHeight }; - _totalSize.height += lineHeight; } continue; } - - // Increase index of next character to layout stringIndex += fitLength; // Save line and origin for when it is drawn [_ctLines addObject:(id)fitLine]; _lineOrigins.emplace_back(CGPoint{ rect.origin.x, rect.origin.y + lineHeight }); - double fitWidth = CTLineGetTypographicBounds(fitLine, nullptr, nullptr, nullptr); + const double fitWidth = CTLineGetTypographicBounds(fitLine, nullptr, nullptr, nullptr); drawnWidth += fitWidth; if (remainingRect.size.width > 0 && stringIndex < lineEnd) { origin = { remainingRect.origin.x, origin.y }; } else { origin = { 0.0f, origin.y + lineHeight }; - _totalSize.height += lineHeight; - _totalSize.width = std::max(_totalSize.width, rect.origin.x + (CGFloat)fitWidth); - if (!__lineHasGlyphsAfterIndex(line, stringIndex)) { + _totalSize.width = std::max(_totalSize.width, rect.origin.x + fitWidth); + if (stringIndex > indexOfLastVisibleChar) { // Ended up with whitespace at end of the line, break to next line break; } } } } + + _totalSize.height = origin.y; } - (void)layoutIfNeeded { diff --git a/Frameworks/include/CoreGraphics/DWriteWrapper.h b/Frameworks/include/CoreGraphics/DWriteWrapper.h index fb9f3b79dd..74082dd0dc 100644 --- a/Frameworks/include/CoreGraphics/DWriteWrapper.h +++ b/Frameworks/include/CoreGraphics/DWriteWrapper.h @@ -29,10 +29,6 @@ #import // General DWrite helpers -#ifdef __cplusplus -extern "C++" std::wstring _GetUserDefaultLocaleName(); -#endif - static inline CFStringRef _CFStringCreateUppercaseCopy(CFStringRef string) { CFMutableStringRef ret = CFStringCreateMutableCopy(nullptr, CFStringGetLength(string), string); CFStringUppercase(ret, nullptr); diff --git a/Frameworks/include/CoreTextInternal.h b/Frameworks/include/CoreTextInternal.h index 57ad684a56..dd874914aa 100644 --- a/Frameworks/include/CoreTextInternal.h +++ b/Frameworks/include/CoreTextInternal.h @@ -50,7 +50,6 @@ inline void _SafeRelease(T** p) { @public StrongId _attributedString; StrongId _string; - std::vector _characters; } @end diff --git a/build/CoreGraphics/dll/CoreGraphics.def b/build/CoreGraphics/dll/CoreGraphics.def index 569d4a9991..8d178608a1 100644 --- a/build/CoreGraphics/dll/CoreGraphics.def +++ b/build/CoreGraphics/dll/CoreGraphics.def @@ -37,7 +37,6 @@ LIBRARY CoreGraphics CGBitmapContextGetImage ; DWriteWrapper Additions - _GetUserDefaultLocaleName _CFStringFromLocalizedString _DWriteCopyFontFamilyNames _DWriteCopyFontNamesForFamilyName