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

Font not rendering smoothly like screenshot #101

Closed
leshow opened this issue Jan 6, 2017 · 29 comments
Closed

Font not rendering smoothly like screenshot #101

leshow opened this issue Jan 6, 2017 · 29 comments

Comments

@leshow
Copy link

@leshow leshow commented Jan 6, 2017

Thanks for your work on this, it's great to have something promising for a new term.

Unfortunately the output of alacritty appears really 'chunky', for lack of a better word, when compared to my regular terminal. I'm not quite sure why.

alacritty:
alacritty

urxvt:
urxvt

I thought maybe it just wasn't working yet, until I saw your screenshot and the fonts looked really smooth. If I can give you any more information to debug please ask.

@insunaa
Copy link

@insunaa insunaa commented Jan 6, 2017

I think it's because of your font. It's too thin and the subpixel rendering doesn't really flatter thin fonts.

@leshow
Copy link
Author

@leshow leshow commented Jan 7, 2017

I'm using a bitmap font in my .Xresources. Could that be it? Does alacritty respect settings from there?

@insunaa
Copy link

@insunaa insunaa commented Jan 7, 2017

No, I don't think it does. You set the font in ~/.alacritty.yml

@leshow
Copy link
Author

@leshow leshow commented Jan 7, 2017

@d3rrial duhhh. sorry about that. I have copied it to the correct location now. Do you have an example of your font setting from the alacritty.yml?

Whatever I put here generates an incoherent parse error.

i.e.

  normal: 'Fira Mono'
  normal: "Fira Mono"
  normal: Fira Mono

all seem to fail

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 7, 2017

@leshow You can get the value you need there by searching with fc-match -a Fira | head. You may also want to put in a specific style to use if the defaults aren't what you want.

As for the yml strings, you shouldn't need quotes at all.

@insunaa
Copy link

@insunaa insunaa commented Jan 7, 2017

mine looks like this:

font:
  normal:
    family: Ubuntu Mono
  bold:
    family: Ubuntu Mono
  italic:
    family: Ubuntu Mono
  size: 13.0
  offset:
    x: 0.9
    y: -10.0

@leshow
Copy link
Author

@leshow leshow commented Jan 7, 2017

Wonderful, thanks guys.

I'll close this issue. Just out of curiosity has anyone tried bitmap fonts? that's what I use in urxvt.

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 7, 2017

You shouldn't need bitmap fonts with Alacritty. We essentially create a bitmap font on-the-fly to amortize the cost of rasterization over the life of the program.

If freetype can load them like other fonts then it would probably work just fine.

@leshow
Copy link
Author

@leshow leshow commented Jan 7, 2017

I know I don't "need" them per-say it's just some of them I find look very nice. I'll experiment. Thanks for all your help guys.

@leshow leshow closed this Jan 7, 2017
@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 7, 2017

@leshow It sounds like you may need a switch to turn of the sub-pixel rasterization. If that turns out to be the case, please file an issue.

@leshow
Copy link
Author

@leshow leshow commented Jan 9, 2017

Having experimented with a bunch of fonts, I'm still finding the rendering not anywhere close to as smooth as your screenshot or other applications.

chunky

Perhaps switching off subpixel rasterization would help?

@leshow leshow reopened this Jan 9, 2017
@leshow
Copy link
Author

@leshow leshow commented Jan 14, 2017

I'd like to submit a PR for a config option to toggle subpixel rendering. Would you be open to reviewing that so I can get it merged?

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 14, 2017

I'd be happy to. I don't have time at the moment, but we should talk about the plan before you get started. I'll try and follow up in a couple of hours.

@leshow
Copy link
Author

@leshow leshow commented Jan 15, 2017

Sure, whenever you've got time you can follow up. I have only briefly looked at the code so far but I thought we could add another field to the Config struct to hold the state of that option. And I've been looking through to see where the application of subpixel rendering on fonts might happen and I'm guessing here?

https://github.com/jwilm/alacritty/blob/67aba7f4e4758c5fcb7eda50f083b138017fbfdb/font/src/darwin/mod.rs#L415

Could we toggle this state based on the config value? We could leave subpixel rendering on by default if there's no value.

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 15, 2017

Yeah the config option part is pretty straight forward. If you're on a macOS system, that's about the right spot for fixing subpixel rendering. If you're on Linux, it's going to be in the FreeType wrapper. Specifically, it can be disabled by removing the lcd filter flag. This changes the number of channels output from the rasterizer from 3 to 1. To keep everything working with the shaders, the number of channels output from our font abstraction needs to remain at 3. This can be accomplished by basically repeating each element 3 times.

@leshow
Copy link
Author

@leshow leshow commented Jan 15, 2017

Awesome, thank you. I have the config value loading from file and I was able to turn off the subpixel rendering, running two terminals side by side I can see one is much less 'smooth' than the other.

The spot I'm missing is getting the value from Config to font. Does it make sense to go through GlyphCache -> Rasterizer? That's the only spot I see with a config ref that talks to font.

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 15, 2017

The rasterizer is initialized in Display::new. You should be able to pass a value in there.

At some point, I would like to define a font::Config type and have the font::Rasterizer take an Into<font::Config> argument so we can just pass Alacritty's Config type.

For now, feel free to add more arguments to the font::Rasterizer::new() method. You will need to update the CoreText renderer as well for the new argument (actually supporting the option can be left for later).

@leshow
Copy link
Author

@leshow leshow commented Jan 16, 2017

Yeah that would be better, especially since it looks like this is a spot where there could potentially be a lot of options. I'd be open to refactoring that section if you like.

I have added the change and tested it, everything appears to work. One thing that confused me was how I should go about 'repeating the element' is this something I've got to do in gl? Sorry to take up so much of your time for a small change.

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 16, 2017

Oh no problem. So, currently the buffer is filled with 3 elements per pixel. So the first three elements are R, G, and B for a single pixel. Without subpixel rasterization, it will output one element per pixel. Basically, you'll need to take this vector and create a new vector with each element repeated 3 times. Just as an example, something like this would do the trick:

let mut out = Vec::with_capacity(rasterized.len() * 3);
for value in rasterized {
    for _ in 0..3 {
        out.push(value);
    }
}

And then put out in the returned Glyph.

@leshow
Copy link
Author

@leshow leshow commented Jan 16, 2017

I should have explained better before, I wasn't caught on the mechanics of repeating an element. I meant what is an 'element' here in the project, which buffer holds the RGB values?

I had already attempted with the packed vector here:
https://github.com/jwilm/alacritty/blob/master/font/src/ft/mod.rs#L114
but with subpixel off, and repeating the elements the terminal fonts look very strange. See picture below:

term

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 16, 2017

Can I see your code as well? I recall now that buffers returned from FreeType have some extra space.

@leshow
Copy link
Author

@leshow leshow commented Jan 16, 2017

Absolutely. I have it on my master fork, I just rebased so it should be a clean compare:

master...leshow:master

Edit: Oh sorry, this is minus the repeating element code. Ill add that one sec.

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 16, 2017

It doesn't look like you're doing the adjustment. I'm not seeing the part where you repeat pixel values to imitate rgb.

@leshow
Copy link
Author

@leshow leshow commented Jan 16, 2017

I had just removed that because it borked everything. I just commited a quick dirty version that breaks rendering if you'd like to take a look, forgive the gross-ness

master...leshow:masterdiff-69add531793c1339df63499d832c2379R118

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 16, 2017

You probably need to change width() / 3 to width() in the returned glyph for the non-subpixel version.

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 16, 2017

You've also got this for v in packed which I think should be for v in buf.

@leshow
Copy link
Author

@leshow leshow commented Jan 16, 2017

@jwilm
Copy link
Collaborator

@jwilm jwilm commented Jan 16, 2017

wouldn't it not render properly without these changes? Just turning the filter off seemed to work and render correctly on Linux.

Had you said this already? Sorry if I missed it. If that's the case, then I'm mixing up my own memories of FreeType. I recall when adding subpixel rendering initially that the non-subpixel mode gave grayscale pixels (only one value). Maybe I had another flag set..

Well, sorry for the confusion. If it works it's probably right.

@leshow
Copy link
Author

@leshow leshow commented Jan 16, 2017

Haha! Yes I did mention it, it's all good. I'll revert those changes and submit the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants