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

Some fonts' italic text has wavy baselines #13987

Closed
indika-dev opened this issue Sep 14, 2022 · 22 comments · Fixed by #14039
Closed

Some fonts' italic text has wavy baselines #13987

indika-dev opened this issue Sep 14, 2022 · 22 comments · Fixed by #14039
Labels
Area-AtlasEngine Area-Fonts Related to the font Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@indika-dev
Copy link

Windows Terminal version

1.16.2523.0

Windows build number

10.0.19043.0

Other Software

neovim 0.7.2 (WSL2)
FiraCode NF, 12pt as Terminal Font
colorscheme github_dark

Steps to reproduce

Open a java source code file and comments are wavy, non-comment code lines are as expected
WavyCommentLines

Expected Behavior

Open a java source code file and comments look the same way as other code lines

Actual Behavior

comment lines are wavy

@indika-dev indika-dev added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Sep 14, 2022
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 14, 2022
@zadjii-msft
Copy link
Member

Looks like the text is just formatted as italic by neovim. I'm sure whatever neovim config you're using has options for configuring how different elements are rendered - you can always disable it there.

I'm sure there's a way to use font features to disable italics, though, I don't know it off the top of my head.

@zadjii-msft zadjii-msft added Issue-Question For questions or discussion Area-Fonts Related to the font Resolution-Answered Related to questions that have been answered Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. and removed Issue-Bug It either shouldn't be doing this or needs an investigation. labels Sep 14, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 14, 2022
@indika-dev
Copy link
Author

Hi @zadjii-msft ,

that's right, that the comments are formatted in italics, that's defined in the colorscheme. Good point with the configuration, I will try that.
But this only appears in Windows Terminal. With wezterm, the same code is rendered as it should be. Therefore, I think, this is still a bug.
NoWavyCommentLines

@zadjii-msft
Copy link
Member

@indika-dev Does wezterm support italics as emitted by client apps? @wez for some help. Maybe there's something in the default config that disables them for wezterm. (sorry for not being more familiar with wezterm. I probably should be.)

@indika-dev
Copy link
Author

Sorry, but I don't know, what you mean with "...italics as emitted by client apps". What I found in the documentation is this. But I didn't configure this.
neovim itself does not configure fonts. The terminal renders the "output" of neovim.

@zadjii-msft
Copy link
Member

I mean along the lines of #5461. Apps like vim can tell the Terminal "I want this text to be italicized" in the same way they tell the Terminal "I want this text to be red".

@wez
Copy link

wez commented Sep 14, 2022

Yeah, wezterm supports italics (CSI 3 m to enable, CSI 23 m to disable).

My read of this issue is that the problem is that the italics have a wavy baseline in the image in the OP, which suggests to me an issue with how the glyphs are rasterized/hinted, rather than an issue with understanding whether italics should be enabled.

If someone reported this to me in wezterm, I'd be curious about whether it was font specific: wezterm uses a feature of freetype to synthesize italics (by skewing the glyph) when no italic variant of the font is found, and that can behave differently from "normal" font rendering, and I wonder if there are parallels to that in the MS font shaper/rasterizer stack used in MS terminal?

@zadjii-msft
Copy link
Member

My read of this issue is that the problem is that the italics have a wavy baseline in the image in the

goodness gracious, thanks for that. I clearly didn't have enough coffee this morning and just figured OP meant "italic" when they said "wavy". Yea, the baselines do look a little wavy, don't they. Huh.

@indika-dev can you give us your whole settings.json file/? The exact set of settings should help with a repro here.

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 14, 2022
@zadjii-msft zadjii-msft added Priority-3 A description (P3) and removed Issue-Question For questions or discussion Resolution-Answered Related to questions that have been answered labels Sep 14, 2022
@wez
Copy link

wez commented Sep 14, 2022

I clearly didn't have enough coffee this morning

It's still pretty early!

@indika-dev
Copy link
Author

I took a look at #5461 and Nerd Fonts were mentioned. So, I tested a bit and it looks like, that the wavy lines only appear under certain Nerd Fonts:

  • FiraCode NF, CascadiaCove PL and JetBrainsMono NF have wavy lines
  • SauceCodePro NF and BlexMono Nerd Font have no wavy lines

@ghost ghost 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 Sep 14, 2022
@indika-dev
Copy link
Author

@indika-dev can you give us your whole settings.json file/?

Sure, here it is.
settings.zip

@zadjii-msft zadjii-msft changed the title comment lines are somewhat wavy Some fonts' italic text has wavy baselines Sep 14, 2022
@DHowett
Copy link
Member

DHowett commented Sep 14, 2022

Hey @lhecker, is this related to your glyph scaler?

@lhecker
Copy link
Member

lhecker commented Sep 14, 2022

It sure is. FiraCode without adjusting glyph position:
image

With adjustments:
image

Thoughts:

  • Scaling should be relative to the baseline and not to the center of the cell and would probably fix this issue. (But is this correct?)
  • Currently talking internally with DirectWrite wizards why we're getting weird glyph metrics for italic Fira Code (edit: turns out that IDWriteTextLayout::GetOverhangMetrics returns incorrect/overzealous metrics for simulated italics)
    Fun fact: Fira Code has no italic variant. It's automatically generated by DirectWrite and that's the primary source of these issues you're seeing.

@zadjii-msft zadjii-msft removed the Area-Settings Issues related to settings and customizability, for console or terminal label Sep 14, 2022
@zadjii-msft zadjii-msft added Area-AtlasEngine and removed 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 Sep 14, 2022
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Sep 14, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.17 milestone Sep 14, 2022
@zadjii-msft zadjii-msft added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Sep 14, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 14, 2022
@birtles
Copy link

birtles commented Sep 15, 2022

I see the same with Caskaydia Cove Nerd Font from https://www.nerdfonts.com/font-downloads (also using Neovim)

image

It appeared fine before Terminal was updated to 1.16 this morning.

@lhecker
Copy link
Member

lhecker commented Sep 15, 2022

@birtles That's because that font, just like FiraCode, doesn't ship with a proper italic variant. When DirectWrite "simulates" those italic glyphs, it doesn't correctly compute the so called "overhangs" (= how much a glyph is outside of the bounding box / cell) and this confuses the new text renderer.

The original Cascadia Code ships with an actual italic variant which looks quite different (easily visible on the "f" character) and works correctly (because it's not simulated):
image

I'm working on a fix, but I'm not 100% sure how to best approach it yet.

@ghost ghost added the In-PR This issue has a related PR label Sep 19, 2022
@ghost ghost closed this as completed in #14039 Sep 21, 2022
ghost pushed a commit that referenced this issue Sep 21, 2022
This commit changes the glyph scale algorithm to prefer aligning glyphs to
their baseline. This improves the visual appearance of simulated italic glyphs.
However wide Emojis in narrow cells now look slightly worse without centering.

Closes #13987

## Validation Steps Performed
* Use FiraCode which has no italic variant and instead uses simulated italics
* Write italic text
* Baseline is consistent ✅
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Sep 21, 2022
DHowett pushed a commit that referenced this issue Sep 21, 2022
This commit changes the glyph scale algorithm to prefer aligning glyphs to
their baseline. This improves the visual appearance of simulated italic glyphs.
However wide Emojis in narrow cells now look slightly worse without centering.

Closes #13987

## Validation Steps Performed
* Use FiraCode which has no italic variant and instead uses simulated italics
* Write italic text
* Baseline is consistent ✅

(cherry picked from commit 97dc5c8)
Service-Card-Id: 85767343
Service-Version: 1.16
@lhecker
Copy link
Member

lhecker commented Sep 21, 2022

FYI The fix that will be published is still incomplete unfortunately. It'll still cause italics to be downsized and cause an uneven appearance, even if it's slightly less bad now:
atlas_engine_baseline

I'm working on a permanent and proper fix in the meantime, but it'll most likely not be part of 1.16 unfortunately.

@ghost
Copy link

ghost commented Sep 23, 2022

🎉This issue was addressed in #14039, which has now been successfully released as Windows Terminal Preview v1.16.2641.0.:tada:

Handy links:

@summelon
Copy link

I'm sorry to reuse this closed thread.
I meet a similar text issue on preview version 1.17.1023.
There seems to be a misalignment in the italic text similar to wavy baselines.
My font is FiraCode NF Retina.
The screenshot is taken from an ubuntu server accessed by using MS Terminal.
Could you have a look at this issue? @lhecker
Many thanks!
image

@cscherrNT
Copy link

cscherrNT commented Sep 7, 2023

I still have weird italics scaling.

grafik

echo "\033[3m  % TlDr; Problem -> Issue -> Branch -> Fix in branch -> Master in Branch mergen für Backport -> Mergerequest \033[0m"

grafik

@lhecker
Copy link
Member

lhecker commented Sep 7, 2023

As mentioned above (#13987 (comment)) this issue was closed with a hotfix, that didn't address the entire problem. The proper, thorough fix is #14959 which has first shipped in v1.18.1421.0.

Could you try out the latest Windows Terminal Preview (v1.18.1462.0) and see if the issue still occurs for you?

@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Sep 7, 2023
@cscherrNT
Copy link

cscherrNT commented Sep 8, 2023

I'm using the latest official version shipped in the windows store (I think? There is no update button available): 1.16.11461.0. I'm on a work machine, so I'd rather not install a preview version and keep to stable software. Are the italic scaling issues only fixed for the preview version, not the stable release?

@lhecker
Copy link
Member

lhecker commented Sep 8, 2023

It's fixed starting v1.18.1421.0, which at the time of writing is only available as Preview.

FYI That Preview in particular version was released over 3 months ago. It's unlikely you'll find major bugs in it. You can use it to bridge the gap until 1.18 is the stable version (= within a month). Actual development versions of Windows Terminal are not released in the Windows Store, but rather tested by the team for a while before being released.

@DHowett
Copy link
Member

DHowett commented Sep 8, 2023

I'm using the latest official version shipped in the windows store (I think? There is no update button available): 1.16.11461.0.

That's strange! The latest version available in the store for the Stable channel is 1.17

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Area-Fonts Related to the font Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants