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

Line height does not apply to legacy computing symbols #18498

Open
sphamba opened this issue Feb 3, 2025 · 4 comments
Open

Line height does not apply to legacy computing symbols #18498

sphamba opened this issue Feb 3, 2025 · 4 comments
Assignees
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@sphamba
Copy link

sphamba commented Feb 3, 2025

Windows Terminal version

1.21.3231.0

Windows build number

10.0.19045.0

Other Software

Steps to reproduce

Printing legacy computing symbols (using a font that supports them), such as ▔🮂🮃▀🮄🮅🮆█ █🮋🮊🮉▐🮈🮇▕ ▏🭰🭱🭲🭳🭳🭵▕ and increase the font line height (cellHeight setting in the JSON).

Expected Behavior

There should be no visible gaps between the characters, either vertically or horizontally, as it is for block elements:

1.1 line height
Image

1.5 line height
Image

Actual Behavior

Legacy computing symbols have noticeable vertical gaps when increasing the line height. There are also smaller horizontal gaps.

1.1 line height
Image

1.5 line height
Image

@sphamba sphamba 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 Feb 3, 2025
@DHowett
Copy link
Member

DHowett commented Feb 3, 2025

Thanks for the report!

@lhecker is there a compelling reason for us not to cover the Legacy Computing supplement with the built-in box drawing render?

@carlos-zamora carlos-zamora added the Needs-Attention The core contributors need to come back around and look at this ASAP. label Feb 5, 2025
@lhecker
Copy link
Member

lhecker commented Feb 10, 2025

Currently we don't intend for the line/cell height setting to affect the rendering of glyphs. It's strictly meant to modify the line height, like in a text editor.

It does apply to some "builtin" glyphs (the box drawing ones), because they're widely used, and they're supposed to be gapless, so we added support for that. The newer Legacy Computing Supplement is significantly less widely used and would be difficult to implement as builtin glyphs due to its complexity. It's not clear yet which of the new glyphs should be builtin and which ones don't need our support.

As such, I'd like to wait until more people want support for the Legacy Computing Supplement and/or until it's more widely used. Please let me know if you disagree with my take.

@lhecker lhecker added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Feb 10, 2025
@sphamba
Copy link
Author

sphamba commented Feb 10, 2025

I agree that Legacy Computing symbols are not widely used yet. Still, I was surprised to see that they were not displaying correctly on my side, even though the cascadia-code team seems to demonstrate the integration of the new symbols in their font using Windows Terminal, see cascadia-code#728 and blog post.

When you say that the line height applies to "builtin" glyphs, do you mean that these symbols, when drawn, do not rely on the current font's glyphs and are instead rendered using a different method?

In any case, I guess the sets of eights blocks, sextans, octans, and diagonals, would not be too complex to implement (rather, time-consuming). The same method as the one currently used for the symbols shown in the original post could be adopted (and I'd be happy to contribute if welcome). Other characters like the "large type pieces" are also supposed to be rendered gap-less, but if glyphs cannot be stretched, that would be more challenging...

Basically, I think there could be at least a partial support, potentially progressively expanding, starting with the sets that clearly represent blocks.
Looking forward to seeing your perspective on this!

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Feb 10, 2025
@lhecker
Copy link
Member

lhecker commented Feb 24, 2025

When you say that the line height applies to "builtin" glyphs, do you mean that these symbols, when drawn, do not rely on the current font's glyphs and are instead rendered using a different method?

Yes exactly. They're drawn using Direct2D via programmatic primitives. This allows them to perfectly fill all gaps, as this helps them look visually consistent when used for e.g. backgrounds or other, visually larger, arrangements.

Other characters like the "large type pieces" are also supposed to be rendered gap-less, but if glyphs cannot be stretched, that would be more challenging...

In my opinion the change in line height is just like changing the line height in e.g. Word. It results in a line height that does not match what the font was designed for, and so it will look incorrect. While builtin glyphs don't have this issue, I consider builtin glyphs a workaround, because many fonts don't contain well-designed box drawing glyphs to begin with.

Basically, I think there could be at least a partial support, potentially progressively expanding, starting with the sets that clearly represent blocks.

That said, I don't really disagree with you. Builtin glyphs will fundamentally always look better than those declared in a font, precisely because we can scale them to a perfect fit. However, our text renderer is in a shared codebase with conhost (a system component) and I don't want to put any more into the text renderer than necessary, to avoid increasing its binary size.

All of the above combined, makes me hesitant to add these glyphs to our set of builtin ones at this time:

  • They're not widely used yet
  • Fonts may end up having these as well-designed glyphs, making builtin ones unneccessary
  • My concern about the binary size

If you'd like to try it anyway, you can try integrating a subset into our rendering code, while ideally not increasing its size too much: https://github.com/microsoft/terminal/blob/main/src/renderer/atlas/BuiltinGlyphs.cpp

If you need directions for how to compile and work with the code, please let me know!

@lhecker lhecker added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. and removed 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 Needs-Attention The core contributors need to come back around and look at this ASAP. labels Feb 24, 2025
@lhecker lhecker added this to the Icebox ❄ milestone Feb 24, 2025
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 Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants