-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Manually draw pixel-perfect glyphs for Box Drawing and Block Elements characters #2409
Comments
I think this depends on the font being used, I can't repro using the canvas (default) renderer. |
@Tyriar how can I share the needed info? |
Can you try using this font? Both your fontFamily and Hack work for me on my Windows 10 box. |
I have tried the following on my home Windows 10 64 bit machine in Yandex Browser (based on Chromium):
|
FYI: Some terminals, including gnome-terminal (vte) and konsole manually draw the U+2500..257F (or even up to 259F) characters, rather than taking them from the font, in order to provide beautifully connected look. Coincidentally both of these emulators took pretty similar technical approach: define the look of (most of) these glyphs as a 5x5 bitmap with uneven "pixel" sizes that are computed runtime based on font size. |
@egmontkob Lulz, I remember doing it likewise in my old emulator. Always wondered why xterm.js does not show that behavior, but never investigated. @Tyriar Just did a few tests with different fonts with chrome and FF with these results - I see in every engine with every font listed in the demo a tiny space (tested in canvas and DOM renderer). The space itself varies with font size and the font itself, FF is one pixel off with a bigger gap (we have that pixel offset in FF also at other places/issues). Conclusion: All of my tested fonts show spaces (from hardly visible to a prominent gap), therefore I think that the glyphs simply dont cover the height completely (just a guess, have not inspected single glyphs with a font tool). The differences in spaces are mostly font renderer related that tries to round/align it to some given font size. Furthermore I dont see any gap in vscode (not even the slightest, tested with |
Perhaps vscode (or at least my setup) is just lucky with its font size, family, window zoom level, etc.
We don't and should not imo clip the cells in the DOM renderer so I don't think this is an option. Provided it doesn't add too much code* we could hardcode our own glyphs for these 👍, it would be good if it was consistent with the DOM renderer though. * I have doubts on this |
Now that #2572 has been marked as dup, this bug is definitely about U+2500..257F "Box Drawing" and U+2580..259F "Block Elements" too. Also keep an eye on forthcoming (Unicode 13) U+1FB00 "Symbols for Legacy Computing", see e.g. VTE 189. |
@egmontkob Lol, I guess those newish glyphs are meant to align perfectly as well? Geez, who needs a font renderer, if one can do it the hard way... |
I'd love to understand the reasons none of the fonts gets it right. But I don't have time and motivation to investigate. |
One interesting thing this could do that fonts couldn't is fill the entire cell when a line height is specified. |
@guregu the canvas renderer by design must clip the rows or it will end up having artifacts showing up, but anyway the plan is to remove the canvas renderer in favor of DOM and webgl (basically superior in every way to canvas). This is still marked as help wanted and maybe it can/should only be done in the webgl renderer in which case it would be a matter of handling those particular characters specially when drawing the char to the texture atlas: xterm.js/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts Lines 304 to 306 in 45077d7
|
FWIW I've managed to fix it without messing with the clipping. Is there a particular reason it was set to Here's a screenshot of the code running from the start of this PR without the fix: |
@guregu previously the webgl renderer used top, but it ended up making the text top aligned: I fear this will happen if we use See #2613, #2645, microsoft/vscode#95813 |
Btw @nojus297 output pretty much looks like an 0.5 px offset issue. I stumbled over antialiasing issues with a 0.5 offset myself the other day, when I messed around with self-drawn curly underlines. Not sure if this applies here at all (still not used to the renderer code lol). |
Would could check your zoom level as well to make sure it's 1. Should we maybe be bumping it down 0.5px when the font size is odd? 🤔 |
I had some friends test out my fix on Windows and it actually makes things worse 😅 |
@guregu thanks for looking into it.
Maybe, we allow that. Something that may work is to clip the character glyph to the size of a cell only for these box drawing characters? |
I've done some experimenting with manually drawing certain characters. Code is rough, but available here: master...guregu:box-drawing Currently, it's two parts, one is Next is I don't yet support the polygon characters from the Symbols for Legacy Computing block, but using similar techniques should work for them. I'm not sure how to go about drawing the shade characters like ░ ▒ ▓, but maybe they could be special-cased. There's also the question of how far we want to go with this. To clean the code up, maybe we could define an interface for drawing segments, and its implementors could be lines, curves, n-th boxes, etc. This is all very experimental, but if it seems like a good idea I'd be happy to contribute (and add support for the WebGL renderer). |
@guregu nice work, a contribution would be awesome, you just need to make sure it's not too derivative of the iTerm2 algorithm as GPL2 is sticky. I'd suggest only doing this for the webgl renderer (not canvas as it's going to be scrapped). Definitely based the lines on devicePixelRatios so the width is great on all monitors.
These should be relatively easy to do but this could easily be deferred as well. The main problem in this area are the solid ones. |
…ox chars Until xtermjs/xterm.js#2409 gets resolved, we're using a fork on xterm.js that has [this patch](xtermjs/xterm.js@master...guregu:box-drawing). See #5.
@meganrogge and I just finished this, here's the result: Canvas (1.1 line height, 1 letter spacing): Webgl (same): The setting is |
@Tyriar Wow, nice work. 👍 So many pretty boxes - must be xmas already, wonder whats inside 🤔 Edit: Any idea for the DOM renderer yet? Just give any box glyph a tiny bigger font-size? Edit2: Since you are both at it - how about those other underline styles/colors? 😸 |
For the DOM renderer I'm not sure we want to add the complexity there of rendering custom glyphs. Especially since I want to make the canvas renderer its own addon which would mean we don't even need the custom glyph code in the core xterm.js bundle.
I think underline styles and colors run into issues packing those bits in, there's also the issue that they probably can't just be added to the glyphs like this since they're repeating patterns that would get cut off, ie. Also special callout to @guregu whose work this builds upon ❤️ |
This was my observation as well when playing around with it here: #1145 (comment) To me it seems underline might need its own render layer with its own progression only loosely coupled to glyph runwidths (like the wave period depends on the cell's global line position not trying to tie it's shape to glyph start). I learned that lesson when debugging browsers standard behavior, which tries to tie the dashed/dotted shape perfectly on glyph start and width, but they kinda fail at various font size miserably (see images in other comment above). |
That's what the link render layer is which is actually also used in webgl for convenience, it just covers hover link underlines right now. It could be done in it's own webgl render pass as well, using an underline style texture atlas and coordinates. |
Details
Steps to reproduce
index.html:
There are several related issues but it does not help:
#475
#992
The text was updated successfully, but these errors were encountered: