[iOS] increasing font scale (content size catgory) breaks text #45857

jpudysz · 2024-08-01T07:46:36Z

Description

Hello team,

Everything started from an issue reported in the Unistyles repo: issue. After some debugging, I was able to isolate the issue to React Native core. It’s not easily visible, as with the new architecture, the app is not hot reloading when the user changes the font scale (this was also reported here).

It has nothing to do with Unistyles, but it was spotted by @yzhe554 because Unistyles auto re-renders the view if any native change happens, like a font scale change.

I created a repro repo with no external dependencies.

Steps to reproduce

Create new app with new architecture
Run iOS simulator
Add <Text> that depends on fontSize
Change contentSizeCategory (fontScale)
Force re-render

React Native Version

0.75-rc.6

Affected Platforms

Runtime - iOS

Areas

Fabric - The New Renderer

Output of `npx react-native info`

System:
  OS: macOS 14.5
  CPU: (12) arm64 Apple M3 Pro
  Memory: 325.67 MB / 36.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.15.0
    path: /usr/local/bin/node
  Yarn:
    version: 3.6.4
    path: /opt/homebrew/bin/yarn
  npm:
    version: 10.7.0
    path: /usr/local/bin/npm
  Watchman:
    version: 2024.06.24.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2024.1 AI-241.18034.62.2411.12071903
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.11
    path: /usr/bin/javac
  Ruby:
    version: 2.6.10
    path: /usr/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.3.1
    wanted: 18.3.1
  react-native:
    installed: 0.75.0-rc.6
    wanted: 0.75.0-rc.6
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

No crash

Reproducer

https://github.com/jpudysz/react-native-new-arch-repro

Screenshots and Videos

Result for paper:

Screen.Recording.2024-08-01.at.09.17.20.mov

Result for fabric:

Screen.Recording.2024-08-01.at.09.21.30.mov

As you can see with new architecture text breaks at the end of the line.

The text was updated successfully, but these errors were encountered:

react-native-bot · 2024-08-01T07:46:50Z

⚠️	Add or Reformat Version Info
ℹ️	We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.70.2

react-native-bot · 2024-08-01T07:46:59Z

⚠️	Add or Reformat Version Info
ℹ️	We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.70.2

j-piasecki · 2024-08-06T07:47:04Z

I'd like to look into this issue

j-piasecki · 2024-08-06T15:33:00Z

After some looking into this, it seems to be caused by the dirty flag used to optimize layout calculations. Changing the state causes a re-render but doesn't affect the layout so it wouldn't dirty the nodes. When the state changes the node (like when it's displayed as a string) the node is dirtied and the layout gets updated.

What I've tried, and seems to be working for text, is checking whether fontSizeMultiplier changed between the commits and dirtying the whole tree when it happens. This can obviously optimized by only dirtying the paths that end with a ParagraphShadowNode, though I don't know whether it's a direction you would want to take this in. It basically handles automatically adjusting the font-sizes after changing the OS setting, since there seems to be a commit triggered when navigating back to the app.

I've also seen some weird behavior around TextInputs, but I'd like to know your opinion before looking further into it. cc @cortinico, @cipolleschi

cipolleschi · 2024-08-07T13:11:04Z

This sounds like a good approach @j-piasecki. I was talking with @sammy-SC about that and he agreed that we should implement it properly.
I suspect that the dirty flag is not set to true when the fontSizeMultiplier changes, right?
We can do that and see if it fixes.

I've also seen some weird behavior around TextInputs

Which are the other strange behaviors you observed?

j-piasecki · 2024-08-07T13:28:10Z

I suspect that the dirty flag is not set to true when the fontSizeMultiplier changes, right?

That's right

Which are the other strange behaviors you observed?

TextInput uses attributed string from the state for measurement in some cases, and the state is updated during layout. During the first pass, the already stored attributed string is used which also stores the fontSizeMultiplier in every fragment.

I've also observed the layout being wrong after changing the scale when dirtying the tree on fontSizeMultiplier change, though I'm not yet sure what exactly is causing it.

This video shows both things:

Screen.Recording.2024-08-07.at.15.23.43.mp4

Dirtying the tree on every commit fixes the layout after text input updates, so the root cause may be similar. I need to look more into it though.

j-piasecki · 2024-08-08T13:16:25Z

I figured out the wrong text layout after pressing the button and it's not related to text input. Yoga caches layout in the nodes, the button press results in appendChild being called with an old shadow node that has the previous measurement cached. This results in a new tree, with an outdated layout being committed. Dirtying the node in the branch where it's known that React has a wrong reference fixes it, but I'm not sure whether it's a proper solution.

I'm not clear on the connection between the JS part and Native code. Do you have any tips for going forward with this? I doubt that I will be able to get much from reading ReactFabric-dev.js 😄.

cipolleschi · 2024-08-08T13:58:00Z

I suppose that setting dirty to false when needed and invalidating the cache when we know should be invalidated should be a proper fix.

ReactFabric-dev.js is the renderer that is used in debug mode. As far as I know, the renderer creates the react commits that are then sent to the UIManagerBinding in C++ which interprets them and convert in shadow node and in the whole tree.

But @sammy-SC is the real expert here, and he just went on PTO today! 😱

Setting dirty for every commit is not a solution as we will lose some optimizations we introduced this year.
But we can start dirtying it when we know it should be dirtied and we can try to invalidate the shadow node caches to retrigger measurements. WDYT?

j-piasecki · 2024-08-12T14:12:24Z

I opened a draft PR with the initial implementation - #45978. Let me know if it's going in the right direction.

Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki

Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki

Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki

Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki

Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki

Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki fbshipit-source-id: 240fb5fa4967a9182bce7e885798b233d1e25aea

jpudysz added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Aug 1, 2024

react-native-bot added the Needs: Version Info label Aug 1, 2024

react-native-bot added Needs: Author Feedback API: Easing Platform: iOS iOS applications. and removed Needs: Triage 🔍 labels Aug 1, 2024

cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Author Feedback Needs: Version Info labels Aug 1, 2024

cortinico assigned j-piasecki Aug 6, 2024

j-piasecki mentioned this issue Aug 12, 2024

Fix changing font scale breaking text #45978

Closed

roryabraham mentioned this issue Feb 13, 2025

[$250] Improvement: Make the "press the green + button" text in the "Submit an Expense" task on mobile a clickable emoji Expensify/App#54643

Open

8 tasks

facebook-github-bot closed this as completed in c008604 Apr 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] increasing font scale (content size catgory) breaks text #45857

[iOS] increasing font scale (content size catgory) breaks text #45857

jpudysz commented Aug 1, 2024 •

edited

Loading

react-native-bot commented Aug 1, 2024

react-native-bot commented Aug 1, 2024

j-piasecki commented Aug 6, 2024

j-piasecki commented Aug 6, 2024

cipolleschi commented Aug 7, 2024

j-piasecki commented Aug 7, 2024

j-piasecki commented Aug 8, 2024

cipolleschi commented Aug 8, 2024

j-piasecki commented Aug 12, 2024

[iOS] increasing font scale (content size catgory) breaks text #45857

[iOS] increasing font scale (content size catgory) breaks text #45857

Comments

jpudysz commented Aug 1, 2024 • edited Loading

Description

Steps to reproduce

React Native Version

Affected Platforms

Areas

Output of npx react-native info

Stacktrace or Logs

Reproducer

Screenshots and Videos

react-native-bot commented Aug 1, 2024

react-native-bot commented Aug 1, 2024

j-piasecki commented Aug 6, 2024

j-piasecki commented Aug 6, 2024

cipolleschi commented Aug 7, 2024

j-piasecki commented Aug 7, 2024

j-piasecki commented Aug 8, 2024

cipolleschi commented Aug 8, 2024

j-piasecki commented Aug 12, 2024

jpudysz commented Aug 1, 2024 •

edited

Loading

Output of `npx react-native info`