From 6238e5edbde55fb82982ce4a19a0cbca7fe7eb9d Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Sat, 9 Jul 2016 15:40:31 -0700 Subject: [PATCH] [ASTextNode] Fix text node truncation (#1863) * Before truncate a text storage in ASTextKitContext reset the text storage to original value * Fix ASTextNode tests We should pass in the constrained size in both cases and the sizes should be the same. We adjust the calculated size in ASTextNode to be a bit narrower in the second case if we truncate again with the calculated size as constrained size it will truncate more and the resulting size will shrink. --- AsyncDisplayKit/TextKit/ASTextKitContext.h | 13 +++++++- AsyncDisplayKit/TextKit/ASTextKitContext.mm | 30 ++++++++++++++++++- AsyncDisplayKit/TextKit/ASTextKitRenderer.mm | 8 +++-- .../TextKit/ASTextKitTailTruncater.mm | 9 ++++-- AsyncDisplayKitTests/ASTextNodeTests.m | 4 +-- 5 files changed, 56 insertions(+), 8 deletions(-) diff --git a/AsyncDisplayKit/TextKit/ASTextKitContext.h b/AsyncDisplayKit/TextKit/ASTextKitContext.h index ccd7bb0eb9..61488b4a98 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitContext.h +++ b/AsyncDisplayKit/TextKit/ASTextKitContext.h @@ -10,6 +10,8 @@ #import +typedef NSTextStorage *(^ASTextKitContextTextStorageCreationBlock)(NSAttributedString *attributedString); + /** A threadsafe container for the TextKit components that ASTextKit uses to lay out and truncate its text. @@ -30,10 +32,19 @@ constrainedSize:(CGSize)constrainedSize layoutManagerCreationBlock:(NSLayoutManager * (^)(void))layoutCreationBlock layoutManagerDelegate:(id)layoutManagerDelegate - textStorageCreationBlock:(NSTextStorage * (^)(NSAttributedString *attributedString))textStorageCreationBlock; + textStorageCreationBlock:(ASTextKitContextTextStorageCreationBlock)textStorageCreationBlock; +/** + Set the constrained size for the text context. + */ @property (nonatomic, assign, readwrite) CGSize constrainedSize; +/** + Resets the text storage to the original value in case it was truncated before. This method is called within + a locked context. + */ +- (void)resetTextStorage; + /** All operations on TextKit values MUST occur within this locked context. Simultaneous access (even non-mutative) to TextKit components may cause crashes. diff --git a/AsyncDisplayKit/TextKit/ASTextKitContext.mm b/AsyncDisplayKit/TextKit/ASTextKitContext.mm index 7410ed9f45..7f5ed9be98 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitContext.mm +++ b/AsyncDisplayKit/TextKit/ASTextKitContext.mm @@ -21,8 +21,12 @@ NSLayoutManager *_layoutManager; NSTextStorage *_textStorage; NSTextContainer *_textContainer; + + NSAttributedString *_attributedString; } +#pragma mark - Lifecycle + - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString lineBreakMode:(NSLineBreakMode)lineBreakMode maximumNumberOfLines:(NSUInteger)maximumNumberOfLines @@ -37,16 +41,23 @@ // Concurrently initialising TextKit components crashes (rdar://18448377) so we use a global lock. static std::mutex __static_mutex; std::lock_guard l(__static_mutex); + + _attributedString = [attributedString copy]; + // Create the TextKit component stack with our default configuration. if (textStorageCreationBlock) { _textStorage = textStorageCreationBlock(attributedString); } else { - _textStorage = (attributedString ? [[NSTextStorage alloc] initWithAttributedString:attributedString] : [[NSTextStorage alloc] init]); + _textStorage = [[NSTextStorage alloc] init]; + [self _resetTextStorage]; } + + _layoutManager = layoutCreationBlock ? layoutCreationBlock() : [[ASLayoutManager alloc] init]; _layoutManager.usesFontLeading = NO; _layoutManager.delegate = layoutManagerDelegate; [_textStorage addLayoutManager:_layoutManager]; + _textContainer = [[NSTextContainer alloc] initWithSize:constrainedSize]; // We want the text laid out up to the very edges of the container. _textContainer.lineFragmentPadding = 0; @@ -58,6 +69,21 @@ return self; } +#pragma mark - Text Storage + +- (void)resetTextStorage +{ + std::lock_guard l(_textKitMutex); + [self _resetTextStorage]; +} + +- (void)_resetTextStorage +{ + [_textStorage setAttributedString:_attributedString]; +} + +#pragma mark - Setter / Getter + - (CGSize)constrainedSize { std::lock_guard l(_textKitMutex); @@ -70,6 +96,8 @@ _textContainer.size = constrainedSize; } +#pragma mark - Locking + - (void)performBlockWithLockedTextKitComponents:(void (^)(NSLayoutManager *, NSTextStorage *, NSTextContainer *))block diff --git a/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm b/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm index 05463f39e4..ef32d0dc40 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm +++ b/AsyncDisplayKit/TextKit/ASTextKitRenderer.mm @@ -129,8 +129,12 @@ static NSCharacterSet *_defaultAvoidTruncationCharacterSet() // If we're updating an existing context, make sure to use the same inset logic used during initialization. // This codepath allows us to reuse the CGSize shadowConstrainedSize = [[self shadower] insetSizeWithConstrainedSize:constrainedSize]; - if (_context) _context.constrainedSize = shadowConstrainedSize; - if (_fontSizeAdjuster) _fontSizeAdjuster.constrainedSize = shadowConstrainedSize; + if (_context) { + _context.constrainedSize = shadowConstrainedSize; + } + if (_fontSizeAdjuster) { + _fontSizeAdjuster.constrainedSize = shadowConstrainedSize; + } } } } diff --git a/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm b/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm index 57a9eb2f27..e44ed03ef5 100755 --- a/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm +++ b/AsyncDisplayKit/TextKit/ASTextKitTailTruncater.mm @@ -152,9 +152,14 @@ - (void)truncate { - [_context performBlockWithLockedTextKitComponents:^(NSLayoutManager *layoutManager, NSTextStorage *textStorage, NSTextContainer *textContainer) { - NSUInteger originalStringLength = textStorage.length; + // Reset the text storage to start always with the full string + [_context resetTextStorage]; + // Start truncation + [_context performBlockWithLockedTextKitComponents:^(NSLayoutManager *layoutManager, NSTextStorage *textStorage, NSTextContainer *textContainer) { + + NSUInteger originalStringLength = textStorage.length; + [layoutManager ensureLayoutForTextContainer:textContainer]; NSRange visibleGlyphRange = [layoutManager glyphRangeForBoundingRect:{ .size = textContainer.size } diff --git a/AsyncDisplayKitTests/ASTextNodeTests.m b/AsyncDisplayKitTests/ASTextNodeTests.m index 6990b786d5..e928a22523 100644 --- a/AsyncDisplayKitTests/ASTextNodeTests.m +++ b/AsyncDisplayKitTests/ASTextNodeTests.m @@ -125,7 +125,7 @@ static BOOL CGSizeEqualToSizeWithIn(CGSize size1, CGSize size2, CGFloat delta) for (NSInteger i = 10; i < 500; i += 50) { CGSize constrainedSize = CGSizeMake(i, i); CGSize calculatedSize = [_textNode measure:constrainedSize]; - CGSize recalculatedSize = [_textNode measure:calculatedSize]; + CGSize recalculatedSize = [_textNode measure:constrainedSize]; XCTAssertTrue(CGSizeEqualToSizeWithIn(calculatedSize, recalculatedSize, 4.0), @"Recalculated size %@ should be same as original size %@", NSStringFromCGSize(recalculatedSize), NSStringFromCGSize(calculatedSize)); } @@ -136,7 +136,7 @@ static BOOL CGSizeEqualToSizeWithIn(CGSize size1, CGSize size2, CGFloat delta) for (CGFloat i = 10; i < 500; i *= 1.3) { CGSize constrainedSize = CGSizeMake(i, i); CGSize calculatedSize = [_textNode measure:constrainedSize]; - CGSize recalculatedSize = [_textNode measure:calculatedSize]; + CGSize recalculatedSize = [_textNode measure:constrainedSize]; XCTAssertTrue(CGSizeEqualToSizeWithIn(calculatedSize, recalculatedSize, 11.0), @"Recalculated size %@ should be same as original size %@", NSStringFromCGSize(recalculatedSize), NSStringFromCGSize(calculatedSize)); }