Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.62.0-rc.3] Catalyst macOS build issue typedef redefinition with different types #27845

Closed
mgcrea opened this issue Jan 23, 2020 · 4 comments
Closed
Labels
Bug Resolution: Locked This issue was locked by the bot.

Comments

@mgcrea
Copy link

mgcrea commented Jan 23, 2020

React Native version:

System:
    OS: macOS 10.15.2
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
    Memory: 52.18 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 13.7.0 - /var/folders/2d/y3l4yzcj0pz9r62r554t2nqr0000gn/T/yarn--1579780584922-0.8762400095093377/node
    Yarn: 1.21.1 - /var/folders/2d/y3l4yzcj0pz9r62r554t2nqr0000gn/T/yarn--1579780584922-0.8762400095093377/yarn
    npm: 6.13.6 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.2, DriverKit 19.0, macOS 10.15, tvOS 13.2, watchOS 6.1
    Android SDK:
      API Levels: 23, 25, 26, 27, 28
      Build Tools: 23.0.1, 26.0.3, 27.0.3, 28.0.3
      System Images: android-28 | Intel x86 Atom, android-28 | Intel x86 Atom_64, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom
  IDEs:
    Android Studio: 3.5 AI-191.8026.42.35.5900203
    Xcode: 11.3.1/11C504 - /usr/bin/xcodebuild
  npmPackages:
    react: 16.11.0 => 16.11.0 
    react-native: 0.62.0-rc.1 => 0.62.0-rc.1

Steps To Reproduce

  1. Create a new project npx react-native init SandboxProject_62 --version 0.62.0-rc.1
  2. Enable macOS target in xcode
  3. Build

Describe what you expected to happen:

Successful build for macOS target

Errors

[...]/SandboxProject_62/ios/Pods/Headers/Private/Flipper-Folly/folly/portability/Time.h:51:17:
Typedef redefinition with different types ('uint8_t' (aka 'unsigned char') vs 'enum clockid_t')

If I comment out this line, Flipper-Folly build does seem to pass but I end up with an OpenSSL linking error:

ld: in [...]/SandboxProject_62/ios/Pods/OpenSSL-Universal/ios/lib/libcrypto.a(cryptlib.o), building for Mac Catalyst, but linking in object file built for iOS Simulator, file '[...]/SandboxProject_62/ios/Pods/OpenSSL-Universal/ios/lib/libcrypto.a' for architecture x86_64

Would be really awesome to have a working macOS target for the 0.62 release as everything seems to be pretty ready.

@mgcrea mgcrea added the Bug label Jan 23, 2020
@mgcrea

This comment has been minimized.

@mgcrea mgcrea changed the title [0.62.0-rc.1] Catalyst macOS build issue typedef redefinition with different types [0.62.0-rc.2] Catalyst macOS build issue typedef redefinition with different types Feb 13, 2020
@mgcrea mgcrea changed the title [0.62.0-rc.2] Catalyst macOS build issue typedef redefinition with different types [0.62.0-rc.3] Catalyst macOS build issue typedef redefinition with different types Feb 26, 2020
@mgcrea
Copy link
Author

mgcrea commented Feb 26, 2020

FYI this bug still exists in the latest 0.62.0-rc.3 release.
I've opened an issue in Flipper: facebook/flipper#834

@0xmtn
Copy link

0xmtn commented Apr 29, 2020

Hi @mgcrea,

Have you had any success since posting this? I'm having the same problem and got stuck at libcrypto.a error.

@johnsonjo4531
Copy link

@0xmtn @mgcrea I fixed this with updating my iOS Deployment Target Build setting in my app to be 9.0 and my platform :ios '10.0' to be platform :ios '9.0' in my ios/Podfile :

Change this:

ios/Podfile

platform :ios '10.0'

to this:

ios/Podfile

platform :ios, '9.0'

Don't forget to npx pod-install from the root of your project afterwards.

I guess this means flipper-folly doesn't like iOS version 10 as a deployment target...

facebook-github-bot pushed a commit that referenced this issue Aug 27, 2020
…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
philippeauriach pushed a commit to philippeauriach/react-native that referenced this issue May 5, 2021
…liasing (facebook#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: facebook#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 (facebook#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
@facebook facebook locked as resolved and limited conversation to collaborators Nov 3, 2021
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants