Skip to content

Commit 058eb2f

Browse files
committed
Code clean up / robustification and additional logging for render
Clean up code per clang analyzer. Make sure to initialize structs properly and clean up potential memory leaks. When drawing wide characters, clean up the other cell to be empty just to be clean. When updating fonts for a MacVim window, if it's not the main window, make sure to not update the shared font manager as it's previously a minor bug where it would do that. Add additional debug logging for renderer to help debug potential CoreText renderer issues. See #1164.
1 parent c9a5a7f commit 058eb2f

File tree

5 files changed

+70
-20
lines changed

5 files changed

+70
-20
lines changed

src/MacVim/MMAppController.m

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,9 +1775,9 @@ - (NSArray *)filterFilesAndNotify:(NSArray *)filenames
17751775
[alert setMessageText:NSLocalizedString(@"Multiple files not found",
17761776
@"File not found dialog, title")];
17771777
text = [NSString stringWithFormat:NSLocalizedString(
1778-
@"Could not open file with name %@, and %d other files.",
1778+
@"Could not open file with name %@, and %u other files.",
17791779
@"File not found dialog, text"),
1780-
firstMissingFile, count-[files count]-1];
1780+
firstMissingFile, (unsigned int)(count-[files count]-1)];
17811781
}
17821782

17831783
[alert setInformativeText:text];

src/MacVim/MMCoreTextView.m

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ - (void)invertBlockFromRow:(int)row column:(int)col numRows:(int)nrows
176176
// with `:set nofu`. If that gets fixed, then delete this workaround.
177177
static GridCell* grid_cell_safe(Grid *grid, int row, int col) {
178178
if (row >= grid->rows || col >= grid->cols) {
179-
static GridCell scratch_cell;
179+
static GridCell scratch_cell = {};
180180
return &scratch_cell;
181181
}
182182
return grid_cell(grid, row, col);
@@ -377,6 +377,7 @@ - (NSRect)rectForColumnsInRange:(NSRange)range
377377
- (void)setFont:(NSFont *)newFont
378378
{
379379
if (!newFont) {
380+
ASLogInfo(@"Trying to set null font");
380381
return;
381382
}
382383
if (!forceRefreshFont) {
@@ -414,7 +415,9 @@ - (void)setFont:(NSFont *)newFont
414415
CTFontDescriptorRef desc = CTFontDescriptorCreateWithNameAndSize((CFStringRef)[newFont fontName], pt);
415416
CTFontRef fontRef = CTFontCreateWithFontDescriptor(desc, pt, NULL);
416417
CFRelease(desc);
417-
418+
if (!fontRef) {
419+
ASLogInfo(@"CTFontCreateWithFontDescriptor failed (preserveLineHeight == false, fontName: %@), pt: %f", [newFont fontName], pt);
420+
}
418421
font = (NSFont*)fontRef;
419422
} else {
420423
font = [newFont retain];
@@ -439,7 +442,10 @@ - (void)setWideFont:(NSFont *)newFont
439442
if (!newFont) {
440443
// Use the normal font as the wide font (note that the normal font may
441444
// very well include wide characters.)
442-
if (font) [self setWideFont:font];
445+
if (font) {
446+
[self setWideFont:font];
447+
return;
448+
}
443449
} else if (newFont != fontWide) {
444450
[fontWide release];
445451
fontWide = [newFont retain];
@@ -730,14 +736,22 @@ - (void)drawRect:(NSRect)rect
730736
NSGraphicsContext *context = [NSGraphicsContext currentContext];
731737
CGContextRef ctx = context.CGContext;
732738
[context setShouldAntialias:antialias];
733-
CGContextSetFillColorSpace(ctx, CGColorSpaceCreateWithName(kCGColorSpaceSRGB));
739+
{
740+
CGColorSpaceRef colorSpace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
741+
if (colorSpace) {
742+
CGContextSetFillColorSpace(ctx, colorSpace);
743+
CGColorSpaceRelease(colorSpace);
744+
} else {
745+
ASLogInfo(@"Could not create sRGB color space");
746+
}
747+
}
734748
CGContextSetTextMatrix(ctx, CGAffineTransformIdentity);
735749
CGContextSetTextDrawingMode(ctx, kCGTextFill);
736750
CGContextSetFontSize(ctx, [font pointSize]);
737751
CGContextSetShouldSmoothFonts(ctx, YES);
738752
CGContextSetBlendMode(ctx, kCGBlendModeCopy);
739753

740-
int originalSmoothingStyle;
754+
int originalSmoothingStyle = 0;
741755
if (thinStrokes) {
742756
originalSmoothingStyle = CGContextGetFontSmoothingStyle(ctx);
743757
CGContextSetFontSmoothingStyle(ctx, fontSmoothingStyleLight);
@@ -757,8 +771,8 @@ - (void)drawRect:(NSRect)rect
757771

758772
__block NSMutableString *lineString = nil;
759773
__block CGFloat lineStringStart = 0;
760-
__block CFRange lineStringRange;
761-
__block GridCell lastStringCell;
774+
__block CFRange lineStringRange = {};
775+
__block GridCell lastStringCell = {};
762776
void (^flushLineString)() = ^{
763777
if (!lineString.length)
764778
return;
@@ -774,20 +788,35 @@ - (void)drawRect:(NSRect)rect
774788
CGContextSetFillColor(ctx, COMPONENTS(lastStringCell.fg));
775789
CGContextSetTextPosition(ctx, lineStringStart, rowRect.origin.y + fontDescent);
776790
CGContextSetBlendMode(ctx, kCGBlendModeNormal);
791+
792+
const NSUInteger lineStringLength = lineString.length;
777793
CTLineRef line = [self lineForCharacterString:lineString textFlags:lastStringCell.textFlags];
778-
for (id obj in (NSArray*)CTLineGetGlyphRuns(line)) {
794+
NSArray* glyphRuns = (NSArray*)CTLineGetGlyphRuns(line);
795+
if ([glyphRuns count] == 0) {
796+
ASLogDebug(@"CTLineGetGlyphRuns no glyphs for: %@", lineString);
797+
}
798+
for (id obj in glyphRuns) {
779799
CTRunRef run = (CTRunRef)obj;
780800
CFIndex glyphCount = CTRunGetGlyphCount(run);
781801
CFIndex indices[glyphCount];
782802
CGPoint positions[glyphCount];
783803
CGGlyph glyphs[glyphCount];
784804
CTRunGetStringIndices(run, CFRangeMake(0, 0), indices);
785805
CTRunGetGlyphs(run, CFRangeMake(0, 0), glyphs);
786-
for (CFIndex i = 0; i < glyphCount; i++)
806+
for (CFIndex i = 0; i < glyphCount; i++) {
807+
if (indices[i] >= lineStringLength) {
808+
ASLogDebug(@"Invalid glyph pos index: %ld, len: %lu", (long)indices[i], (unsigned long)lineStringLength);
809+
continue;
810+
}
787811
positions[i] = positionsByIndex[indices[i]];
812+
}
788813
CTFontRef font = CFDictionaryGetValue(CTRunGetAttributes(run), kCTFontAttributeName);
814+
if (!font) {
815+
ASLogDebug(@"Null font for rendering. glyphCount: %ld", (long)glyphCount);
816+
}
789817
CTFontDrawGlyphs(font, glyphs, positions, glyphCount, ctx);
790818
}
819+
791820
CGContextSetBlendMode(ctx, kCGBlendModeCopy);
792821
[lineString deleteCharactersInRange:NSMakeRange(0, lineString.length)];
793822
};
@@ -867,8 +896,9 @@ - (void)drawRect:(NSRect)rect
867896
[lineString release];
868897
CGContextRestoreGState(ctx);
869898
}
870-
if (thinStrokes)
899+
if (thinStrokes) {
871900
CGContextSetFontSmoothingStyle(ctx, originalSmoothingStyle);
901+
}
872902
}
873903

874904
- (void)performBatchDrawWithData:(NSData *)data
@@ -1094,6 +1124,7 @@ - (NSFont *)fontVariantForTextFlags:(int)textFlags {
10941124
fontRef = [NSFontManager.sharedFontManager convertFont:fontRef toHaveTrait:NSFontItalicTrait];
10951125
if (textFlags & DRAW_BOLD)
10961126
fontRef = [NSFontManager.sharedFontManager convertFont:fontRef toHaveTrait:NSFontBoldTrait];
1127+
10971128
fontVariants[cacheFlags] = fontRef;
10981129
}
10991130
return fontRef;
@@ -1104,8 +1135,9 @@ - (CTLineRef)lineForCharacterString:(NSString *)string
11041135
int cacheFlags = flags & (DRAW_WIDE | DRAW_ITALIC | DRAW_BOLD);
11051136
NSNumber *key = @(cacheFlags);
11061137
NSCache<NSString *,id> *strCache = characterLines[key];
1107-
if (!strCache)
1138+
if (!strCache){
11081139
strCache = characterLines[key] = [[[NSCache alloc] init] autorelease];
1140+
}
11091141
CTLineRef line = (CTLineRef)[strCache objectForKey:string];
11101142
if (!line) {
11111143
NSAttributedString *attrString = [[NSAttributedString alloc]
@@ -1476,12 +1508,13 @@ - (void)setString:(NSString *)string
14761508
withFlags:(int)flags foregroundColor:(int)fg
14771509
backgroundColor:(int)bg specialColor:(int)sp
14781510
{
1479-
BOOL wide = flags & DRAW_WIDE ? YES : NO;
1511+
const BOOL wide = flags & DRAW_WIDE ? YES : NO;
14801512
__block size_t cells_filled = 0;
14811513
[string enumerateSubstringsInRange:NSMakeRange(0, string.length)
14821514
options:NSStringEnumerationByComposedCharacterSequences
14831515
usingBlock:^(NSString *substring, NSRange substringRange, NSRange enclosingRange, BOOL *stop) {
1484-
GridCell *cell = grid_cell_safe(&grid, row, col + cells_filled++ * (wide ? 2 : 1));
1516+
const int curCol = col + cells_filled++ * (wide ? 2 : 1);
1517+
GridCell *cell = grid_cell_safe(&grid, row, curCol);
14851518
GridCellInsertionPoint insertionPoint = {0};
14861519
if (flags & DRAW_TRANSP)
14871520
insertionPoint = cell->insertionPoint;
@@ -1502,6 +1535,13 @@ - (void)setString:(NSString *)string
15021535
.insertionPoint = insertionPoint,
15031536
.string = characterString,
15041537
};
1538+
1539+
if (wide) {
1540+
// Also clear the next cell just to be tidy (even though this cell would be skipped during rendering)
1541+
const GridCell clearCell = { .bg = defaultBackgroundColor.argbInt };
1542+
GridCell *nextCell = grid_cell_safe(&grid, row, curCol + 1);
1543+
*nextCell = clearCell;
1544+
}
15051545
}];
15061546
[self setNeedsDisplayFromRow:row
15071547
column:col

src/MacVim/MMVimController.m

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ @interface MMTouchBarInfo : NSObject;
8888
@property (readonly) NSMutableDictionary *itemDict;
8989
@property (readonly) NSMutableArray *itemOrder;
9090

91-
- (id)initWithVimController:(MMVimController *)controller;
92-
9391
@end
9492

9593
@interface MMTouchBarItemInfo : NSObject;
@@ -828,6 +826,7 @@ - (void)handleMessage:(int)msgid data:(NSData *)data
828826
// This should only happen if the system default font has changed
829827
// name since MacVim was compiled in which case we fall back on
830828
// using the user fixed width font.
829+
ASLogInfo(@"Failed to load font '%@' / %f", name, size);
831830
font = [NSFont userFixedPitchFontOfSize:size];
832831
}
833832

@@ -1634,7 +1633,7 @@ - (void)addTouchbarItemWithLabel:(NSString *)label
16341633
touchbarItem = item;
16351634
}
16361635

1637-
MMTouchBarItemInfo *touchbarItemInfo = [[MMTouchBarItemInfo alloc] initWithItem:touchbarItem label:touchbarLabel];
1636+
MMTouchBarItemInfo *touchbarItemInfo = [[[MMTouchBarItemInfo alloc] initWithItem:touchbarItem label:touchbarLabel] autorelease];
16381637
if (submenu) {
16391638
[touchbarItemInfo makeChildTouchBar];
16401639
}
@@ -2009,6 +2008,10 @@ @implementation MMTouchBarInfo
20092008

20102009
- (id)init
20112010
{
2011+
if (!(self = [super init])) {
2012+
return nil;
2013+
}
2014+
20122015
_touchbar = [[NSTouchBar alloc] init];
20132016

20142017
_itemDict = [[NSMutableDictionary alloc] init];

src/MacVim/MMWindowController.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
- (void)setScrollbarThumbValue:(float)val proportion:(float)prop
7676
identifier:(int32_t)ident;
7777

78-
- (unsigned int)calculateStyleMask;
7978
- (void)setBackgroundOption:(int)dark;
8079
- (void)refreshApperanceMode;
8180

src/MacVim/MMWindowController.m

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,13 @@ - (void)setDefaultColorsBackground:(NSColor *)back foreground:(NSColor *)fore
692692

693693
- (void)setFont:(NSFont *)font
694694
{
695-
[[NSFontManager sharedFontManager] setSelectedFont:font isMultiple:NO];
695+
const NSWindow* mainWindow = [NSApp mainWindow];
696+
if (mainWindow && (mainWindow == decoratedWindow || mainWindow == fullScreenWindow)) {
697+
// Update the shared font manager with the new font, but only if this is the main window,
698+
// as the font manager is shared among all the windows.
699+
[[NSFontManager sharedFontManager] setSelectedFont:font isMultiple:NO];
700+
}
701+
696702
[[vimView textView] setFont:font];
697703
[self updateResizeConstraints];
698704
shouldMaximizeWindow = YES;
@@ -1159,6 +1165,7 @@ - (void)windowDidBecomeMain:(NSNotification *)notification
11591165
[[MMAppController sharedInstance] setMainMenu:[vimController mainMenu]];
11601166

11611167
if ([vimView textView]) {
1168+
// Update the shared font manager to always be set to the font of the main window.
11621169
NSFontManager *fm = [NSFontManager sharedFontManager];
11631170
[fm setSelectedFont:[[vimView textView] font] isMultiple:NO];
11641171
}
@@ -1247,6 +1254,7 @@ - (void)windowDidResize:(id)sender
12471254

12481255
- (void)windowDidChangeBackingProperties:(NSNotification *)notification
12491256
{
1257+
ASLogDebug(@"");
12501258
[vimController sendMessage:BackingPropertiesChangedMsgID data:nil];
12511259
}
12521260

0 commit comments

Comments
 (0)