Merge pull request #1173 from ychin/code-clean-up-additional-logging

Code clean up / robustification and additional logging for render

Author: ychin

src/MacVim/MMAppController.m

@@ -1775,9 +1775,9 @@ - (NSArray *)filterFilesAndNotify:(NSArray *)filenames
             [alert setMessageText:NSLocalizedString(@"Multiple files not found",
                     @"File not found dialog, title")];
             text = [NSString stringWithFormat:NSLocalizedString(
-                    @"Could not open file with name %@, and %d other files.",
+                    @"Could not open file with name %@, and %u other files.",
                     @"File not found dialog, text"),
-                firstMissingFile, count-[files count]-1];
+                firstMissingFile, (unsigned int)(count-[files count]-1)];
         }
 
         [alert setInformativeText:text];

src/MacVim/MMCoreTextView.m

@@ -176,7 +176,7 @@ - (void)invertBlockFromRow:(int)row column:(int)col numRows:(int)nrows
 // with `:set nofu`. If that gets fixed, then delete this workaround.
 static GridCell* grid_cell_safe(Grid *grid, int row, int col) {
     if (row >= grid->rows || col >= grid->cols) {
-        static GridCell scratch_cell;
+        static GridCell scratch_cell = {};
         return &scratch_cell;
     }
     return grid_cell(grid, row, col);
@@ -377,6 +377,7 @@ - (NSRect)rectForColumnsInRange:(NSRange)range
 - (void)setFont:(NSFont *)newFont
 {
     if (!newFont) {
+        ASLogInfo(@"Trying to set null font");
         return;
     }
     if (!forceRefreshFont) {
@@ -414,7 +415,9 @@ - (void)setFont:(NSFont *)newFont
         CTFontDescriptorRef desc = CTFontDescriptorCreateWithNameAndSize((CFStringRef)[newFont fontName], pt);
         CTFontRef fontRef = CTFontCreateWithFontDescriptor(desc, pt, NULL);
         CFRelease(desc);
-
+        if (!fontRef) {
+            ASLogInfo(@"CTFontCreateWithFontDescriptor failed (preserveLineHeight == false, fontName: %@), pt: %f", [newFont fontName], pt);
+        }
         font = (NSFont*)fontRef;
     } else {
         font = [newFont retain];
@@ -439,7 +442,10 @@ - (void)setWideFont:(NSFont *)newFont
     if (!newFont) {
         // Use the normal font as the wide font (note that the normal font may
         // very well include wide characters.)
-        if (font) [self setWideFont:font];
+        if (font) {
+            [self setWideFont:font];
+            return;
+        }
     } else if (newFont != fontWide) {
         [fontWide release];
         fontWide = [newFont retain];
@@ -730,14 +736,22 @@ - (void)drawRect:(NSRect)rect
     NSGraphicsContext *context = [NSGraphicsContext currentContext];
     CGContextRef ctx = context.CGContext;
     [context setShouldAntialias:antialias];
-    CGContextSetFillColorSpace(ctx, CGColorSpaceCreateWithName(kCGColorSpaceSRGB));
+    {
+        CGColorSpaceRef colorSpace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
+        if (colorSpace) {
+            CGContextSetFillColorSpace(ctx, colorSpace);
+            CGColorSpaceRelease(colorSpace);
+        } else {
+            ASLogInfo(@"Could not create sRGB color space");
+        }
+    }
     CGContextSetTextMatrix(ctx, CGAffineTransformIdentity);
     CGContextSetTextDrawingMode(ctx, kCGTextFill);
     CGContextSetFontSize(ctx, [font pointSize]);
     CGContextSetShouldSmoothFonts(ctx, YES);
     CGContextSetBlendMode(ctx, kCGBlendModeCopy);
 
-    int originalSmoothingStyle;
+    int originalSmoothingStyle = 0;
     if (thinStrokes) {
         originalSmoothingStyle = CGContextGetFontSmoothingStyle(ctx);
         CGContextSetFontSmoothingStyle(ctx, fontSmoothingStyleLight);
@@ -757,8 +771,8 @@ - (void)drawRect:(NSRect)rect
 
         __block NSMutableString *lineString = nil;
         __block CGFloat lineStringStart = 0;
-        __block CFRange lineStringRange;
-        __block GridCell lastStringCell;
+        __block CFRange lineStringRange = {};
+        __block GridCell lastStringCell = {};
         void (^flushLineString)() = ^{
             if (!lineString.length)
                 return;
@@ -774,20 +788,35 @@ - (void)drawRect:(NSRect)rect
             CGContextSetFillColor(ctx, COMPONENTS(lastStringCell.fg));
             CGContextSetTextPosition(ctx, lineStringStart, rowRect.origin.y + fontDescent);
             CGContextSetBlendMode(ctx, kCGBlendModeNormal);
+
+            const NSUInteger lineStringLength = lineString.length;
             CTLineRef line = [self lineForCharacterString:lineString textFlags:lastStringCell.textFlags];
-            for (id obj in (NSArray*)CTLineGetGlyphRuns(line)) {
+            NSArray* glyphRuns = (NSArray*)CTLineGetGlyphRuns(line);
+            if ([glyphRuns count] == 0) {
+                ASLogDebug(@"CTLineGetGlyphRuns no glyphs for: %@", lineString);
+            }
+            for (id obj in glyphRuns) {
                 CTRunRef run = (CTRunRef)obj;
                 CFIndex glyphCount = CTRunGetGlyphCount(run);
                 CFIndex indices[glyphCount];
                 CGPoint positions[glyphCount];
                 CGGlyph glyphs[glyphCount];
                 CTRunGetStringIndices(run, CFRangeMake(0, 0), indices);
                 CTRunGetGlyphs(run, CFRangeMake(0, 0), glyphs);
-                for (CFIndex i = 0; i < glyphCount; i++)
+                for (CFIndex i = 0; i < glyphCount; i++) {
+                    if (indices[i] >= lineStringLength) {
+                        ASLogDebug(@"Invalid glyph pos index: %ld, len: %lu", (long)indices[i], (unsigned long)lineStringLength);
+                        continue;
+                    }
                     positions[i] = positionsByIndex[indices[i]];
+                }
                 CTFontRef font = CFDictionaryGetValue(CTRunGetAttributes(run), kCTFontAttributeName);
+                if (!font) {
+                    ASLogDebug(@"Null font for rendering. glyphCount: %ld", (long)glyphCount);
+                }
                 CTFontDrawGlyphs(font, glyphs, positions, glyphCount, ctx);
             }
+
             CGContextSetBlendMode(ctx, kCGBlendModeCopy);
             [lineString deleteCharactersInRange:NSMakeRange(0, lineString.length)];
         };
@@ -867,8 +896,9 @@ - (void)drawRect:(NSRect)rect
         [lineString release];
         CGContextRestoreGState(ctx);
     }
-    if (thinStrokes)
+    if (thinStrokes) {
         CGContextSetFontSmoothingStyle(ctx, originalSmoothingStyle);
+    }
 }
 
 - (void)performBatchDrawWithData:(NSData *)data
@@ -1094,6 +1124,7 @@ - (NSFont *)fontVariantForTextFlags:(int)textFlags {
             fontRef = [NSFontManager.sharedFontManager convertFont:fontRef toHaveTrait:NSFontItalicTrait];
         if (textFlags & DRAW_BOLD)
             fontRef = [NSFontManager.sharedFontManager convertFont:fontRef toHaveTrait:NSFontBoldTrait];
+
         fontVariants[cacheFlags] = fontRef;
     }
     return fontRef;
@@ -1104,8 +1135,9 @@ - (CTLineRef)lineForCharacterString:(NSString *)string
     int cacheFlags = flags & (DRAW_WIDE | DRAW_ITALIC | DRAW_BOLD);
     NSNumber *key = @(cacheFlags);
     NSCache<NSString *,id> *strCache = characterLines[key];
-    if (!strCache)
+    if (!strCache){
         strCache = characterLines[key] = [[[NSCache alloc] init] autorelease];
+    }
     CTLineRef line = (CTLineRef)[strCache objectForKey:string];
     if (!line) {
         NSAttributedString *attrString = [[NSAttributedString alloc]
@@ -1476,12 +1508,13 @@ - (void)setString:(NSString *)string
         withFlags:(int)flags foregroundColor:(int)fg
   backgroundColor:(int)bg specialColor:(int)sp
 {
-    BOOL wide = flags & DRAW_WIDE ? YES : NO;
+    const BOOL wide = flags & DRAW_WIDE ? YES : NO;
     __block size_t cells_filled = 0;
     [string enumerateSubstringsInRange:NSMakeRange(0, string.length)
                                options:NSStringEnumerationByComposedCharacterSequences
                             usingBlock:^(NSString *substring, NSRange substringRange, NSRange enclosingRange, BOOL *stop) {
-        GridCell *cell = grid_cell_safe(&grid, row, col + cells_filled++ * (wide ? 2 : 1));
+        const int curCol = col + cells_filled++ * (wide ? 2 : 1);
+        GridCell *cell = grid_cell_safe(&grid, row, curCol);
         GridCellInsertionPoint insertionPoint = {0};
         if (flags & DRAW_TRANSP)
             insertionPoint = cell->insertionPoint;
@@ -1502,6 +1535,13 @@ - (void)setString:(NSString *)string
             .insertionPoint = insertionPoint,
             .string = characterString,
         };
+
+        if (wide) {
+            // Also clear the next cell just to be tidy (even though this cell would be skipped during rendering)
+            const GridCell clearCell = { .bg = defaultBackgroundColor.argbInt };
+            GridCell *nextCell = grid_cell_safe(&grid, row, curCol + 1);
+            *nextCell = clearCell;
+        }
     }];
     [self setNeedsDisplayFromRow:row
                           column:col

src/MacVim/MMVimController.m

@@ -88,8 +88,6 @@ @interface MMTouchBarInfo : NSObject;
 @property (readonly) NSMutableDictionary *itemDict;
 @property (readonly) NSMutableArray *itemOrder;
 
-- (id)initWithVimController:(MMVimController *)controller;
-
 @end
 
 @interface MMTouchBarItemInfo : NSObject;
@@ -828,6 +826,7 @@ - (void)handleMessage:(int)msgid data:(NSData *)data
             // This should only happen if the system default font has changed
             // name since MacVim was compiled in which case we fall back on
             // using the user fixed width font.
+            ASLogInfo(@"Failed to load font '%@' / %f", name, size);
             font = [NSFont userFixedPitchFontOfSize:size];
         }
 
@@ -1634,7 +1633,7 @@ - (void)addTouchbarItemWithLabel:(NSString *)label
         touchbarItem = item;
     }
 
-    MMTouchBarItemInfo *touchbarItemInfo = [[MMTouchBarItemInfo alloc] initWithItem:touchbarItem label:touchbarLabel];
+    MMTouchBarItemInfo *touchbarItemInfo = [[[MMTouchBarItemInfo alloc] initWithItem:touchbarItem label:touchbarLabel] autorelease];
     if (submenu) {
         [touchbarItemInfo makeChildTouchBar];
     }
@@ -2009,6 +2008,10 @@ @implementation MMTouchBarInfo
 
 - (id)init
 {
+    if (!(self = [super init])) {
+        return nil;
+    }
+
     _touchbar = [[NSTouchBar alloc] init];
 
     _itemDict = [[NSMutableDictionary alloc] init];

src/MacVim/MMWindowController.h

@@ -75,7 +75,6 @@
 - (void)setScrollbarThumbValue:(float)val proportion:(float)prop
                     identifier:(int32_t)ident;
 
-- (unsigned int)calculateStyleMask;
 - (void)setBackgroundOption:(int)dark;
 - (void)refreshApperanceMode;
 

src/MacVim/MMWindowController.m

@@ -692,7 +692,13 @@ - (void)setDefaultColorsBackground:(NSColor *)back foreground:(NSColor *)fore
 
 - (void)setFont:(NSFont *)font
 {
-    [[NSFontManager sharedFontManager] setSelectedFont:font isMultiple:NO];
+    const NSWindow* mainWindow = [NSApp mainWindow];
+    if (mainWindow && (mainWindow == decoratedWindow || mainWindow == fullScreenWindow)) {
+        // Update the shared font manager with the new font, but only if this is the main window,
+        // as the font manager is shared among all the windows.
+        [[NSFontManager sharedFontManager] setSelectedFont:font isMultiple:NO];
+    }
+
     [[vimView textView] setFont:font];
     [self updateResizeConstraints];
     shouldMaximizeWindow = YES;
@@ -1159,6 +1165,7 @@ - (void)windowDidBecomeMain:(NSNotification *)notification
     [[MMAppController sharedInstance] setMainMenu:[vimController mainMenu]];
 
     if ([vimView textView]) {
+        // Update the shared font manager to always be set to the font of the main window.
         NSFontManager *fm = [NSFontManager sharedFontManager];
         [fm setSelectedFont:[[vimView textView] font] isMultiple:NO];
     }
@@ -1247,6 +1254,7 @@ - (void)windowDidResize:(id)sender
 
 - (void)windowDidChangeBackingProperties:(NSNotification *)notification
 {
+    ASLogDebug(@"");
     [vimController sendMessage:BackingPropertiesChangedMsgID data:nil];
 }