feat!: Support fractional grid and font sizes #2485
Conversation
fredizzimo
force-pushed
the
fsundvik/fractional_fonts
branch
from
35c412b
to
4c7b0db
Compare
|
I improved the skia font caching, which solves most of the performance problems, at least it's very usable on my system now. But there's still some unexplainable stuttering. I don't see any explanation in Tracy, and the GPU is also idling according to the tools that I have in hand right now. I will try to find a way to do better GPU profiling on Linux. |
|
Waiting on a rebase to review this one |
fredizzimo
force-pushed
the
fsundvik/fractional_fonts
branch
from
4c7b0db
to
4d02c55
Compare
| ); | ||
| set_font_cache_limit((percent_font_cache_used * 1.5) as usize); | ||
| } | ||
| pub fn cleanup_font_cache(&self) { |
There was a problem hiding this comment.
This is the only change that gives me pause. How can we be sure that the cache size constant is big enough?
There was a problem hiding this comment.
There are a few reasons:
- Even if the cache is too small it will still work, it just becomes a lot slower.
- The default cache size was 2 MB before, and we never hit that. We would have known, since there was a bug in the growing code which sets it to 1 or 2 bytes, it multiplied by percent not bytes.
- The temporal cache during the scrolling is much bigger than the statically needed cache for just displaying the text, since it needs to store all subpixel positions encountered during the scrolling. With the tracy logger the total cache size never got bigger than around 2MB when scrolling completely through a few big buffers, so with the current code. The cleanup condition was not even met when storing all that in the cache. It would perhaps be worth doing the same with some Asian languages and a big font, and a completely full 4k+ screen, but I'm pretty sure that it's still way more than enough.
- Even if we clean up all the cache needed for scrolling it would just be as slow as when you first start scrolling, which still has a pretty acceptable performance IMO.
|
lgtm baring a quick question about cache sizes |
|
This needs rebased but then I think we should merge it |
NOTE: The grid is still integer sized, so it's rendered wrong
… offsets again The baseline snapping needs to be disabled so that the lines are properly placed, and not snapped to pixels. While using integer based scroll offsets greatly reduces the pressure on the Skia font cache.
A bigger default size with cleanup when idling
fredizzimo
force-pushed
the
fsundvik/fractional_fonts
branch
from
4d02c55
to
e5f7a38
Compare
|
Ok, it's rebased now |
|
So this one looks like it wrecks the menu again, but this time worse as now the shadow also appears to be off, in addition to the border: 
|
|
Ah, sorry, I did not test it properly after resolving the conflicts. I will look at it now. |
|
All good 🙏🏼 Eternally grateful |
|
#2496 should fix it. |
|
But upon closer look, I think we might need to revert this. I think the font render quality is worse, and the text looks too thick and otherwise off. |
What kind of change does this PR introduce?
This properly renders arbitrary font sizes, so the fudge factor is now removed. The hack for floating point font sizes is also removed, since it always caused the wrong spacing.
NOTE: This currently puts much more pressure on the Skia font cache, which can cause glitches when scrolling, so some optimization might be needed.
NOTE: Depends on a lot of other PRs, so it remains in draft until those are merged.
Did this PR introduce a breaking change?
A breaking change includes anything that breaks backwards compatibility either at compile or run time.