Skip to content

Commit fe2305d

Browse files
committed
Fix composing characters renderered at an offset
MacVim previously didn't really render composing characters with multiple glyphs correctly. For simple ones like 'â' it would work fine because Core Text just generates one glyph for it, but for more complicated ones like 'x⃗' the renderer didn't properly query the advance of the glyphs to put them at the correct position. Refactor the logic to keep track of the current cell/glyphs and make sure to track the advances fo the glyphs so the composing chars' glyphs would be laid out properly on top of the cell. We need to be careful with the tracking, because in Vim we force the text rendering to be monospaced, and so we maintain the tracking within a single grapheme (which can consist of multiple glyphs), but when we move to the next grapheme we reset the position so we can get proper monospaced rendering even for non-monospaced texts like CJK or stylized texts. Fix #995 Fix #1172
1 parent 49e7a1d commit fe2305d

1 file changed

Lines changed: 97 additions & 15 deletions

File tree

src/MacVim/MMCoreTextView.m

Lines changed: 97 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ - (void)invertBlockFromRow:(int)row column:(int)col numRows:(int)nrows
142142
int fraction;
143143
} GridCellInsertionPoint;
144144

145+
/// A cell in the grid. Each cell represents a grapheme, which could consist of one or more
146+
/// characters. If textFlags contains DRAW_WIDE, then it's a 'wide' cell, which means a grapheme
147+
/// takes up two cell spaces to render (e.g. emoji or CJK characters). When this is the case, the
148+
/// next cell in the grid should be ignored and skipped.
145149
typedef struct {
146150
// Note: All objects should be weak references.
147151
// Fields are grouped by draw order.
@@ -160,7 +164,7 @@ - (void)invertBlockFromRow:(int)row column:(int)col numRows:(int)nrows
160164
unsigned fg;
161165
unsigned sp;
162166
int textFlags;
163-
NSString* string; // Owned by characterStrings.
167+
NSString* string; ///< Owned by characterStrings. Length would be >1 if there are composing chars.
164168
} GridCell;
165169

166170
typedef struct {
@@ -786,14 +790,19 @@ - (void)drawRect:(NSRect)rect
786790
__block CFRange lineStringRange = {};
787791
__block GridCell lastStringCell = {};
788792
void (^flushLineString)() = ^{
793+
// This function flushes the current pending line out to be rendered. When ligature is
794+
// enabled it could be quite long. Otherwise, lineString would be just one cell/grapheme. Note
795+
// that even one cell can have lineString.length > 1 and also multiple glyphs due to
796+
// composing characters (limited by Vim's 'maxcombine').
789797
if (!lineString.length)
790798
return;
791-
CGPoint positionsByIndex[lineString.length];
799+
size_t cellOffsetByIndex[lineString.length];
792800
for (size_t i = 0, stringIndex = 0; i < lineStringRange.length; i++) {
793801
GridCell cell = *grid_cell(&grid, r, lineStringRange.location + i);
794802
size_t cell_length = cell.string.length;
795-
for (size_t j = 0; j < cell_length; j++)
796-
positionsByIndex[stringIndex++] = CGPointMake(i * cellSize.width, 0);
803+
for (size_t j = 0; j < cell_length; j++) {
804+
cellOffsetByIndex[stringIndex++] = i;
805+
}
797806
if (cell.textFlags & DRAW_WIDE)
798807
i++;
799808
}
@@ -807,26 +816,98 @@ - (void)drawRect:(NSRect)rect
807816
if ([glyphRuns count] == 0) {
808817
ASLogDebug(@"CTLineGetGlyphRuns no glyphs for: %@", lineString);
809818
}
819+
820+
CGSize accumAdvance = CGSizeZero; // Accumulated advance for the currently cell's glyphs (we can get more than one glyph when we have composing chars)
821+
CGPoint expectedGlyphPosition = CGPointZero; // The expected layout glyph position produced by CTLine
822+
size_t curCell = -1; // The current cell offset within lineStrangeRange
823+
810824
for (id obj in glyphRuns) {
811825
CTRunRef run = (CTRunRef)obj;
812826
CFIndex glyphCount = CTRunGetGlyphCount(run);
813-
CFIndex indices[glyphCount];
827+
828+
CTFontRef runFont = CFDictionaryGetValue(CTRunGetAttributes(run), kCTFontAttributeName);
829+
if (!runFont) {
830+
ASLogDebug(@"Null font for rendering. glyphCount: %ld", (long)glyphCount);
831+
}
832+
814833
CGPoint positions[glyphCount];
815-
CGGlyph glyphs[glyphCount];
816-
CTRunGetStringIndices(run, CFRangeMake(0, 0), indices);
817-
CTRunGetGlyphs(run, CFRangeMake(0, 0), glyphs);
834+
835+
CFIndex indices_storage[glyphCount];
836+
const CFIndex* indices = NULL;
837+
if ((indices = CTRunGetStringIndicesPtr(run)) == NULL) {
838+
CTRunGetStringIndices(run, CFRangeMake(0, 0), indices_storage);
839+
indices = indices_storage;
840+
}
841+
842+
const CGGlyph* glyphs = NULL;
843+
CGGlyph glyphs_storage[glyphCount];
844+
if ((glyphs = CTRunGetGlyphsPtr(run)) == NULL) {
845+
CTRunGetGlyphs(run, CFRangeMake(0, 0), glyphs_storage);
846+
glyphs = glyphs_storage;
847+
}
848+
849+
const CGSize* advances = NULL;
850+
CGSize advances_storage[glyphCount];
851+
if ((advances = CTRunGetAdvancesPtr(run)) == NULL) {
852+
CTRunGetAdvances(run, CFRangeMake(0, 0), advances_storage);
853+
advances = advances_storage;
854+
}
855+
856+
const CGPoint* layoutPositions = CTRunGetPositionsPtr(run);
857+
CGPoint layoutPositions_storage[glyphCount];
858+
if (layoutPositions == NULL) {
859+
CTRunGetPositions(run, CFRangeMake(0, 0), layoutPositions_storage);
860+
layoutPositions = layoutPositions_storage;
861+
}
862+
818863
for (CFIndex i = 0; i < glyphCount; i++) {
819864
if (indices[i] >= lineStringLength) {
820865
ASLogDebug(@"Invalid glyph pos index: %ld, len: %lu", (long)indices[i], (unsigned long)lineStringLength);
821866
continue;
822867
}
823-
positions[i] = positionsByIndex[indices[i]];
824-
}
825-
CTFontRef font = CFDictionaryGetValue(CTRunGetAttributes(run), kCTFontAttributeName);
826-
if (!font) {
827-
ASLogDebug(@"Null font for rendering. glyphCount: %ld", (long)glyphCount);
868+
if (curCell != -1 && curCell == cellOffsetByIndex[indices[i]]) {
869+
// We are still in the same cell/grapheme as last glyph. This usually only happens
870+
// when we have 1 or more composing characters (e.g. U+20E3 or U+20D7), and
871+
// Core Text decides to render them as separate glyphs instead of a single
872+
// one (e.g. 'â' will result in a single glyph instead).
873+
//
874+
// Don't do anything to allow the last glyph's advance to accumulate.
875+
} else {
876+
// We are in a new cell/grapheme with a new string to render. This is the
877+
// normal case.
878+
// In this situation we reset the accumulated advances because we render
879+
// every cell aligned to the grid and force everything to a monospace.
880+
accumAdvance = CGSizeZero;
881+
curCell = cellOffsetByIndex[indices[i]];
882+
}
883+
884+
// Align the position to the grid cell. Ignore what the typesetter wants (we
885+
// should be using a monospace font anyway, but if you are say entering CJK
886+
// characters font substitution will result in a non-monospaced typesetting)
887+
const CGPoint curCellPosition = CGPointMake(curCell * cellSize.width, 0);
888+
positions[i] = curCellPosition;
889+
890+
// Add the accumulated advances, which would be non-zero if this is not the
891+
// first glyph for this cell/character (due to composing chars).
892+
positions[i].x += accumAdvance.width;
893+
positions[i].y += accumAdvance.height;
894+
895+
// The expected glyph position should usually match what the layout wants to do.
896+
// Sometimes though the typesetter will offset it slightly (e.g. when rendering
897+
// 'x゙⃣', the first 'x' will be offsetted to give space to the later composing
898+
// chars. Since we are manually rendering to curCellPosition to align to a grid,
899+
// we take the offset and apply that instead of directly using layoutPositions.
900+
positions[i].x += (layoutPositions[i].x - expectedGlyphPosition.x);
901+
positions[i].y += (layoutPositions[i].y - expectedGlyphPosition.y);
902+
903+
// Accumulate the glyph's advance
904+
accumAdvance.width += advances[i].width;
905+
accumAdvance.height += advances[i].height;
906+
expectedGlyphPosition.x += advances[i].width;
907+
expectedGlyphPosition.y += advances[i].height;
908+
828909
}
829-
CTFontDrawGlyphs(font, glyphs, positions, glyphCount, ctx);
910+
CTFontDrawGlyphs(runFont, glyphs, positions, glyphCount, ctx);
830911
}
831912

832913
CGContextSetBlendMode(ctx, kCGBlendModeCopy);
@@ -976,7 +1057,8 @@ - (void)drawRect:(NSRect)rect
9761057

9771058
// Text strikethrough
9781059
// We delay the rendering of strikethrough and only do it as a second-pass since we want to draw them on top
979-
// of text, and text rendering is currently delayed via flushLineString().
1060+
// of text, and text rendering is currently delayed via flushLineString(). This is important for things like
1061+
// emojis where the color of the text is different from the underline's color.
9801062
if (cell.textFlags & DRAW_STRIKE) {
9811063
hasStrikeThrough = YES;
9821064
}

0 commit comments

Comments
 (0)