Skip to content

Commit

Permalink
Fix Catalyst text rendering by disabling inappropriate subpixel-antia…
Browse files Browse the repository at this point in the history
…liasing (#29609)

Summary:
I noticed when porting my iOS app to macOS via Catalyst that the text rendering was somewhat different on the two platforms. Text looked blurry and over-weight on macOS, even when disabling the Catalyst scaling transform.

I hazily remembered that I'd seen this problem before in my old Cocoa development days: this kind of blurring occurs when rendering text with sub-pixel anti-aliasing into an offscreen buffer which will then be traditionally composited, because when the SPAA algorithm attempts to blend with the underlying content (i.e. in the offscreen buffer), there isn't any. SPAA is disabled on iOS, so the issue wouldn't appear there. On macOS, typical approachs to displaying text (e.g. `CATextLayer`) normally disable SPAA, since it's been incompatible with the platform's compositing strategy since the transition to layer-backed views some years ago. But React Native uses `NSLayoutManager` to rasterize text (rather than relying on the system render server via `CATextLayer`), and that class doesn't touch the context's font smoothing bit before drawing.

This change makes macOS/Catalyst text rendering consistent with iOS text rendering by disabling SPAA.

It appears that the code I've modified is in the process of being refactored (for Fabric?). It looks like [this](https://github.com/facebook/react-native/blob/8d6b41e9bcede07fb627d57cf6c11050ae590d57/ReactCommon/react/renderer/textlayoutmanager/platform/ios/RCTTextLayoutManager.mm#L111) is the corresponding place in the new code (sammy-SC, is that right?). I'm happy to include a change to the new renderer in this patch if someone can point me at how to test that change.

## Changelog

[iOS] [Fixed] - Improved text rendering on macOS Catalyst

Pull Request resolved: #29609

Test Plan:
1. Prepare RNTester for running on macOS (or apply [this patch](https://gist.github.com/andymatuschak/d0f5b4fc1a28efc4f860801aa1deddcd) to handle parts 1 and 2, but you'll still need to do part 3):
   1. Open the workspace, navigate to the `RNTester` target's configuration, and check the "Mac" checkbox under "Deployment Info.
   2. Flipper doesn't yet compile for Catalyst (#27845), so you must disable it by: a) commenting out `use_flipper!` and `flipper_post_install` in the Podfile, then running `pod install`; and b) removing the `FB_SONARKIT_ENABLED` preprocessor flags in the Xcode project.
   3. macOS has different signing rules from iOS; you must set a development team in the "Signing & Capabilities" tab of the `RNTester` target configuration pane. Unfortunately, you must also do this in the `Pods` project for the `React-Core-AccessibilityResources` target ([this is an issue which CocoaPods must fix](CocoaPods/CocoaPods#8891)).
2. Run RNTester with and without the patch. You'll see that the font hinting is overweight without the patch; see screenshots below (incorrect rendering above, correct rendering below; note that fonts still remain slightly blurred because of Catalyst's window scaling transform, but that's removed on Big Sur).

![Screen Shot 2020-08-12 at 10 03 50 AM](https://user-images.githubusercontent.com/2771/90045523-0374c700-dc84-11ea-8945-2d9830bccbd1.png)
![Screen Shot 2020-08-12 at 10 03 15 AM](https://user-images.githubusercontent.com/2771/90045547-0bcd0200-dc84-11ea-88af-37a8879b4efd.png)

Reviewed By: PeteTheHeat

Differential Revision: D23344751

Pulled By: sammy-SC

fbshipit-source-id: 1bbf682b681e381a8a90e152245d9b0df8ec7697
  • Loading branch information
andymatuschak authored and facebook-github-bot committed Aug 27, 2020
1 parent 5ffabca commit 694e22d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
13 changes: 13 additions & 0 deletions Libraries/Text/Text/RCTTextView.m
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ - (void)drawRect:(CGRect)rect
NSLayoutManager *layoutManager = _textStorage.layoutManagers.firstObject;
NSTextContainer *textContainer = layoutManager.textContainers.firstObject;

#if TARGET_OS_MACCATALYST
CGContextRef context = UIGraphicsGetCurrentContext();
CGContextSaveGState(context);
// NSLayoutManager tries to draw text with sub-pixel anti-aliasing by default on
// macOS, but rendering SPAA onto a transparent background produces poor results.
// CATextLayer disables font smoothing by default now on macOS; we follow suit.
CGContextSetShouldSmoothFonts(context, NO);
#endif

NSRange glyphRange = [layoutManager glyphRangeForTextContainer:textContainer];
[layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:_contentFrame.origin];
[layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:_contentFrame.origin];
Expand Down Expand Up @@ -148,6 +157,10 @@ - (void)drawRect:(CGRect)rect
[_highlightLayer removeFromSuperlayer];
_highlightLayer = nil;
}

#if TARGET_OS_MACCATALYST
CGContextRestoreGState(context);
#endif
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,19 @@ - (void)drawAttributedString:(AttributedString)attributedString
NSLayoutManager *layoutManager = textStorage.layoutManagers.firstObject;
NSTextContainer *textContainer = layoutManager.textContainers.firstObject;

#if TARGET_OS_MACCATALYST
CGContextRef context = UIGraphicsGetCurrentContext();
CGContextSaveGState(context);
CGContextSetShouldSmoothFonts(context, NO);
#endif

NSRange glyphRange = [layoutManager glyphRangeForTextContainer:textContainer];
[layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:frame.origin];
[layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:frame.origin];

#if TARGET_OS_MACCATALYST
CGContextRestoreGState(context);
#endif
}

- (LinesMeasurements)getLinesForAttributedString:(facebook::react::AttributedString)attributedString
Expand Down

0 comments on commit 694e22d

Please sign in to comment.