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

Fix: Multi-line text rendered outside control boundaries #17891

Closed

Conversation

dme-compunet
Copy link
Contributor

@dme-compunet dme-compunet commented Jan 5, 2025

What does the pull request do?

It fixes a bug that causes multi-line text to render outside the control's boundaries, resulting in truncated or completely hidden text.

What is the current behavior?

Multi-line text sometimes truncated at the control's boundaries or is not displayed at all. This can happen due to two reasons:

  1. Bidi text processing causes trailing whitespace to reverse and appear before the text, and pushing part of the text out.
  2. Trailing whitespace increases the TextLayout constraint size (stored in TextPresenter._constraint field) during ArrangeOverride, causing text to render outside the control's boundaries.

Example gif:
bug1
bug2

Xaml Code

Gif 1

<StackPanel Margin="6" Spacing="6">
    
  <TextBox Width="100"
            Padding="0"
            CornerRadius="0"
            Text="aaaaaa  b"
            FontSize="30"
            TextWrapping="Wrap"
            FlowDirection="RightToLeft" />
    
  <TextBox Width="100"
            Padding="0"
            CornerRadius="0"
            Text="אאאאא  ב"
            FontSize="30"
            TextWrapping="Wrap" />
    
</StackPanel>

Gif 2

<TextBox Width="100"
          Padding="0"
          CornerRadius="0"
          Text="aaaa bbbb         c"
          FontSize="30"
          TextWrapping="Wrap" />

What is the updated/expected behavior with this PR?

Text is displayed accurately and without truncation.

Example gif:
fix1
fix2

How was the solution implemented (if it's not obvious)?

  1. Added CompressReversedTrailingWhitespace method to check for reversed trailing whitespace due to bidi processing, and if found, sets the GlyphAdvance of whitespace glyphs to 0.
  2. Corrected logic in TextPresenter.ArrangeOverride to check the actual finalSize instead of finalSize including trailing whitespace for TextLayout constraint size.

Checklist

@Gillibald
Copy link
Contributor

The GlyphAdvance should not be altered. Please add a unit tests for the actual issue. Then find a solution that solves this without altering the GlyphAdvance. Did you tests the behavior against latest master?

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0054120-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@dme-compunet
Copy link
Contributor Author

dme-compunet commented Jan 5, 2025

@Gillibald The GlyphAdvance altering seems like the best way to me, the GlyphAdvance ​​altering is also implemented in InterWordJustification.Justify, so why not use it here too?

Please add a unit tests for the actual issue

Once another solution is found (if the current one is not acceptable), then I can add unit tests according to the solution.

Did you tests the behavior against latest master?

Yes

@dme-compunet dme-compunet marked this pull request as draft January 5, 2025 13:47
@dme-compunet dme-compunet marked this pull request as ready for review January 5, 2025 17:45
@Gillibald
Copy link
Contributor

Trailing whitespace is hit testable for FlowDirection.LeftToRight so it should also be hit testable for FlowDirection.RightToLeft. Your changes make that impossible.

@Gillibald
Copy link
Contributor

There is something wrong with TextWrapping and trailing whitespaces. The TextLayout is bigger than the constraint sometimes which causes some issues.

@dme-compunet
Copy link
Contributor Author

I just checked the Adobe InDesign behavior in this case, and it also compresses the whitespaces width, even there they are not hit testable.

@Gillibald
Copy link
Contributor

This is how WPF is handling the text so there is that issue with the TextWrapping I mentioned above.
Screenshot 2025-01-06 094944

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0054131-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@dme-compunet
Copy link
Contributor Author

dme-compunet commented Jan 7, 2025

@Gillibald Is the correct solution to reset the bidi levels for whitespaces at the end of a line to the paragraph embedding level after text wrapping?

https://www.unicode.org/reports/tr9/#L1

Would this be an acceptable solution?

@Gillibald
Copy link
Contributor

Gillibald commented Jan 8, 2025

@Gillibald Is the correct solution to reset the bidi levels for whitespaces at the end of a line to the paragraph embedding level after text wrapping?

https://www.unicode.org/reports/tr9/#L1

Would this be an acceptable solution?

It looks like trailing whitespace should always be at the end in the resolved direction. So this means if the resolved level is RTL the whitespace should be visible at the left side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants