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

GL: improve text rendering performance and quality #24605

Merged
merged 1 commit into from
Apr 20, 2024

Conversation

sarbes
Copy link
Member

@sarbes sarbes commented Jan 29, 2024

Description

The PR moves the mostly CPU driven vertex manipulation of our font system to the GPU. It also improves upon the quality of the rendered text.

Motivation and context

Our font engine has been plagued with quality issues when animating. Especially zooming would look really bad. Letters would jitter when doing the animation, and would rest in "random" positions. Each glyph could stretch in both directions (each glyph in a text body could be distorted differently).

Rotated glyphs are distorted as well. An "H" might look a bit like an "A", as the lower portion might stretch while the upper portion might shrink.

Text shadow would be misaligned if zoomed.

Besides the quality issues, the amount of geometry processing done on the CPU was unjustified. Zooming and rotating would be processed on the CPU. Each frame, a new set of vertex data would have been calculated and uploaded.

On the GPU side, the new default shader will perform better (one less matrix multiplication). The GPU based clipping shader will perform roughly the same.

Overall, the code will be way less complex (when GLES and DX are adjusted).

How has this been tested?

Builds and runs fine. I took some comparison screenshots.

What is the effect on users?

  • Glyphs without zoom/rotation will render the same as before
  • Shadows when scaling are properly aligned
  • No more jittery animation
  • Rotated text is not skewed anymore.
  • Minor performance increase

Screenshots (if appropriate):

The string "Years" used in Estuary, enlarged by a zoom animation. Left: previous implementation. Right: fix. Note the better aligned shadow:
image

The watched status of Arctic Zephyr Reloaded. The letters are totally crooked in the old implementation (left):
image

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • Student submission (PR was done for educational purposes and will be treated as such)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@sarbes sarbes added Type: Improvement non-breaking change which improves existing functionality Component: GUI rendering Component: GL rendering v22 "P" labels Jan 29, 2024
@sarbes sarbes added this to the "P" 22.0 Alpha 1 milestone Jan 29, 2024
@lrusak
Copy link
Contributor

lrusak commented Jan 29, 2024

Can you do GLES also?

@sarbes
Copy link
Member Author

sarbes commented Jan 29, 2024

I'm planning to. It is straight forward.

I just want to get this PR merged before, so I don't have to incorporate possible change requests twice.

@neo1973
Copy link
Member

neo1973 commented Feb 6, 2024

For me scrolling labels stopped working with this change.

OS: Arch
CPU/GPU: Ryzen 7 5700U

@sarbes
Copy link
Member Author

sarbes commented Feb 11, 2024

Oh, yeah.

(What have I got myself into...)

@sarbes
Copy link
Member Author

sarbes commented Feb 13, 2024

It has cost me some hairs, but here we go...

We don't need nearly as much buffers anymore. Previously, each frame of a zoom animation would have its own mesh in the cache. I'm not exactly sure, but it seems to me that those individual zoom levels had a high chance of being displayed just once, effectively destroying the mesh cache.

Now, even reoccurring meshes (like the "Watched" string) share one and the same mesh.
image

I'm sorry for the ifdeffing. It will go once we have similar shaders for GLES and DX.

@neo1973
Copy link
Member

neo1973 commented Feb 13, 2024

Thanks sarbes! It does scroll now but the beginning and of the labels are overlapping:
grafik

Edit: Just noticed that subtitles aren't showing anymore.

@sarbes
Copy link
Member Author

sarbes commented Feb 14, 2024

Fixed the subs.

But I have no idea where the snippet you've posted might be. Every instance of scrolling text (I could find) is working fine for me.

@sarbes
Copy link
Member Author

sarbes commented Feb 16, 2024

The issues should be fixed.

Another cool thing about this PR is the fact that meshes are stored at the "resolution" they are rendered at (plus 0.5px extra margin):
image

@sarbes sarbes force-pushed the font-rendering-improvement branch from 4f362c6 to a7283ef Compare April 10, 2024 08:26
@sarbes sarbes merged commit c7520ee into xbmc:master Apr 20, 2024
2 checks passed
@sarbes sarbes mentioned this pull request Apr 20, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: GL rendering Component: GUI rendering Type: Improvement non-breaking change which improves existing functionality v22 "P"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants