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

various updates and improvements for v0.3.0 #17

Merged
merged 33 commits into from
Apr 2, 2024
Merged

various updates and improvements for v0.3.0 #17

merged 33 commits into from
Apr 2, 2024

Conversation

phip1611
Copy link
Owner

@phip1611 phip1611 commented Jan 9, 2024

@phip1611 phip1611 self-assigned this Jan 9, 2024
Use proper font metrics for calculating line height & update fonts
@phip1611 phip1611 changed the title various updates and improvements various updates and improvements for v0.3.0 Jan 9, 2024
@phip1611
Copy link
Owner Author

phip1611 commented Jan 9, 2024

Hm. @toothbrush7777777 I can't merge it like this, unfortunately. :/

If you run cargo run --example show_chars_in_window --features all, you will see:

image

The character is pretty wide. How do we want to solve this nicely? Jetbrains IDEs solve
it is like this:

image

They just center it and cut it at the edges.

@toothbrush7777777
Copy link
Contributor

toothbrush7777777 commented Jan 12, 2024

@phip1611 Okay, I'll add some extra code to check if the character is too wide. The main issue seems to be that the font doesn't contain the replacement symbol (); I'm not sure where that comes from. Do you want to scale the character down (e.g using image) or use only the centre part?

@phip1611
Copy link
Owner Author

phip1611 commented Jan 12, 2024

The main issue seems to be that the font doesn't contain the replacement symbol

I don't understand why you say that. The replacement character is there? It is just cut of on the right side?

I'm not sure what's the right strategy to handle very wide characters, either. If it's not too complicated, you can try to make the symbol smaller. Otherwise, we should center it and cut it at the edges

@phip1611
Copy link
Owner Author

@toothbrush7777777 Any update?

@toothbrush7777777
Copy link
Contributor

toothbrush7777777 commented Jan 28, 2024

I don't think that the Noto Sans Mono font contains the replacement character glyph because I see an outlined box, not the glyph that you see.
I tried a couple of things with centring the glyph, which seems to work as far as I can tell. I'll try it on another Linux system which has the full desktop to double-check.

@toothbrush7777777
Copy link
Contributor

toothbrush7777777 commented Jan 29, 2024

Anyway, I've fixed it in #18 by centring the glyphs. Scaling the replacement character down looked terrible at sizes smaller than 24px.

@phip1611
Copy link
Owner Author

phip1611 commented Feb 2, 2024

Thank you so much for working on this! I'll merge it as soon as I'm back from my long vacation in March

@phip1611
Copy link
Owner Author

I still have this on my list! Sorry for the long delay

@phip1611
Copy link
Owner Author

This is a screenshot of the current rendering in https://github.com/rust-osdev/bootloader (on the right). The left side shows my system's default mono space rendering using Source Code Pro as font:

image

So either something that we do or Noto Sans Mono itself causes the high line height. Nevertheless, I think that this is the best that we can get - for what it is.

@phip1611 phip1611 merged commit 32681b0 into main Apr 2, 2024
26 checks passed
@phip1611 phip1611 deleted the dev branch April 2, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants