Thin strokes strokes shouldn't always be enabled

While investigating the differences I observed in the issue https://gitlab.com/gnachman/iterm2/issues/4956 I noticed that in OS X applications the smoothing style to obtain thin strokes isn't applied to all text. The logic is still really basic:

1. Obtain brightnessComponent of both the text foreground and background color
2. Check if the brightness of the foreground is higher than the background one
3. If so apply the smoothing style 16, else don't

From some quick tests the difference is noticeable, especially when the text is drawn on bright colours and it looks better imho. iTerm doesn't seem to set the background color attribute so I hacked together support for it just for testing, but I'm still not comfortable with your codebase so this is probably the wrong approach on how to code it. Anyway here's a diff that cleanly applies to last master if you want to test the difference yourself.

diff --git a/sources/PTYTextView.m b/sources/PTYTextView.m
index 701b9d1..958dbb4 100644
--- a/sources/PTYTextView.m
+++ b/sources/PTYTextView.m
@@ -1064,8 +1064,8 @@ static const int kDragThreshold = 3;
 }
 
 - (CGFloat)minimumBaselineOffset {
-    CGFloat asciiOffset = -(floorf(self.font.leading) - floorf(self.font.descender));
-    CGFloat nonasciiOffset = self.useNonAsciiFont ? -(floorf(self.nonAsciiFont.leading) - floorf(self.nonAsciiFont.descender)) : 0;
+    CGFloat asciiOffset = -(floorf(self.font.leading) - floorf(self.font.descender + 0.5));
+    CGFloat nonasciiOffset = self.useNonAsciiFont ? -(floorf(self.nonAsciiFont.leading) - floorf(self.nonAsciiFont.descender + 0.5)) : 0;
     return MIN(asciiOffset, nonasciiOffset);
 }
 
diff --git a/sources/iTermTextDrawingHelper.m b/sources/iTermTextDrawingHelper.m
index ef1d84b..456e193 100644
--- a/sources/iTermTextDrawingHelper.m
+++ b/sources/iTermTextDrawingHelper.m
@@ -50,6 +50,7 @@ typedef struct {
     BOOL initialized;
     BOOL shouldAntiAlias;
     NSColor *foregroundColor;
+    NSColor *backgroundColor;
     BOOL boxDrawing;
     NSFont *font;
     BOOL bold;
@@ -915,15 +916,6 @@ typedef struct iTermTextColorContext {
                                origin:(VT100GridCoord)initialOrigin
                             positions:(NSArray<NSNumber *> *)positions
                             inContext:(CGContextRef)ctx {
-    int savedFontSmoothingStyle = 0;
-    BOOL useThinStrokes = [self useThinStrokes];
-    if (useThinStrokes) {
-        // This seems to be available at least on 10.8 and later. The only reference to it is in
-        // WebKit. This causes text to render just a little lighter, which looks nicer.
-        savedFontSmoothingStyle = CGContextGetFontSmoothingStyle(ctx);
-        CGContextSetFontSmoothingStyle(ctx, 16);
-    }
-    
     NSPoint point = initialPoint;
     VT100GridCoord origin = initialOrigin;
     NSInteger start = 0;
@@ -946,9 +938,6 @@ typedef struct iTermTextColorContext {
 
     }
 
-    if (useThinStrokes) {
-        CGContextSetFontSmoothingStyle(ctx, savedFontSmoothingStyle);
-    }
 }
 
 - (void)drawBoxDrawingCharacter:(unichar)theCharacter withAttributes:(NSDictionary *)attributes at:(NSPoint)pos {
@@ -1012,6 +1001,9 @@ typedef struct iTermTextColorContext {
     DLog(@"Draw attributed string beginning at %d", (int)round(origin.x));
     NSDictionary *attributes = [attributedString attributesAtIndex:0 effectiveRange:nil];
     NSColor *color = attributes[NSForegroundColorAttributeName];
+    NSColor *color2 = attributes[NSBackgroundColorAttributeName];
+    CGFloat br1 = [color brightnessComponent];
+    CGFloat br2 = [color2 brightnessComponent];
 
     BOOL bold = [attributes[iTermBoldAttribute] boolValue];
     BOOL fakeBold = [attributes[iTermFakeBoldAttribute] boolValue];
@@ -1033,6 +1025,15 @@ typedef struct iTermTextColorContext {
     CGContextSetShouldAntialias(cgContext, antiAlias);
     CGContextSetFillColorWithColor(cgContext, [self cgColorForColor:color]);
     CGContextSetStrokeColorWithColor(cgContext, [self cgColorForColor:color]);
+    int savedFontSmoothingStyle = 0;
+    bool changedStyle = false;
+    if (br1 > br2) {
+        // This seems to be available at least on 10.8 and later. The only reference to it is in
+        // WebKit. This causes text to render just a little lighter, which looks nicer.
+        savedFontSmoothingStyle = CGContextGetFontSmoothingStyle(cgContext);
+        CGContextSetFontSmoothingStyle(cgContext, 16);
+        changedStyle = true;
+    }
 
     CGFloat c = 0.0;
     if (fakeItalic) {
@@ -1095,6 +1096,9 @@ typedef struct iTermTextColorContext {
             CTFontDrawGlyphs(runFont, buffer, (NSPoint *)positions, length, cgContext);
             CGContextTranslateCTM(cgContext, antiAlias ? -_antiAliasedShift : -1, 0);
         }
+        if (changedStyle) {
+            CGContextSetFontSmoothingStyle(cgContext, savedFontSmoothingStyle);
+        }
     }
     CFRelease(lineRef);
     [ctx restoreGraphicsState];
@@ -1262,6 +1266,7 @@ static BOOL iTermTextDrawingHelperIsCharacterDrawable(screen_char_t *c,
             // Properly compare object fields
             *combinedAttributesChanged = (newAttributes->shouldAntiAlias != previousAttributes->shouldAntiAlias ||
                                           ![newAttributes->foregroundColor isEqual:previousAttributes->foregroundColor] ||
+                                           ![newAttributes->backgroundColor isEqual:previousAttributes->backgroundColor] ||
                                           newAttributes->boxDrawing != previousAttributes->boxDrawing ||
                                           ![newAttributes->font isEqual:previousAttributes->font] ||
                                           newAttributes->bold != previousAttributes->bold ||
@@ -1315,6 +1320,7 @@ static BOOL iTermTextDrawingHelperIsCharacterDrawable(screen_char_t *c,
                                                                          i,
                                                                          textColorContext);
     }
+    attributes->backgroundColor = textColorContext->backgroundColor;
 
     const BOOL complex = c->complexChar;
     const unichar code = c->code;
@@ -1338,6 +1344,7 @@ static BOOL iTermTextDrawingHelperIsCharacterDrawable(screen_char_t *c,
     return @{ (NSString *)kCTLigatureAttributeName: @0,
               NSForegroundColorAttributeName: attributes->foregroundColor,
               NSFontAttributeName: attributes->font,
+              NSBackgroundColorAttributeName: attributes->backgroundColor,
               iTermAntiAliasAttribute: @(attributes->shouldAntiAlias),
               iTermIsBoxDrawingAttribute: @(attributes->boxDrawing),
               iTermFakeBoldAttribute: @(attributes->fakeBold),

I think I saw logic like this in Terminal.app. Where else did you see it?

Milestone changed to %Feature Complete 3.1

Fixed in commit 84f0fb6d

Status changed to closed