search

microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.33k stars 1.14k forks source link

React Native occasionally truncates text #9343

Open rozele opened 2 years ago

rozele commented 2 years ago

Problem Description

Filing this here, since I can only repro on react-native-windows right now, but I found a very peculiar case where text gets truncated when it should wrap. I believe this is a Yoga caching issue, but react-native-windows may be the best place to debug.

Steps To Reproduce

  1. Copy app from Gist here: https://gist.github.com/rozele/7a8704a69ff548613407c0f9fb72257b
  2. Resize window on the boundary of where the text would break from one line to two (very slowly and carefully, as shown in the video):

https://user-images.githubusercontent.com/1106239/148614641-78c33fc5-aad5-4f81-a8c2-5c5699794a1d.mp4

  1. Notice that, if the stars align (and presumably the Yoga node measurement caches are in the right state), the text will be truncated.

Expected Results

Text should never be truncated.

CLI version

npx react-native --version

Environment

npx react-native info

Target Platform Version

No response

Target Device(s)

No response

Visual Studio Version

No response

Build Configuration

No response

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

See video and Gist above for a minimal-ish repro.

rozele commented 2 years ago

Please note, this repros on DPI / scale settings of 150%, which I believe is important because it affects rounding errors when checking cached measurements.

chrisglein commented 2 years ago

Primary question is whether this some sort of DPI rounding issue in UWP, or whether it's the interaction between Yoga and the quirky UWP rounding that's causing the issue.

@rozele outside of the reduced repro (thanks for that), does this impact your product on a static layout, just during resize, or any other flavor you can offer?

rozele commented 2 years ago

We've had bug reports of this occurring in static layout for message bubbles in Messenger (without resizing the window), which makes me think this could be a DPI rounding issue in UWP / XAML.

Here's what I know about Yoga - there definitely seems to be an occasional bug where a cached flex-measured ancestor of text pulls a height from cache that is smaller than the measured text height, and switches to exact measurements rather than undefined / flexible measurements for the text node. This is likely the issue that emerges when resizing the window.

However, it may be the case that there is non-determinism in XAML where it occasionally reports that it can squeeze the text into one less line and the cache is actually correct, so the root cause may be XAML in both the window resize and static layout case.

Here's what I know about layout for text on react-native-windows (today):

  1. We do not customize the scale factor in the YogaContext, so we use a point scale factor of 1 regardless of DPI settings. This is probably a good thing, because apps may have multiple windows residing on different screens with different DPI settings, and we'd need to refactor react-native-windows to create a per-root YogaContext that is applied to children and also add APIs to track/update for DPI changes for UWP / XAML Islands / WinAppSDK. Here's where we'd need to wire all this up: https://github.com/microsoft/react-native-windows/blob/59df2c597641c9beee3a4eafcb3b880f5e040f6e/vnext/Microsoft.ReactNative/Modules/NativeUIManager.cpp#L144
  2. I have experimented with explicitly specifying the point scale factor on the YGContextRef to match the OS settings for DPI and still reproduced the bug (though only while resizing the window). I think the sequence might look like the following:

Text line height is 20, based on constraints, measures or pulls from cache 80w x 40h logical pixels Ancestor pulls from cache from previous measurement where text fit in 100w x 20h When reconciling the parent constraint height of 20p and available width of 100p, text gets constrained to 80x20.

Here's some things we should probably look into:

  1. Add logging so understand the circumstances where the "bad" height gets cached when resizing the window - can we confirm in this case that the text node measured to the same height, and if so was XAML actually able to render the text into one line at that point?
  2. Is XAML text measurement 100% deterministic? Given the same constraints, will it always return the same desired size?
  3. Perhaps this could occur due to XAMLs arrange pass. I haven't investigated this yet, but given we round to whole pixels, perhaps rounding to a whole pixel for layout position truncates the space available when XAML does it's own rounding when mapping logical to physical pixels. E.g., consider the following:

Imagine you have 150% scaling, and you have text that measures to 30 logical pixels. This maps to 45 physical pixels. If the left position of the view gets mapped to 1 logical pixel (which would sit physical pixel 1.5), where does XAML actually position the view, and is there any way that XAML ends up forcing the view to fit into less than 30 logical / 45 physical pixels depending on where the left position gets rounded?

lyahdav commented 2 years ago

I'm investigating this since we've received a lot of reports of this issue. In order to debug this better, I created a branch where I can reproduce the issue like @rozele detailed above. I also put in a hack fix which I think will be the foundation for a PR soon.

Here's a video of me using the hack to fix the bug (after I resized the window to repro the bug):

https://user-images.githubusercontent.com/359157/167702385-7c3fdf6d-aa11-418f-a9b1-9b437d396571.mov

That menu item just marks all Text nodes as dirty (the menu item name is misleading, it doesn't dirty just the last one). Next my plan is to subscribe to the TextBlock.IsTextTrimmedChanged Event and dirtying the Text node there.

lyahdav commented 2 years ago

Looks like the IsTextTrimmedChanged works. Here's a branch in my fork with the WIP fix: https://github.com/lyahdav/react-native-windows/tree/yoga-text-wrap-bug-with-fix. Here's a video showing how the bug momentarily appears and then fixes itself:

https://user-images.githubusercontent.com/359157/167716399-33417764-b126-4740-9bc7-dc3089680eee.mov

Ideally the Text wouldn't look truncated momentarily, but I'm not sure how to do so and I think this is a good enough fix for now.

As a next step I'll work on a PR.

lyahdav commented 2 years ago

I'm still trying to root cause this as the PR I created above may lead to other issues (e.g. a layout cycle). For posterity, this branch in my fork is in a good state for reproducing the issue now as it has a bunch of logging from Yoga: https://github.com/lyahdav/react-native-windows/tree/yoga-text-bug-root-causing

lyahdav commented 2 years ago

Note also I can repro on display scale of 125% about as often as I could on 150%.

Edit: This may not be true, I mistakenly thought I was on 125% when I was on 150%. Still TBD.

lyahdav commented 2 years ago

@acoates-ms and I are still debugging this issue. The latest theory is a bug in XAML since we see that when the text is truncated the XAML debugger shows that the ActualWidth property doesn't match the Width property. Also by inspecting the view's properties with the XAML debugger it fixes the bug, somehow triggering a layout and fixing the mismatch between the Width and ActualWidth properties.

One thing I might try next is seeing if I can repro this outside of RNW.

Also, we found that there were previous PRs and issues in this repo about related issues, though none of them gave a clue to how we should fix this issue:

Here's a branch in my fork where the bug happens on launch: https://github.com/lyahdav/react-native-windows/tree/yoga-text-bug-repros-on-launch-at-150-scale. We were also able to reduce the JS code in the test case while still reproducing the bug.

Other things we tried that don't fix this issue:

acoates-ms commented 2 years ago

Ok, I think I understand what is happening.

In the specific example in the branch above, yoga is providing 60% of the width to the text. The window is 1252 wide, so 60% is 751.2px (Or 500.799988 dip).

When calling measure on the TextBlock with that available width. DWrite determines the text metrics to be: {width=500.734863 height=19.9511719 } Then XAML pixel aligns the value to be: {width=501.333344 height=20.0000000 } [Note this size is now larger than the original available] Then XAML clips the desired size to the available size, since the desired shouldn't be larger than the available, which trims the width to 500.799988 again - but note this doesn't increase the height as it should if its wrapping text. Then XAML pixel aligns the value again, giving us a desired size of: {width=500.666656 height=20.0000000 }, which is smaller than it should be.

acoates-ms commented 2 years ago

I think adding this code before the call to element.Measure in DefaultYogaSelfMeasureFunc should fix the issue.


    if (auto text = view.try_as<xaml::Controls::TextBlock>()) {
      if (auto xamlRoot = text.XamlRoot()) {
        auto scale = static_cast<float>(xamlRoot.RasterizationScale());
        availableSpace.Width = floor(availableSpace.Width * scale) / scale;
        availableSpace.Height = floor(availableSpace.Height * scale) / scale;
      }
    }

And please remove the hack added previously in GetConstrainedResult while you are at it.

lyahdav commented 2 years ago

@acoates-ms I can confirm that at least fixes the scenario we reproduced, thanks!

In this branch without the fix you can see the text is truncated: image

In this branch with the fix the text is no longer truncated: image

As a next step I'll try the fix in that scenario where the bug was only reproducing while resizing the window and report back.

lyahdav commented 2 years ago

@acoates-ms Turns out your proposed fix doesn't resolve this bug in all cases. @rozele is going to take this over as he says he's close to having a fix.

rozele commented 2 years ago

For a deterministic repro of this bug in Yoga: https://github.com/facebook/yoga/pull/1161

rozele commented 2 years ago

Re-opening, as this is not a complete fix.