Fabric `Text` component has some unnecessary redraws

[chrisglein]

Problem Description

I was debugging through some Text behavior on Fabric and noticed some logic that can trigger a redraw when one isn't necessary:

react-native-windows/vnext/Microsoft.ReactNative/Fabric/Composition/ParagraphComponentView.cpp

Line 46 in a8aa860

if (oldViewProps.textAttributes.foregroundColor != newViewProps.textAttributes.foregroundColor) {

This compares the color values, which is going to check for equality of both the resolved m_color and the m_platformColor. However it's possible that there are different platform color values that resolve to the same color. Example I'm seeing: "ButtonForeground" and "ButtonForegroundPointerOver" both resolve to #00000000. There should be no rendered change, but we set this as needing a redraw.

react-native-windows/vnext/Microsoft.ReactNative/Fabric/Composition/ParagraphComponentView.cpp

Line 49 in a8aa860

if (oldViewProps.textAttributes.opacity != newViewProps.textAttributes.opacity) {

This compares the opacity value, but opacity can be NaN. And if you compare NaN to NaN, it will return false. Effectively these should be the same, but once again it invalidates the text.

Steps To Reproduce

Place a breakpoint in DrawText and see it get hit when you wouldn't expect (e.g. hovering over a Button).

Expected Results

No response

CLI version

12.0.0-alpha.6

Environment

info Fetching system and libraries information...
System:
  OS: Windows 10 10.0.23528
  CPU: "(24) x64 AMD Ryzen Threadripper PRO 3945WX 12-Cores     "
  Memory: 37.38 GB / 63.86 GB
Binaries:
  Node:
    version: 18.17.1
    path: C:\Program Files\nodejs\node.EXE
  Yarn:
    version: 1.22.19
    path: C:\Program Files (x86)\Yarn\bin\yarn.CMD
  npm:
    version: 9.4.1
    path: C:\Program Files\nodejs\npm.CMD
  Watchman: Not Found
SDKs:
  Android SDK: Not Found
  Windows SDK:
    AllowDevelopmentWithoutDevLicense: Enabled
    AllowAllTrustedApps: Enabled
    Versions:
      - 10.0.18362.0
      - 10.0.19041.0
      - 10.0.22000.0
      - 10.0.22621.0
IDEs:
  Android Studio: Not Found
  Visual Studio:
    - 17.6.33829.357 (Visual Studio Community 2022)
    - 16.11.33801.447 (Visual Studio Community 2019)
Languages:
  Java: Not Found
  Ruby: Not Found
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.0-nightly-20230721-ccc50ddd2
    wanted: 0.73.0-nightly-20230721-ccc50ddd2
  react-native-windows:
    installed: 0.0.0-canary.693
    wanted: 0.0.0-canary.693
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

Target Platform Version

None

Target Device(s)

No response

Visual Studio Version

None

Build Configuration

None

Snack, code example, screenshot, or link to a repository

No response

[chrisglein]

Something like this?

  // Opacity could be set to NaN, and comparing NaN to NaN will return false, so we need to check
  if ((oldViewProps.textAttributes.opacity != newViewProps.textAttributes.opacity) && 
      !(std::isnan(oldViewProps.textAttributes.opacity) && std::isnan(newViewProps.textAttributes.opacity))) {
    m_requireRedraw = true;
  }

Not sure if there is something more concise. How many places do we call this?
Looks like Image and View have the same comparison (but not necessarily the same problematic invalidation assumption):
https://github.com/search?q=repo%3Amicrosoft%2Freact-native-windows+opacity+path%3Avnext%2FMicrosoft.ReactNative%2FFabric&type=code