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

Canary: rendering issue #17810

Closed
german-one opened this issue Aug 27, 2024 · 6 comments · Fixed by #17826
Closed

Canary: rendering issue #17810

german-one opened this issue Aug 27, 2024 · 6 comments · Fixed by #17826
Assignees
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@german-one
Copy link
Contributor

Windows Terminal version

Canary v. 1.22.2351.0

Windows build number

10.0.22631.4037

Other Software

VS Code for reference

Steps to reproduce

The batch script used to generate the output is

@echo off
>nul chcp 65001
echo(
echo सिखाया
echo(
pause

... saved UTF-8 (no BOM) encoded.

Expected Behavior

Readable text 😉 Preferably rendered as in the screenshot of VS Code.

Actual Behavior

Before I say anything - the new width measuring of grapheme clusters in v.1.22 is far better than what we ever had before. I love how Terminal's Unicode support is getting better and better.

I'm still playing around to see how it behaves. Scripts like Devanagari did never work correctly. But even the rendering of this has been improved a lot in most cases now.
However, sometimes there are visual effects that I'd consider a regression since overlapping makes text almost unreadable in a usual font size (the font in every window of the screenshot below is Cascadia Mono btw.).

The screenshot shows:

  • top left: output in Canary v. 1.22.2351.0
  • bottom left: output in v. 1.20.11781.0
  • right: script code in VS Code v. 1.92.2
hindi

cc @lhecker

@german-one german-one added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 27, 2024
@lhecker lhecker self-assigned this Aug 27, 2024
@german-one
Copy link
Contributor Author

I'm afraid I can't really help you solve this problem, but if I can help you look up the code points in ucd.nounihan.grouped.xml, it might save you some time.

सिखाया (sikhaaya)

      <group age="1.1" JSN="" gc="Lo" ccc="0" dt="none" dm="#" nt="None" nv="NaN" bc="L" bpt="n" bpb="#" Bidi_M="N" bmg="" suc="#" slc="#" stc="#" uc="#" lc="#" tc="#" scf="#" cf="#" jt="U" jg="No_Joining_Group" ea="N" lb="AL" sc="Deva" scx="Deva" Dash="N" WSpace="N" Hyphen="N" QMark="N" Radical="N" Ideo="N" UIdeo="N" IDSB="N" IDST="N" hst="NA" DI="N" ODI="N" Alpha="Y" OAlpha="N" Upper="N" OUpper="N" Lower="N" OLower="N" Math="N" OMath="N" Hex="N" AHex="N" NChar="N" VS="N" Bidi_C="N" Join_C="N" Gr_Base="Y" Gr_Ext="N" OGr_Ext="N" Gr_Link="N" STerm="N" Ext="N" Term="N" Dia="N" Dep="N" IDS="Y" OIDS="N" XIDS="Y" IDC="Y" OIDC="N" XIDC="Y" SD="N" LOE="N" Pat_WS="N" Pat_Syn="N" GCB="XX" WB="LE" SB="LE" CE="N" Comp_Ex="N" NFC_QC="Y" NFD_QC="Y" NFKC_QC="Y" NFKD_QC="Y" XO_NFC="N" XO_NFD="N" XO_NFKC="N" XO_NFKD="N" FC_NFKC="#" CI="N" Cased="N" CWCF="N" CWCM="N" CWKCF="N" CWL="N" CWT="N" CWU="N" NFKC_CF="#" InSC="Consonant" InPC="NA" PCM="N" vo="R" RI="N" blk="Devanagari" isc="" na1="" Emoji="N" EPres="N" EMod="N" EBase="N" EComp="N" ExtPict="N" NFKC_SCF="#" ID_Compat_Math_Start="N" ID_Compat_Math_Continue="N" IDSU="N" InCB="None">

0938 स
         <char cp="0938" na="DEVANAGARI LETTER SA" InCB="Consonant"/>
093F ि
         <char cp="093F" na="DEVANAGARI VOWEL SIGN I" gc="Mc" lb="CM" OAlpha="Y" IDS="N" XIDS="N" GCB="SM" WB="Extend" SB="EX" InSC="Vowel_Dependent" InPC="Left"/>
0916 ख
         <char cp="0916" na="DEVANAGARI LETTER KHA" InCB="Consonant"/>
093E ा
         <char cp="093E" na="DEVANAGARI VOWEL SIGN AA" gc="Mc" lb="CM" OAlpha="Y" IDS="N" XIDS="N" GCB="SM" WB="Extend" SB="EX" InSC="Vowel_Dependent" InPC="Right"/>
092F य
         <char cp="092F" na="DEVANAGARI LETTER YA" InCB="Consonant"/>
093E ा
         <char cp="093E" na="DEVANAGARI VOWEL SIGN AA" gc="Mc" lb="CM" OAlpha="Y" IDS="N" XIDS="N" GCB="SM" WB="Extend" SB="EX" InSC="Vowel_Dependent" InPC="Right"/>

@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 28, 2024
@zadjii-msft zadjii-msft added this to the Terminal v1.23 milestone Aug 28, 2024
@lhecker
Copy link
Member

lhecker commented Aug 29, 2024

Looking at the list of other Spacing Marks it seems to me like we should assign them a non-zero width. The "spacing" means after all that they occupy their own space, unlike non-spacing marks. 🤔

@lhecker
Copy link
Member

lhecker commented Aug 29, 2024

Yeah, that looks a lot better, doesn't it?

image

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Aug 29, 2024
@german-one
Copy link
Contributor Author

german-one commented Aug 29, 2024

Way better indeed!
Ideally the horizontal bars should fill the entire width of a cell. However, that's rather nit picking and I'm not even sure if this is a font issue. Readability is for sure far more important.
Curious - what about the width of the whole word now. If you surround it with quotes it looks like that in v.1.21:
quoted
I don't expect that this extra spacing at the end is back again, but it might be worth testing.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 29, 2024
@lhecker
Copy link
Member

lhecker commented Aug 29, 2024

The gaps are mostly a layout issue. Since we rely on font fallback, the given fallback fonts may simply have a different idea of what the advance width of the glyphs are. In fact, usually the fallback fonts are proportional while we expect monospace rendering. I think that's simply an issue we'll have to live with until someone designs a monospace font for that language.

The extra gap at the end (near the quote) is the result from my text layout in AtlasEngine. Centering glyphs in their cell isn't quite as trivial as it may initially seem, because ligatures can seemingly form across arbitrary columns (after shaping). It's way way simpler to implement this by left-aligning all glyphs. We're tracking that issue here: #16656

@german-one
Copy link
Contributor Author

Thanks for the further explanation! It confirms what I thought how it works.
Turns out that "सिखाया" was no good choice for the gap at the end that I had in mind. "हिन्दी" would have been better. Your update already flew with the latest Canary and works as expected:
new

Vs. the old width measuring before 1.22:
old

Excellent work, thanks again for fixing that on such short notice!

DHowett pushed a commit that referenced this issue Sep 4, 2024
Spacing marks are called so, because they have a positive advance
width, unlike their non-spacing neighbors (as the name indicates).
After this we stop assigning such gc=Mc codepoints a zero width.

Closes #17810

(cherry picked from commit 0cb3426)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgSg1L4
Service-Version: 1.22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants