[Catalyst][Text] Fix Catalyst text rendering by disabling inappropriate subpixel-antialiasing

[andymatuschak]

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 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

Test Plan

1. Prepare RNTester for running on macOS (or apply this patch 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 ([0.62.0-rc.3] Catalyst macOS build issue typedef redefinition with different types #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).
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).

[andymatuschak]

Changed category to "iOS" from "Catalyst" per @pull-bot.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 79f305e

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,840,195	-1,908
android	hermes	armeabi-v7a	7,450,193	-5,687
android	hermes	x86	8,298,482	-2,112
android	hermes	x86_64	8,191,573	-2,047
android	jsc	arm64-v8a	9,999,729	-1,896
android	jsc	armeabi-v7a	9,601,845	-5,704
android	jsc	x86	9,886,114	-2,104
android	jsc	x86_64	10,465,398	-2,038

Base commit: b7d6b32

[sammy-SC]

It appears that the code I've modified is in the process of being refactored (for Fabric?). It looks like this is the corresponding place in the new code (@sammy-SC, is that right?).

Hello @andymatuschak,

the code you have refactored will stay in place until Fabric is completely rolled out. Fabric on iOS is a complete rewrite.

You have found the corresponding place in Fabric :).

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.

If you define RN_FABRIC_ENABLED, RNTester will start with the new renderer.

[andymatuschak]

Thanks for that, @sammy-SC! I've just spent a while trying to get Fabric up and running (with iOS in general, not even on Catalyst). I faced immediate configuration issues which suggested to me that master may have some out of date Fabric components: several configuration files contain what appear to be outdated paths, and a dependency on flow-node was introduced without being added to the package.

I was able to make some progress by making the changes:

diff --git a/ReactCommon/react/renderer/graphics/React-graphics.podspec b/ReactCommon/react/renderer/graphics/React-graphics.podspec
index ddd519790e..07d54f808f 100644
--- a/ReactCommon/react/renderer/graphics/React-graphics.podspec
+++ b/ReactCommon/react/renderer/graphics/React-graphics.podspec
@@ -5,7 +5,7 @@
 
 require "json"
 
-package = JSON.parse(File.read(File.join(__dir__, "..", "..", "..", "package.json")))
+package = JSON.parse(File.read(File.join(__dir__, "..", "..", "..", "..", "package.json")))
 version = package['version']
 
 source = { :git => 'https://github.com/facebook/react-native.git' }
diff --git a/package.json b/package.json
index b9d85cdd85..99f3510d0f 100644
--- a/package.json
+++ b/package.json
@@ -142,6 +142,7 @@
     "eslint-plugin-react-native": "3.8.1",
     "eslint-plugin-relay": "1.7.1",
     "flow-bin": "^0.131.0",
+    "flow-remove-types": "^2.131.0",
     "jest": "^26.0.1",
     "jest-junit": "^10.0.0",
     "jscodeshift": "^0.9.0",
diff --git a/scripts/react_native_pods.rb b/scripts/react_native_pods.rb
index 73bb9ad55a..534dd6d298 100644
--- a/scripts/react_native_pods.rb
+++ b/scripts/react_native_pods.rb
@@ -52,7 +52,7 @@ def use_react_native! (options={})
 
   if fabric_enabled
     pod 'React-Fabric', :path => "#{prefix}/ReactCommon"
-    pod 'React-graphics', :path => "#{prefix}/ReactCommon/fabric/graphics"
+    pod 'React-graphics', :path => "#{prefix}/ReactCommon/react/renderer/graphics"
     pod 'React-jsi/Fabric', :path => "#{prefix}/ReactCommon/jsi"
     pod 'React-RCTFabric', :path => "#{prefix}/React"
     pod 'Folly/Fabric', :podspec => "#{prefix}/third-party-podspecs/Folly.podspec"

But even after these patches, the React-graphics pod appears to have some misconfiguration issues. It fails to build with this error:

/Users/andym/Development/External/react-native/ReactCommon/react/renderer/graphics/platform/ios/Color.h:13:10: 'react/renderer/graphics/ColorComponents.h' file not found

… which led me to notice that the React-graphics Pod's Xcode project is missing most of its source files, including the RCTTextLayoutManager.mm file I've modified. This screenshot shows the source listing for React-graphics:

At this point, I stopped, because these problems don't appear to be local to my machine—it looks to me more like I'm simply not running known-good sources for Fabric.

I've updated the test plan in the pull request description to include the steps for running RNTester on macOS/Catalyst, and I've pushed a commit to my branch which implements my change for Fabric. If you'd be willing to test the change yourself or to point me to where I've gone wrong in my attempt above, I'd be grateful.

[sammy-SC]

Hello @andymatuschak,

Re Fabric:
Apparently it isn't ready to be run in open source yet. I will import this PR into our systems and take it for a spin in Fabric to make sure it works.

Thank you

[facebook-github-bot]

@sammy-SC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @andymatuschak in 694e22d.

^{When will my fix make it into a release? | Upcoming Releases}

[andymatuschak]

Thank you for testing and landing this!

[sammy-SC]

@andymatuschak thank you for working on this :)