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: TextBlock enters an infinite loop if the given available space is too small to fit even one character. #2598

Merged
merged 2 commits into from
Feb 15, 2025

Conversation

net2cn
Copy link
Contributor

@net2cn net2cn commented Jan 19, 2025

PR Details

If TextBlock is given an available space that the width is so small to fit in even one character, TextBlock will enter an infinite loop of repeatedly adding and removing the first character, and creating a new line in the wrapped text. This unexpected infinite loop will crash the editor due to a memory leak.

With this PR, the wrapped text will have at least one character (even if the character is zero width) from the original text in each new line.

Not wrapped

(Wrap Text option disabled, horizontal overflow)

Wrapped

(Wrap Text option enabled with the fix, horizontal wrapped with vertical overflow. Without this fix the engine will crash eventually given enough time)

Related Issue

Fixes #2514

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

@net2cn
Copy link
Contributor Author

net2cn commented Jan 19, 2025

@dotnet-policy-service agree

Copy link
Collaborator

@Eideren Eideren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. One small note, looks good otherwise !

@@ -329,7 +330,13 @@ private void UpdateWrappedText(Vector3 availableSpace)
}

// we reached the end of the line.
if (indexOfLastSpace < 0) // no space in the line
if (currentLine.Length <= 1 || CalculateTextSize(currentLine).X <= 0) // just one or all empty characters... just go one by one.
Copy link
Collaborator

@Eideren Eideren Feb 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you replace CalculateTextSize(currentLine).X <= 0 with lineCurrentSize <= 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, had been on a vacation lately. You're right, they are equivalent. I'll clean it up a bit.

@Eideren Eideren added the area-UI label Feb 3, 2025
It is the same value as lineCurrentSize.

Signed-off-by: net2cn <[email protected]>
Copy link
Member

@Kryptos-FR Kryptos-FR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick reading it looks good to me. I'll let other people do another check, if possible.

@Eideren Eideren merged commit 48701e2 into stride3d:master Feb 15, 2025
3 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Feb 15, 2025

Thanks !

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

Successfully merging this pull request may close these issues.

Stride TextBlock newline infinite loop and memory leak
3 participants