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

DOM clipping some emoji characters #4813

Open
theflyingape opened this issue Sep 19, 2023 · 5 comments
Open

DOM clipping some emoji characters #4813

theflyingape opened this issue Sep 19, 2023 · 5 comments

Comments

@theflyingape
Copy link
Contributor

Upfront, I had to switch back to DOM renderer after this minor dot update to 5.3.0, because it introduced a weird WebGl context error similar to what was reported in #4804 -- is WebGL going away in v6 from #4779 ?

Back to this issue, I noticed some emoji characters under DOM do not fill both spaces, but instead clip. I continue to have the unicode11 add-on with term.unicode.activeVersion = '11' and clipping did not occur under WebGL.

image
Note Class, HP, Weapon,, and Armor emoji are clipped but all of the others are not.

@jerch
Copy link
Member

jerch commented Sep 20, 2023

Hmm yeah, thats a downside of the new char handling in the DOM renderer - it highly depends on correct wcwidth values. While the canvas and the webgl renderers have special handling for allowing chars to "overprint" to the right, thats not so easy anymore with the letter-spacing cell metrics in the DOM renderer.
There might be a workaround possible though, if we isolate much wider chars than the wcwidth value indicates, and place them with a sized span and proper overflow rules. Not sure yet if thats applicable, as it might screw up non monospace fonts.

Ideally the newer unicode addon has the right wcwidth values for those codepoints. You can also check what unicode 15 has to say about them (look for east asian width property).

@mofux
Copy link
Contributor

mofux commented Sep 20, 2023

I believe that this problem has always existed with the DOM renderer, and it is not easy to fix without having regressions in performance. The character is not clipped (as it is allowed to overflow), but the cell (span) after the emoji has a background color set, which draws over the overflowing emoji, making it appear as clipped.

This does not happen if there is no background color set as demonstrated here:

image

The only proper fix I can think of is to draw the background and the characters in separate layers, but that would introduce more DOM elements which will decrease the rendering performance.

@jerch
Copy link
Member

jerch commented Sep 20, 2023

@mofux Youre right, I forgot about the BG issue. I also think that separately drawing the BG is not viable for the DOM renderer.

We prolly have the same issue with some powerline glyphs as well. Some use PUA in unicode (private use area), its codepoints are undefined by default, and to be overloaded by application specific usage. A common fix on nerdfont/powerline side is to add SP behind the faulty glyph to "give room" to the right. We could do likewise in the DOM renderer by a forward lookup and spacing it 2 cells, if a whitespace with the same BG was found. This is still wonky, as it might break the SP from the glyph during resizing (well its only a work around).

A better fix for the PUA issue would be to establish a sequence allowing to define wcwidths for codepoints from app side, raw outline:

query:
OSC <some_free_id> ; <codepoint> ; <start> : <end(excl)>; ... ST
response & set:
OSC <some_free_id> ; <codepoint> # <wcwidth> ; <start> : <end(excl)> # 1121... ; ... ST

<start> : <end(excl)> is a shortcut for <codepoint1> ; <codepoint2> ; ... ; <codepointN> to make range requests easier. Its response is also aggregated into a sequence of digits.

Edit: Ultimately the <codepoint> part could be expanded to support grapheme clusters, this way we can deal with any unicode width ambiguity in the future. I am not sure yet about the correct repr for <codepoint>, maybe this should be a hex string to avoid issues with UTF-8/unicode in OSC payload.

The usage scenario for such a sequence is pretty straight forward - any app that places ambiguous codepoints, can either query the render widths and adjust its own buffer repr based on that, or forces the TE to render the intended width by an explicit set call.
A RIS and prolly DECSTR as well should reset any overloaded wcwidths.

Created #4817 for the unicode issues. We have several issues with unicode, imho its a good idea to collect and address them somehow.

@mofux
Copy link
Contributor

mofux commented Sep 20, 2023

@jerch After giving it another thought, drawing the background separately might not be as bad as one might think first. Continuous cells with different backgrounds are being split into multiple spans anyway, so we might as well draw a separate background span for them, and then the text could stay in a single span. I created a little experiment in #4818 that I'd like to iterate on. This also gives us the display: inline for character spans which might even boost performance.

@jerch
Copy link
Member

jerch commented Sep 20, 2023

Hmm right, lets see how this turns out. Indeed the inline-displaying will give rather big perf bonus, as I already tested during the renderer rewrite. I did not yet address it there, as it would have created tons of follow-up issues. Maybe your idea in #4818 can fix this alltogether.

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

No branches or pull requests

3 participants