fix: Fix underline rendering

[AThePeanut4]

What kind of change does this PR introduce?

• Fix

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 because self.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?

• No

Fixes #2561

[fredizzimo]

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 baseline_snap on so the offset should still be the same

neovide/src/renderer/fonts/font_loader.rs

Line 28 in 818ff73

skia_font.set_baseline_snap(true);

[github-actions]

Test Results

  6 files  ±0    6 suites  ±0   20s ⏱️ +6s
110 tests ±0  110 ✅ ±0  0 💤 ±0  0 ❌ ±0 
644 runs  ±0  644 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 31b97bf. ± Comparison against base commit 818ff73.

♻️ This comment has been updated with latest results.

[AThePeanut4]

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.

[AThePeanut4]

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]

I think the underline is too high now, the right one is rendered with LibreOffice

I also think that we should use this for the thickness, maybe scaled by neovide_underline_stroke_scale

/// Recommended thickness of an underline or strikeout stroke.
pub stroke_size: f32,

Edit: But it was indeed too low before

[fredizzimo]

It looks better now, I still think it would be better to not use antialiasing though, round the sizes and render at exact pixel coordinates. Here's a comparison again between LibreOffice and the current version

[AThePeanut4]

@fredizzimo Yeah the anti-aliasing is just making the lines too thick which messes with everything. It looks much better now.

[fredizzimo]

Thank you, this is good now!

[fredizzimo]

This also greatly improved the undercurl rendering, now text with undercurls is actually readable.