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

Issue rendering ➜ & ✔ glyphs #160

Closed
icaromh opened this issue Jul 26, 2023 · 24 comments
Closed

Issue rendering ➜ & ✔ glyphs #160

icaromh opened this issue Jul 26, 2023 · 24 comments
Labels
bug Something isn't working
Milestone

Comments

@icaromh
Copy link

icaromh commented Jul 26, 2023

Hey 👋🏻

First of all, fantastic project.

I've been using ohmyz.sh with robbyrussell theme + Dracula colors.
And I faced some issues rendering the characters and , they appear cropped. I've tried to change the terminal font sizing, without success.

For reference, below is a screenshot comparing iterm vs rio:
image

Not sure, but it can be related to #135?


System:

  • Rio 0.0.14 (20230528.115631) installed via brew
  • robbyrussell
  • oh-my-zsh
  • macos (MacOS 13.4.1)
  • Monaco font
@raphamorim raphamorim added this to the 0.0.15 milestone Jul 26, 2023
@raphamorim
Copy link
Owner

Thank you @icaromh for the bug report, yes I am aware of this bug (think was @ralgozino that mentioned in the discord channel). Glad that now we have a proper issue to track.

Moved it to next release wishlist (0.0.15) 🙏

@raphamorim raphamorim added the bug Something isn't working label Jul 26, 2023
@ralgozino
Copy link
Contributor

Yes, I can confirm is the same thing

@raphamorim
Copy link
Owner

Fixed at main

Screenshot 2023-07-27 at 01 39 10

Screenshot 2023-07-27 at 01 41 00

@ralgozino
Copy link
Contributor

Now I'm not sure it was the same issue... 😆

This is my shell running main:
image

iTerm2 for reference:
image

I think the difference is that these are 2-width glyphs though (and they come from nerd fonts), should I open another issue?

@raphamorim raphamorim reopened this Jul 27, 2023
@raphamorim
Copy link
Owner

Weird though that even your checkmark wasn't affect by the change.

I don't believe the icons should 2 char width, I know Iterm2 (I think terminal.app too) does that way but I feel is more for the "style" than follow ANSI rules. For example Alacritty and Kitty:

Screenshot 2023-07-27 at 10 30 45 Screenshot 2023-07-27 at 10 33 52

Although I plan to allow the user define if want a multi width for a specific font in the next version.

@raphamorim
Copy link
Owner

This is my shell running main:

Was it cropping before as well right?

@ralgozino
Copy link
Contributor

This is my shell running main:

Was it cropping before as well right?

It started cropping with the fix to #148

There's some improvement on main though (top window is Rio 0.0.14, bottom main):
image

@raphamorim
Copy link
Owner

Very interesting because I am not able to reproduce it, before at least for me the icons always have been rendered with 2x less width/height to fix 1 char width. Like the image below

Screenshot 2023-07-27 at 10 51 20

So my assumption was ➜ & ✔ glyphs were coming from unicode fonts instead of icons which was the case (at least for me)

Which font/font-size are you using @ralgozino ?

@raphamorim
Copy link
Owner

Would you mind share the configuration file?

@ralgozino
Copy link
Contributor

16, but I tested with 14 and 18 and got the same result

❯ cat ~/.config/rio/config.toml
font = "VictorMono Nerd Font"
font-size = 16
window-width=1200
window-height=900

I don't know if it's related, but the glyph is smaller than it should be:
image

@raphamorim
Copy link
Owner

ah I got it, VictorMono Nerd Font has the icons inside of the font right?

@raphamorim
Copy link
Owner

Because Rio is bundling the Nerd Font so you wouldn't need to have a font with nerd fonts patched, but it has been considering the icons aren't coming from a text font.

@raphamorim
Copy link
Owner

raphamorim commented Jul 27, 2023

Can you just check if you get same results using a system font (menlo, consolas for example)? (just for a sanity check)

@ralgozino
Copy link
Contributor

ah I got it, VictorMono Nerd Font has the icons inside of the font right?

correct

Can you just check if you get same results using a system font (menlo, consolas for example)? (just for a sanity check)

commenting out the config for the font and font-size, so using the default font (I believe it uses an integrated font, right?) it renders better but the glyphs are still a little small:

image

@raphamorim
Copy link
Owner

raphamorim commented Jul 27, 2023

Gotcha, ok so logically make sense: sugarloaf is considering text font as text but it can happen to have icons that aren’t monospaced. This is something that needs a fix.

regarding the small icons is a very tricky issue because iterm2 and terminal.app doesn’t follow strictly the ANSI rules regarding cells. Icons aren’t characters with width bigger than 1.

kitty has same issue kovidgoyal/kitty#1463

Nerd fonts use private use unicode characters. These are all single cell characters. If you want them rendered in more than a single cell, you have to follow them with one or more spaces, then kitty will use the space cells to render the character as needed.

Kitty has one valid approach around it.

I think for now fix the renderer to understand glyph width and reposition to 1 cell.

Then allow people to configure width of specific characters or fonts. So could say “I want all the icons as char-width 2”.

But support icons as 2 cells for special unicode can’t be the default behaviour (only by configuration)

@ralgozino
Copy link
Contributor

ralgozino commented Jul 27, 2023

But support icons as 2 cells for special unicode can’t be the default behaviour (only by configuration)

as a user, I find this is acceptable. I understand that no everybody uses this glyphs or complex prompts :-)

FYI: I tried using Victor Mono as font instead of the patched version VictorMono Nerd Font as suggested in the linked issue, and the result is similar to using the default font, i.e. glyphs are rendered better but still small:

image

EDIT: I've just realized that when using Victor Mono, the underline does not work anymore (it does in iTerm).

@raphamorim
Copy link
Owner

raphamorim commented Jul 27, 2023

@ralgozino can you test later using the branch dynamic-font-calculator? https://github.com/raphamorim/rio/tree/dynamic-font-calculator

My guess is that it will not work (if doesn't add Mono in the end).

works

font = "FiraCode Nerd Font Mono"

should not

font = "FiraCode Nerd Font"

after you confirm, I will add monospace as query in the font loader.

@ralgozino
Copy link
Contributor

I don't notice any difference... what should I be looking at?
image

@raphamorim
Copy link
Owner

raphamorim commented Jul 27, 2023

Ok at least I know that your icons aren't cropped at least 🙏 (just small)

image

@raphamorim
Copy link
Owner

is ➜ & ✔ glyphs ok?

The problem is just the underline right?

@ralgozino
Copy link
Contributor

ralgozino commented Jul 28, 2023

Hi @raphamorim

here are some tests (from main because I saw you merged the branch):

default config:
image

using Victor Mono (without the Nerd font glyphs):
image

using VictorMono Nerd Font (Victor Mono patched with the Nerd font glyphs):
image

so, I'd say that the smaller glyphs problem is common to all the cases, but using Victor Mono also doesn't draw the underline.

I noticed that using Victor Mono the italics also don't get rendered correctly:
image
image

The difference is evident in the comments, I think that Rio is not detecting the available features of the font. Italics and Underline work as expected in iTerm2.

@raphamorim
Copy link
Owner

so, I'd say that the smaller glyphs problem is common to all the cases, but using Victor Mono also doesn't draw the underline.

That's expected correctly, the smaller icons should be default behaviour until Rio has the configuration to allow it with char width 2.

Regarding Victor Mono yes, I download yesterday and honestly everything looks weird. i will need to investigate that one tbh.

@ralgozino
Copy link
Contributor

the good thing is that glyphs are not being cut anymore 😄 🎉

@raphamorim
Copy link
Owner

Ok, I will close this one and open an issue regarding Victor Mono. Thanks @icaromh @ralgozino for reporting and testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants