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

Improve texture atlas utilization and fix glyph corruption when merging #4732

Merged
merged 2 commits into from Aug 26, 2023

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Aug 26, 2023

  • Changed the threshold at which pages start to merge from MAX/2 to MAX.
  • If a glyph is about to create a new page that will be merged, check if the existing row fits when ignoring the row pixel threshold. This will improve texture utilization by using the available space before the page is merged and becomes static.
  • The corruption was caused by confusion with page indexes when merging pages. The fix is to simplify it by deleting all pages and then just adding the new page.

Pages merged at the bottom of the demo after running CJK test button below

Before:

Screenshot 2023-08-26 at 9 42 42 AM

After:

Screenshot 2023-08-26 at 9 43 22 AM

After the first page doesn't get merged as expected as there aren't many short glyphs:

Screenshot 2023-08-26 at 9 43 37 AM

- Changed the threshold at which pages start to merge from MAX/2 to
  MAX.
- If a glyph is about to create a new page that will be merged,
  check if the existing row fits when ignoring the row pixel
  threshold. This will improve texture utilization by using the
  available space before the page is merged and becomes static.
The corruption was caused by confusion with page indexes when
merging pages. The fix is to simplify it by deleting all pages
and then just adding the new page.

Fixes xtermjs#4534
Fixes xtermjs#4351
@Tyriar Tyriar added this to the 5.3.0 milestone Aug 26, 2023
@Tyriar Tyriar self-assigned this Aug 26, 2023
@Tyriar Tyriar enabled auto-merge August 26, 2023 17:24
@Tyriar Tyriar disabled auto-merge August 26, 2023 17:25
@Tyriar Tyriar merged commit a0d0c92 into xtermjs:master Aug 26, 2023
14 checks passed
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.

None yet

1 participant