8000 Add support for drawing undercurls by kchibisov · Pull Request #5837 · alacritty/alacritty · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for drawing undercurls #5837

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 8, 2022
Merged

Add support for drawing undercurls #5837

merged 1 commit into from
Feb 8, 2022

Conversation

kchibisov
Copy link
Member
@kchibisov kchibisov commented Jan 30, 2022 •
edited
Loading

Fixes #1628.

picture

@kchibisov kchibisov requested a review from chrisduerr January 30, 2022 11:32
chrisduerr
chrisduerr reviewed Jan 30, 2022
View reviewed changes
chrisduerr
chrisduerr reviewed Jan 30, 2022
View reviewed changes
@kchibisov kchibisov force-pushed the undercurl-support branch 3 times, most recently from 2167542 to 2c9df97 Compare January 30, 2022 14:43
@kchibisov
Copy link
Member Author

Blocked on #5493 . But besides it everything should work.

@kchibisov
Copy link
Member Author

@perfbot

@kchibisov kchibisov added this to the Version 0.11.0 milestone Jan 30, 2022
@kchibisov kchibisov marked this pull request as ready for review January 30, 2022 21:41
@kchibisov kchibisov requested a review from chrisduerr January 30, 2022 21:42
@kchibisov
Copy link
Member Author

I've tested fps on my machine with underlines and it's the same as on master for me. I've applied latest changes, so it's ready for review.

@perfbot
Copy link
perfbot commented Jan 30, 2022

results

@bluz71
Copy link
bluz71 commented Jan 31, 2022

Sorry for the interjection (I will be quick). I've just built and tested the branch. Outstanding job on the rendering style. The undercurls, with my font, look excellent. That applies when I scale the font size up and down. A screenshot:

undercurls

Exciting times for Alacritty, box drawing and undercurls, very nice work to all involved.

@kchibisov kchibisov force-pushed the undercurl-support branch 2 times, most recently from a504424 to de5fca3 Compare February 2, 2022 17:23
chrisduerr
chrisduerr requested changes Feb 2, 2022
View reviewed changes
Copy link
Member
@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd say the curves feel a bit aggressive, but since I won't be using it I don't really care.

@kchibisov kchibisov requested a review from chrisduerr February 2, 2022 22:44
@kchibisov kchibisov force-pushed the undercurl-support branch 3 times, most recently from 3410788 to a04321d Compare February 8, 2022 10:02
chrisduerr
chrisduerr requested changes Feb 8, 2022
View reviewed changes
Copy link
Member
@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think the result looks way too steep and blurry, but if you don't mind it might be okay?

alacritty/res/rect.f.glsl Outdated
int x = int(gl_FragCoord.x - paddingX) % int(cellWidth);
int y = int(gl_FragCoord.y - paddingY) % int(cellHeight);

// We use `undercurlPosition` as amplitude, since it's half of descent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// We use `undercurlPosition` as amplitude, since it's half of descent
// We use `undercurlPosition` as amplitude, since it's half of the descent

alacritty/res/rect.f.glsl Outdated
float undercurl_top = undercurl + (undercurlThickness - 1) / 2.;
float undercurl_bottom = undercurl - (undercurlThickness - 1) / 2.;

// Assume that `gl_FragCoord.y` is on the curve.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still doesn't make sense to me.

alacritty_terminal/src/term/mod.rs Outdated
Comment on lines 1853 to 1855
Attr::CancelUnderline => {
cursor.template.flags.remove(Flags::UNDERLINE | Flags::DOUBLE_UNDERLINE);
cursor.template.flags.remove(Flags::ALL_UNDERLINES);
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can probably remove the linewrap here?

@kchibisov kchibisov merged commit 73c3dd8 into alacritty:master Feb 8, 2022
@kchibisov kchibisov deleted the undercurl-support branch February 8, 2022 17:47
@WhyNotHugo
Copy link

Really glad to see this merged. Thanks a lot to everyone involved! 🎉

@bluz71 bluz71 mentioned this pull request Feb 10, 2022
Closed
@rosvall rosvall mentioned this pull request Apr 27, 2022
Closed
psvenk added a commit to psvenk/dotfiles that referenced this pull request Jun 25, 2022
@psvenk
Vim: enable undercurl
2cc578b 
Added to Alacritty in <alacritty/alacritty#5837>
@the-mikedavis the-mikedavis mentioned this pull request Mar 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisduerr chrisduerr chrisduerr approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
Version 0.11.0
Development

Successfully merging this pull request may close these issues.

Feature request: undercurl support
5 participants
@kchibisov @perfbot @bluz71 33FD @WhyNotHugo @chrisduerr
0