Skip to content

fix: TextBlock enters an infinite loop if the given available space is too small to fit even one character.#2598

Merged
Eideren merged 2 commits into
stride3d:masterfrom
net2cn:textblock_fix
Feb 15, 2025
Merged

fix: TextBlock enters an infinite loop if the given available space is too small to fit even one character.#2598
Eideren merged 2 commits into
stride3d:masterfrom
net2cn:textblock_fix

Conversation

@net2cn

@net2cn net2cn commented Jan 19, 2025

Copy link
Copy Markdown
Contributor

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

net2cn commented Jan 19, 2025

Copy link
Copy Markdown
Contributor Author

@dotnet-policy-service agree

@Eideren Eideren left a comment

Copy link
Copy Markdown
Collaborator

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 !


// 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.

@Eideren Eideren Feb 1, 2025

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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 <mcopener@gmail.com>

@Kryptos-FR Kryptos-FR left a comment

Copy link
Copy Markdown
Member

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
@Eideren

Eideren commented Feb 15, 2025

Copy link
Copy Markdown
Collaborator

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