-
Notifications
You must be signed in to change notification settings - Fork 515
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: Fix underline rendering #2564
Conversation
I haven't tested this yet, but is the line thickness consistent with this change? Also when scrolling around? You could try different fractional font sizes. The reason why I'm asking, is that during the development of the fractional grid support, I noticed some effects like that, where the line thickness was changing depending on the scroll position. Therefore, I think it might be good to round the underline position to an integer pixel position. The fonts already have neovide/src/renderer/fonts/font_loader.rs Line 28 in 818ff73
|
I usually use fractional font sizes, and I'm not seeing any issues there. The line thickness is consistent with the latest commit - the only thing that changes is the vertical positioning of the underline. Underlines are thinner than they were prior to 6b04db5, but if I switch to the commit before that and enable anti-aliasing, the lines become as thin as they are now. I think anti-aliasing being disabled is probably also the reason for why the underline thickness was changing depending scroll position. Scrolling doesn't appear to affect the thickness for me on this branch. So yeah given that I have enabled anti-aliasing for all underline types, I don't think rounding the position is really necessary. But I don't really mind either way, it's an easy enough change. I've been developing and testing this only on my Mac - I'll go test it out on my Linux machine and see if I notice any problems. |
I'm not encountering any problems on my Linux machine (Wayland/Hyprland). I've also added a fix for a different issue with the undercurl rendering, where it would quite frequently render one too many squiggles (thus extending into the next cell) because of floating-point inaccuracies in the while condition. |
@fredizzimo Yeah the anti-aliasing is just making the lines too thick which messes with everything. It looks much better now. |
Thank you, this is good now! |
This also greatly improved the undercurl rendering, now text with undercurls is actually readable. |
* fix: underline rendering * Calculate offset relative to font baseline * Prevent undercurl from going too far * Use stroke_size to calculate underline thickness * Disable anti-aliasing and round everywhere
What kind of change does this PR introduce?
Figured fixing #2561 would be fairly straightforward, so I took a stab at it. The core issue was the sign flip in
src/renderer/grid_renderer.rs:167
-self.shaper.underline_position()
returns a negative number so subtracting it from the final underline position was offsetting the underline in the wrong direction. From what I can tell, the reason why it wasn't causing an issue before 6b04db5 was becauseself.shaper.underline_position()
was usually just returning zero due to lost precision from all the float -> int conversions everywhere.I also had to tweak the positioning of the other underline types, since those were based off of the previous incorrect positioning. I enabled anti-aliasing for all underline types because without it the bottom line of the double underline was appearing super thick for me. I couldn't see any negative effects of that change on any of the other underline types.
Did this PR introduce a breaking change?
Fixes #2561