Wide nerd font characters are cut to single width #3342
Comments
Wild guess - once again unicode v13 issue I have outlined a few concrete ideas to overcome this in #3304. |
@jerch I guess it's related to that, but for these special characters we could just always override them to be wide. Relevant code: xterm.js/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts Lines 370 to 382 in a510ffb |
To me this unicode stuff feels like chasing the white rabbit - we are always late missing crucial aspects, lol. We cannot do this on renderer level alone, as it would screw up the buffer alignment, it def. needs a fix on wcwidth level. But permanently adding it to the v6/v11 tables is imho the wrong move, if they were meant as wide in v6/v11 they already would take 2 cells there. |
Are you sure these are real unicode characters? I was under the impression nerd fonts took unused characters and leveraged the fact that terminals normally render the whole char without trimming like we do sometimes, it's only because we're trimming that this is a problem but our font rendering requires it. |
@Tyriar Well idk anything about nerd fonts internals - my guess would be that they overwrite several codepoints with their own glyph set to get the graphical output they want. Do they use unspecified areas? If so we'd need a global rule for that, but thats kinda dangerous if not backed up by some global rule from the consortium (or some limited pre-specification). Do you have access to the bytes for the output above? Would help to clarify where those chars come from. Edit: Edit2: |
Hey guys. Would you guys mind telling me what the codepoints are for those 2 glyphs being displayed in the above screenshots? I'd like to investigate a little too. (Especially: I think there should be no powerline specific code paths in any terminal emulator, but rather a generic solution for the common case). Many thanks :) |
@christianparpart No, not a powerline specific code path in any terminal emulator directly, but some way to configure it from appside. PUA codepoints can stand for anything, they are left unspecced for the purpose to get vendor/application defined. Since a terminal is the "screen part" of a console application, we would have to make it configurable from that side, so an app using nerdfont symbols can activate the appropriate unicode metrics. Or whatever app with whatever idea of PUA. Welcome to under-specced unicode hell, and ppl using nice looking things without bothering, why it works on their OS version but nowhere else. |
When I worked on this, I installed powerline10k to do some tests and from what I understood the way they handle the powerline glyphs is by manually adding a space character (there is an specific option for this) so that the next glyph doesn't render over the powerline glyph |
@jeanp413 While the space trick would work in xterm.js, I consider this not as a good workaround, because a TE might actually have to do some erasing on an incoming whitespace again cutting things off. Furthermore the fact that this issue exists tells me, that the shell currently does not add those whitespaces. The space trick would fix the issue from the wrong side imho. The only reliable longterm fix is to have correct unicode metrics*. For PUA this means we need some way to get that information from somewhere, as it does not exist in the unicode spec itself. This can either be by some TE extension/setting (hardcoded, not preferred as it needs user interaction), or some scriptable interface from appside (preferred, as only the app knows if it uses unspecced things). [*] As of now xterm.js would just need the correct codepoint runwidths, and later on also their grapheme classification. |
This issue is specific to the webgl renderer (there's some logic that cuts the width of a glyph to just one cell, I fixed this in my PR), powerline glyphs render correctly in the canvas renderer and the whitespace is added by the shell |
Yes the canvas renderer works around this issue with this hack: xterm.js/src/browser/renderer/TextRenderLayer.ts Lines 112 to 134 in fe1d2f6 Since I dont have powerline installed I cannot test the DOM renderer. So do you think creating another hack to work around the existing whitespace hack is good practice? Btw the nerdfont guys are well aware of that issue, they even have a patched To me it makes no sense trying to fix something at renderer level, when we clearly know, that the faulty handling is caused by the parser. Imho we should fix the source of the problem, not just beautify the screen output until it fits. |
Oh, no, not at all, in fact in my PR I removed the |
@jeanp413 the whitespace hack just makes sure something isn't printed on top of the glyph if its interpreted as a single width character. I'm not sure measureText will work since the |
@Tyriar With the tests I've done so far it seems to be working fine. Do you know some edge cases that I could test?
|
The images look like a rounding issue at the clipping borders to me. I had the same issue in the image addon and solved it by doing a floor offset at the right border and the left border of the next cell, which moves things slightly to the left but guarantees perfect clipping. Ofc with flooring there is a chance to cut off some subpixel information at the right side, if the source data got stretched with antialias into that region. But meeh, imho this can only be partially fixed and involves much bigger atlas resolution (here glyph textures). |
You draw a glyph at 0x0 and its AA will overflow to the left, we allow/want this generally for nice flowing text, but not for powerlines glyphs which need to be pixel perfect. |
Since Powerline glyphs are in PUA you could use that as condition, or is that a bad idea? Also,maybe off-topic. But i would like to understand. You are using WebGL for rendering but the Webbrowser stack does the text stuff (IIRC), does this mean you can get the browsers render stack to render to texture instead of a window surface? |
@christianparpart what's PUA? Yes the webgl renderer uses a 2d canvas context to render the text to a texture which is then uploaded to the GPU, so the browser does the text rendering for us. |
Powerlines chars were trimmed to only fill a single cell in #3279, we need to look at the character codes in nerd fonts and make sure wide ones are trimmed to 2 cells.
Actual:
Expected (Terminal.app):
VS Code issue: microsoft/vscode#123629
The text was updated successfully, but these errors were encountered: