From 2e105da555d89aef5da87406fef79159b0cdbc47 Mon Sep 17 00:00:00 2001 From: Bjorn Winckler Date: Sat, 17 Nov 2007 18:03:47 +0100 Subject: [PATCH] Fix a wide character bug When replacing characters in the text storage it could happen that a row 'lost' one column when a wide character was replaced with a normal one. --- src/MacVim/MMBackend.m | 2 +- src/MacVim/MMFullscreenWindow.m | 2 + src/MacVim/MMTextStorage.m | 241 +++++++++++++++++++++++++++++--- 3 files changed, 225 insertions(+), 20 deletions(-) diff --git a/src/MacVim/MMBackend.m b/src/MacVim/MMBackend.m index af2210f2ae..d273777027 100644 --- a/src/MacVim/MMBackend.m +++ b/src/MacVim/MMBackend.m @@ -997,7 +997,7 @@ enum { } } - NSLog(@"WARNING: No color with key %@ found.", stripKey); + //NSLog(@"WARNING: No color with key %@ found.", stripKey); return INVALCOLOR; } diff --git a/src/MacVim/MMFullscreenWindow.m b/src/MacVim/MMFullscreenWindow.m index df2febdb15..fe781677b1 100644 --- a/src/MacVim/MMFullscreenWindow.m +++ b/src/MacVim/MMFullscreenWindow.m @@ -11,6 +11,8 @@ * MMFullscreen * * Support for full-screen editing. + * + * Author: Nico Weber */ #import "MMFullscreenWindow.h" diff --git a/src/MacVim/MMTextStorage.m b/src/MacVim/MMTextStorage.m index d4fabf1477..7df3935085 100644 --- a/src/MacVim/MMTextStorage.m +++ b/src/MacVim/MMTextStorage.m @@ -11,6 +11,24 @@ * MMTextStorage * * Text rendering related code. + * + * Note that: + * - There are exactly 'actualRows' number of rows + * - Each row is terminated by an EOL character ('\n') + * - Each row must cover exactly 'actualColumns' display cells + * - The attribute "MMWideChar" denotes a character that covers two cells, a + * character without this attribute covers one cell + * - Unicode line (U+2028) and paragraph (U+2029) terminators are considered + * invalid and are replaced by spaces + * - Spaces are used to fill out blank spaces + * + * In order to locate a (row,col) pair it is in general necessary to search one + * character at a time. To speed things up we cache the length of each row, as + * well as the offset of the last column searched within each row. + * + * If each character in the text storage has length 1 and is not wide, then + * there is no need to search for a (row, col) pair since it can easily be + * computed. */ #import "MMTextStorage.h" @@ -18,6 +36,10 @@ +// Enable debug log messages for situations that should never occur. +#define MM_TS_PARANOIA_LOG 1 + + // TODO: What does DRAW_TRANSP flag do? If the background isn't drawn when // this flag is set, then sometimes the character after the cursor becomes @@ -37,7 +59,7 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; @interface MMTextStorage (Private) - (void)lazyResize:(BOOL)force; -- (NSRange)charRangeForRow:(int)row column:(int)col cells:(int)cells; +- (NSRange)charRangeForRow:(int)row column:(int*)col cells:(int*)cells; - (void)fixInvalidCharactersInRange:(NSRange)range; @end @@ -109,7 +131,9 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; - (void)replaceCharactersInRange:(NSRange)range withString:(NSString *)string { +#if MM_TS_PARANOIA_LOG NSLog(@"WARNING: calling %s on MMTextStorage is unsupported", _cmd); +#endif //[attribString replaceCharactersInRange:range withString:string]; } @@ -233,9 +257,13 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; return; // Find range of characters in text storage to replace. - NSRange range = [self charRangeForRow:row column:col cells:cells]; - if (NSMaxRange(range) > [[attribString string] length]) { - NSLog(@"%s Out of bounds"); + int acol = col; + int acells = cells; + NSRange range = [self charRangeForRow:row column:&acol cells:&acells]; + if (NSNotFound == range.location) { +#if MM_TS_PARANOIA_LOG + NSLog(@"INTERNAL ERROR [%s] Out of bounds", _cmd); +#endif return; } @@ -287,16 +315,50 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; [attribString replaceCharactersInRange:range withString:string]; [attribString setAttributes:attributes range:r]; + unsigned changeInLength = [string length] - range.length; + if (acells != cells || acol != col) { + if (acells == cells + 1) { + // NOTE: A normal width character replaced a double width + // character. To maintain the invariant that each row covers the + // same amount of cells, we compensate by adding an empty column. + [attribString replaceCharactersInRange:NSMakeRange(NSMaxRange(r),0) + withAttributedString:[emptyRowString + attributedSubstringFromRange:NSMakeRange(0,1)]]; + ++changeInLength; +#if 0 + } else if (acol == col - 1) { + NSLog(@"acol == col - 1"); + [attribString replaceCharactersInRange:NSMakeRange(r.location,0) + withAttributedString:[emptyRowString + attributedSubstringFromRange:NSMakeRange(0,1)]]; + ++changeInLength; + } else if (acol == col + 1) { + NSLog(@"acol == col + 1"); + [attribString replaceCharactersInRange:NSMakeRange(r.location-1,1) + withAttributedString:[emptyRowString + attributedSubstringFromRange:NSMakeRange(0,2)]]; + ++changeInLength; +#endif + } else { + // NOTE: It seems that this never gets called. If it ever does, + // then there is another case to treat. +#if MM_TS_PARANOIA_LOG + NSLog(@"row=%d col=%d acol=%d cells=%d acells=%d", row, col, acol, + cells, acells); +#endif + } + } + if ((flags & DRAW_WIDE) || [string length] != cells) characterEqualsColumn = NO; [self fixInvalidCharactersInRange:r]; [self edited:(NSTextStorageEditedCharacters|NSTextStorageEditedAttributes) - range:range changeInLength:[string length]-range.length]; + range:range changeInLength:changeInLength]; #if MM_USE_ROW_CACHE - rowCache[row].length += [string length] - range.length; + rowCache[row].length += changeInLength; #endif } @@ -322,9 +384,30 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; int i; for (i = 0; i < move; ++i, ++destRow) { - destRange = [self charRangeForRow:destRow column:left cells:width]; - srcRange = [self charRangeForRow:(destRow+count) column:left - cells:width]; + int acol = left; + int acells = width; + destRange = [self charRangeForRow:destRow column:&acol cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != width || acol != left) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + + acol = left; acells = width; + srcRange = [self charRangeForRow:(destRow+count) column:&acol + cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != width || acol != left) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + + if (NSNotFound == destRange.location || NSNotFound == srcRange.location) + { +#if MM_TS_PARANOIA_LOG + NSLog(@"INTERNAL ERROR [%s] Out of bounds", _cmd); +#endif + return; + } + NSAttributedString *srcString = [attribString attributedSubstringFromRange:srcRange]; @@ -347,7 +430,19 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; color, NSBackgroundColorAttributeName, nil]; for (i = 0; i < count; ++i, ++destRow) { - destRange = [self charRangeForRow:destRow column:left cells:width]; + int acol = left; + int acells = width; + destRange = [self charRangeForRow:destRow column:&acol cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != width || acol != left) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + if (NSNotFound == destRange.location) { +#if MM_TS_PARANOIA_LOG + NSLog(@"INTERNAL ERROR [%s] Out of bounds", _cmd); +#endif + return; + } [attribString replaceCharactersInRange:destRange withAttributedString:emptyString]; @@ -387,8 +482,28 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; int i; for (i = 0; i < move; ++i, --destRow, --srcRow) { - destRange = [self charRangeForRow:destRow column:left cells:width]; - srcRange = [self charRangeForRow:srcRow column:left cells:width]; + int acol = left; + int acells = width; + destRange = [self charRangeForRow:destRow column:&acol cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != width || acol != left) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + + acol = left; acells = width; + srcRange = [self charRangeForRow:srcRow column:&acol cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != width || acol != left) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + if (NSNotFound == destRange.location || NSNotFound == srcRange.location) + { +#if MM_TS_PARANOIA_LOG + NSLog(@"INTERNAL ERROR [%s] Out of bounds", _cmd); +#endif + return; + } + NSAttributedString *srcString = [attribString attributedSubstringFromRange:srcRange]; [attribString replaceCharactersInRange:destRange @@ -410,7 +525,19 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; color, NSBackgroundColorAttributeName, nil]; for (i = 0; i < count; ++i, --destRow) { - destRange = [self charRangeForRow:destRow column:left cells:width]; + int acol = left; + int acells = width; + destRange = [self charRangeForRow:destRow column:&acol cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != width || acol != left) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + if (NSNotFound == destRange.location) { +#if MM_TS_PARANOIA_LOG + NSLog(@"INTERNAL ERROR [%s] Out of bounds", _cmd); +#endif + return; + } [attribString replaceCharactersInRange:destRange withAttributedString:emptyString]; @@ -447,7 +574,19 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; int r; for (r=row1; r<=row2; ++r) { - range = [self charRangeForRow:r column:col1 cells:cells]; + int acol = col1; + int acells = cells; + range = [self charRangeForRow:r column:&acol cells:&acells]; +#if MM_TS_PARANOIA_LOG + if (acells != cells || acol != col1) + NSLog(@"INTERNAL ERROR [%s]", _cmd); +#endif + if (NSNotFound == range.location) { +#if MM_TS_PARANOIA_LOG + NSLog(@"INTERNAL ERROR [%s] Out of bounds", _cmd); +#endif + return; + } [attribString replaceCharactersInRange:range withAttributedString:emptyString]; @@ -650,7 +789,8 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; - (unsigned)characterIndexForRow:(int)row column:(int)col { - NSRange range = [self charRangeForRow:row column:col cells:1]; + int cells = 1; + NSRange range = [self charRangeForRow:row column:&col cells:&cells]; return range.location != NSNotFound ? range.location : 0; } @@ -737,7 +877,8 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; rect.size = cellSize; // Wide character take up twice the width of a normal character. - NSRange r = [self charRangeForRow:row column:col cells:1]; + int cells = 1; + NSRange r = [self charRangeForRow:row column:&col cells:&cells]; if (NSNotFound != r.location && [attribString attribute:MMWideCharacterAttributeName atIndex:r.location @@ -751,7 +892,8 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; // are drawn). NSLayoutManager *lm = [[self layoutManagers] objectAtIndex:0]; NSTextContainer *tc = [[lm textContainers] objectAtIndex:0]; - NSRange range = [self charRangeForRow:row column:col cells:1]; + int cells = 1; + NSRange range = [self charRangeForRow:row column:&col cells:&cells]; NSRange glyphRange = [lm glyphRangeForCharacterRange:range actualCharacterRange:NULL]; @@ -825,8 +967,11 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; range:oldRange changeInLength:fullRange.length-oldRange.length]; } -- (NSRange)charRangeForRow:(int)row column:(int)col cells:(int)cells +- (NSRange)charRangeForRow:(int)row column:(int*)pcol cells:(int*)pcells { + int col = *pcol; + int cells = *pcells; + // If no wide chars are used and if every char has length 1 (no composing // characters, no > 16 bit characters), then we can compute the range. if (characterEqualsColumn) @@ -834,13 +979,15 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; NSString *string = [attribString string]; NSRange r, range = { NSNotFound, 0 }; - unsigned idx; + unsigned idx, rowEnd; int i; if (row < 0 || row >= actualRows || col < 0 || col >= actualColumns || col+cells > actualColumns) { +#if MM_TS_PARANOIA_LOG NSLog(@"%s row=%d col=%d cells=%d is out of range (length=%d)", _cmd, row, col, cells, [string length]); +#endif return range; } @@ -850,6 +997,8 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; idx = 0; for (i = 0; i < row; ++i, ++cache) idx += cache->length; + + rowEnd = idx + cache->length; #else // Locate the beginning of the row by scanning for EOL characters. r.location = 0; @@ -873,6 +1022,7 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; } else { range.location = idx; +#if 0 // Backward search seems to be broken... // Cache miss if (col < i - col) { // Search forward from beginning of line. @@ -915,8 +1065,37 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; } } + *pcol = i; cache->col = i; cache->colOffset = idx - range.location; +#else + // Cache miss + if (col < i) { + // Search forward from beginning of line. + i = 0; + } else { + // Search forward from cache spot. + idx += cache->colOffset; + } + + // Forward search + while (col > i) { + r = [string rangeOfComposedCharacterSequenceAtIndex:idx]; + + // Wide chars take up two display cells. + if ([attribString attribute:MMWideCharacterAttributeName + atIndex:idx + effectiveRange:nil]) + ++i; + + idx += r.length; + ++i; + } + + *pcol = i; + cache->col = i; + cache->colOffset = idx - range.location; +#endif } #else idx = r.location; @@ -948,6 +1127,30 @@ static NSString *MMWideCharacterAttributeName = @"MMWideChar"; range.length += r.length; } + *pcells = i; + +#if MM_TS_PARANOIA_LOG +#if MM_USE_ROW_CACHE + if (range.location >= rowEnd-1) { + NSLog(@"INTERNAL ERROR [%s] : row=%d col=%d cells=%d --> range=%@", + _cmd, row, col, cells, NSStringFromRange(range)); + range.location = rowEnd - 2; + range.length = 1; + } else if (NSMaxRange(range) >= rowEnd) { + NSLog(@"INTERNAL ERROR [%s] : row=%d col=%d cells=%d --> range=%@", + _cmd, row, col, cells, NSStringFromRange(range)); + range.length = rowEnd - range.location - 1; + } +#endif + + if (NSMaxRange(range) > [string length]) { + NSLog(@"INTERNAL ERROR [%s] : row=%d col=%d cells=%d --> range=%@", + _cmd, row, col, cells, NSStringFromRange(range)); + range.location = NSNotFound; + range.length = 0; + } +#endif + return range; }