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
feat: improved theme png rendering #971
feat: improved theme png rendering #971
Conversation
Overall this improves glyph rendering in the theme PNGs for docs. Changes: - Replaces VictorMono with latest Hack nerd font - When calculating overall width, count glyphs as double wide - When rendering a glyph, allow ~1.7x space (this seemed right testing many) I hope this is a net improvement that helps theme renders be a bit more accurate and shine!
1.7 introduces some odd offset problems with some backgrounds - 2x is stable.
https://github.com/ryanoasis/nerd-fonts/wiki/Glyph-Sets-and-Code-Points ...checking those points for nerd font glyphs in play I'm unfamiliar with Go here and maybe there's a better way to implement, but the results are much improved.
Go checks were unhappy - this isn't a hot path so breaking it out into a format that matches the docs, for easier maintenance.
...but I hope this works - bit simpler layout to maintain too.
@NickCraver this isn't a bad take. To be honest, I was so happy to get this to work in the first place, so this part of the codebase can be pragmatic. Will there be a cleaner way? Probably. Is there a lot of ROI when spending the time on it? Highly unlikely. |
@@ -320,9 +372,11 @@ func (ir *ImageRenderer) SavePNG(path string) error { | |||
} | |||
|
|||
w, h := dc.MeasureString(str) | |||
w += (w * float64(ir.runeAdditionalWidth(runes[0]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is done to double the width or not, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed! I tried to go through and add comments on all points. It was light on comments before so wasn't sure on stance there, but hopefully the additions here are good - happy to tweak more!
Good PR feedback - adding some more commentary for anyone coming across these bits later
Prerequisites
CONTRIBUTING
guideDescription
This is making the following changes to the PNG render pipeline for themes:
Here are some example before (based on current website) and with this change:
Some things I don't love:
...is this a bad idea? I'm not sure - I hope it helps or sparks another idea at least. My feeling is the result is a net win (unless people hate the font!), but the implementation is icky. If the gut reaction to this is "omg Nick, wtf?!?" I don't mind it getting closed one bit, only trying to explore a possible path here to make the themes a bit more accurate in the previews.