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

Support PowerLine triangle and diagonal line glyphs #4258

Closed
Tyriar opened this issue Nov 9, 2022 · 10 comments · Fixed by #4313
Closed

Support PowerLine triangle and diagonal line glyphs #4258

Tyriar opened this issue Nov 9, 2022 · 10 comments · Fixed by #4313
Labels
help wanted type/enhancement Features or improvements to existing features
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Nov 9, 2022

VS Code issue showing bad rendering when rendering the glyph from the font: microsoft/vscode#163000

We should support custom rendering of e0b8 through e0bf powerline symbols:

image

Code pointer:

/**
* This contains the definitions of the primarily used box drawing characters as vector shapes. The
* reason these characters are defined specially is to avoid common problems if a user's font has
* not been patched with powerline characters and also to get pixel perfect rendering as rendering
* issues can occur around AA/SPAA.
*
* The line variants draw beyond the cell and get clipped to ensure the end of the line is not visible.
*
* Original symbols defined in https://github.com/powerline/fontpatcher
*/
export const powerlineDefinitions: { [index: string]: IVectorShape } = {
// Right triangle solid
'\u{E0B0}': { d: 'M0,0 L1,.5 L0,1', type: VectorType.FILL, rightPadding: 2 },
// Right triangle line
'\u{E0B1}': { d: 'M-1,-.5 L1,.5 L-1,1.5', type: VectorType.STROKE, leftPadding: 1, rightPadding: 1 },
// Left triangle solid
'\u{E0B2}': { d: 'M1,0 L0,.5 L1,1', type: VectorType.FILL, leftPadding: 2 },
// Left triangle line
'\u{E0B3}': { d: 'M2,-.5 L0,.5 L2,1.5', type: VectorType.STROKE, leftPadding: 1, rightPadding: 1 },
// Right semi-circle solid,
'\u{E0B4}': { d: 'M0,0 L0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.FILL, rightPadding: 1 },
// Right semi-circle line,
'\u{E0B5}': { d: 'M0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.STROKE, rightPadding: 1 },
// Left semi-circle solid,
'\u{E0B6}': { d: 'M1,0 L1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.FILL, leftPadding: 1 },
// Left semi-circle line,
'\u{E0B7}': { d: 'M1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.STROKE, leftPadding: 1 }
};

@jerch
Copy link
Member

jerch commented Nov 10, 2022

Wow this is getting tedious, its like writing a custom font for xterm.js in the end. Btw there are more unicode parts, that will need babysitting like that - like the new teletext codepoints for 3x2 blocks.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 10, 2022

We're not covering everything though, just ones that are easy to draw in a canvas. The result is pretty great:

image

like the new teletext codepoints for 3x2 blocks.

You mean these ones? https://en.wikipedia.org/wiki/Teletext_character_set#Graphics_character_sets

I haven't heard about them before so I don't think we'd bother there. Powerline symbols however are used extensively in a terminal.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 10, 2022

Here's what it looks like when rendered using Cascadia Code PL:

image

Verified in microsoft/vscode#163000 (comment) to be 0xE0BE. We should check a few fonts to make sure they're all single width

@jerch
Copy link
Member

jerch commented Nov 10, 2022

You mean these ones? https://en.wikipedia.org/wiki/Teletext_character_set#Graphics_character_sets

Yep. They not commonly used yet, as they got specced pretty recently (like in current or prev unicode version).

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Dec 14, 2022
This also fixes an issue with padding making glyphs regress

Fixes xtermjs#4258
@Tyriar Tyriar added this to the 5.1.0 milestone Dec 14, 2022
@Tyriar
Copy link
Member Author

Tyriar commented Dec 14, 2022

With this I went with single width glyphs, it seems that the original powerline extra symbols used double width but a lot of fonts also use single.

@jerch
Copy link
Member

jerch commented Dec 15, 2022

@Tyriar Imho the powerline glyph and their correct proportions by fonts using them is just broken (or more diplomatic - underspecced). The fact that most fonts propagate them as single width looks wrong to me, and led to that ugly hack by several prompt scripts with additional SPs after those powerline chars.
I am not sure what the right fix here would be, from a clean spec viewpoint the SP hack is just wrong (in that sense all those prompt scripts are wrong). But they did it because of a lack of proper width spec for that codepoint area. So technically powerline itself is to be blamed for using an under specced codepoint area, and letting fonts/users get away with that sloppy SP hack.

AS I said in some other thread comment (idk where it was) - the PUA region is left unspecced on purpose, if someone wants to fill that gap (powerline does here), they have to provide a proper spec. xterm.js on the other hand would have to load that PUA spec and deal with these codepoints accordingly. Thats how it is intended from unicode side and respected by several other PUA overloads. The SP hack introduced by prompt scripts is wrong and should be banned.

Well it is as it is atm, but this hand-waving state with powerline glyphs will just drop on our feet again and again, until the whole "powerline comunity" will fix the spec and SP issue.

@Tyriar
Copy link
Member Author

Tyriar commented Dec 15, 2022

@jerch there have been very few complaints since we shipped this and prompts look great with the custom rendering, the only ones that come to mind are minor unintended regressions. Sure an actual spec would be great but I don't think it's that big of an issue atm. The worst that could happen is they can disable custom glyphs and go back to the terrible regular rendering, or request more fine grained tuning of the custom glyph setting.

@jerch
Copy link
Member

jerch commented Dec 15, 2022

@Tyriar Sure thing - as long as it works okish - "dont change a running system".

Its just sad, that specs get watered more and more intentionally by ignoring basic ideas. Also not the nerdfont guys are to be blamed here, they clearly state somewhere, that their glyphs are intended as wide (even their font patching generator sets them to wide by default). So basically all font maintainers delivering powerline things as half width override the default setting on purpose. Geez. 😪

@Tyriar
Copy link
Member Author

Tyriar commented Dec 15, 2022

Well both https://github.com/ryanoasis/powerline-extra-symbols and https://github.com/ryanoasis/nerd-fonts show wide, should we change it to wide? It's quite an easy change on our end, at least the rendering of them, changing the actual width of the character might cause some problems?

@jerch
Copy link
Member

jerch commented Dec 15, 2022

Really can't say which route is better one atm. If we switch to wide by default, we might get issues then from broken prompt scripts always emitting the SP hack. (Or even have to place some sanity code in the renderer to detect that condition and to skip the superfluous filler SP on purpose).
Idk, maybe lets just stay at the current quirky mode until it causes follow-up issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted type/enhancement Features or improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants