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

Do user research on AtlasEngine usage #13936

Closed
lhecker opened this issue Sep 6, 2022 · 23 comments
Closed

Do user research on AtlasEngine usage #13936

lhecker opened this issue Sep 6, 2022 · 23 comments
Assignees
Labels
Area-AtlasEngine Gathering-Data We'd like to gather telemetry to influence our decision Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@lhecker
Copy link
Member

lhecker commented Sep 6, 2022

We should know how many users are opting out of AtlasEngine and how often they run into known issues. This will tell us what improvements to priorize.

@lhecker lhecker self-assigned this Sep 6, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 6, 2022
@lhecker lhecker added the Issue-Task It's a feature request, but it doesn't really need a major design. label Sep 6, 2022
@237dmitry

This comment was marked as off-topic.

@lhecker

This comment was marked as off-topic.

@zadjii-msft zadjii-msft added Gathering-Data We'd like to gather telemetry to influence our decision Product-Terminal The new Windows Terminal. labels Sep 7, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 7, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.16 milestone Sep 7, 2022
@237dmitry

This comment was marked as off-topic.

@lhecker
Copy link
Member Author

lhecker commented Sep 7, 2022

Is there a later version?

We first develop features here on GitHub and then, every so often, we create a snapshot that we release in the App Store. In other words, the main branch on GitHub includes all the latest changes, whereas the App Store release might lag behind by up to 2 months.
The 1.15 series that you use for instance was forked off on the 2022-07-01. Since then development has continued on the main branch here on GitHub, which will end up being 1.16. After 1.16 has been forked off of main, the main branch will continue to receive all the latest changes again and then end up being 1.17 in the future. In short, the store releases always lag behind GitHub code changes by a bit.
1.16 will be released approximately next week.

ghost pushed a commit that referenced this issue Sep 8, 2022
With AtlasEngine being enabled by default in 1.16 Preview it would look weird
if the `useAtlasEngine` option would still be called "experimental".
Additionally we're interested in how many users opt out of useAtlasEngine,
indicating major issues that would require us to disable it by default again.

Related to #13936

## Validation Steps Performed
* Toggling `useAtlasEngine` works as expected ✅
* Observe new event with TVPP ✅
@zadjii-msft
Copy link
Member

#13939 was a great start, I'm gonna move this to 1.17 for any follow up ideas we have.

@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 8, 2022
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Sep 8, 2022
@dstanberry

This comment was marked as off-topic.

@zadjii-msft

This comment was marked as off-topic.

@nikkoaws
Copy link

What testing is being done on the new Atlas Engine?

First thing I noticed after upgrading was the font rendering looks awful, and a quick side by side comparison of the previous and new renderer confirms this.

@DHowett
Copy link
Member

DHowett commented Sep 17, 2022

What testing is being done on the new Atlas Engine?

We are rolling it out in Preview to get some user feedback. This came only after we moved several hundred internal folks over to the atlas engine as a test.

I'd love to know what font you're using, at what size, and how it looks awful! Would you mind sharing?

@BlackHoleFox
Copy link

BlackHoleFox commented Sep 18, 2022

This is the closest existing issue I found but feel free to point me somewhere else. I started Windows Terminal yesterday and noticed that everything looked much thicker. A quick before/after with AtlasEngine enabled and disabled shows that its making all the text much thicker, changing sizing, and also what looks like shifting the positioning for some characters.

AtlasEngine disabled:
render_old

render_letters_old

AtlasEngine enabled:
render_new

render_letters_new

@nikkoaws
Copy link

What testing is being done on the new Atlas Engine?

We are rolling it out in Preview to get some user feedback. This came only after we moved several hundred internal folks over to the atlas engine as a test.

I'd love to know what font you're using, at what size, and how it looks awful! Would you mind sharing?

Sure, below are some screen shots comparing the renderers, in particular the "a" and "e" highlight the weird rendering I am seeing.

The font is "Ubuntu Mono derivative Powerline", size: 14

new_renderer
old_renderer

@lhecker
Copy link
Member Author

lhecker commented Sep 19, 2022

@BlackHoleFox The old text renderer ("DxEngine") first computes the closest cell size using the given font size and then computes the actual font size based on the cell size. In your case the old engine has rounded down the cell size, resulting in an actual font size of 11.519999775pt. The new engine uses 12pt, just like you specified. Now that #14013 has merged you'll soon be able to specify 11.519999775 for the font size and it'll look exactly like it used to once again (apart from a slightly more compact line height):

atlas_engine_fractional_font_size


@nikkoaws Your problem is basically the exact same. In your case the old engine has rounded down the cell size as well, resulting in an actual font size of 13.5pt. I'd suggest using either 13pt in the (looks identical to 13.5pt but with smaller line height), or disabling AtlasEngine in the meantime. The fractional font size support will likely arrive in 1.16 fairly soon.

BTW I made the change to not round the font size anymore, because I felt like if a user specifies a font size they should get that exact font size. Additionally it fixes some fairly severe blurriness issues for some other fonts.

@DHowett
Copy link
Member

DHowett commented Sep 19, 2022

In your case the old engine has rounded down the cell size, resulting in an actual font size of 11.519999775pt. The new engine uses 12pt, just like you specified.

I'm a little bit worried about this. I don't think that we can (with straight faces!) tell users to go edit their settings files to set a font size of 11.519999775 if they liked the old text rendering. If this truly is 12 point text, and Atlas is rendering it properly as 12 points... that suggests that the font is quite terribly broken.

At the same time, many fonts come with hinting information for point sizes ~8 - 12 (at 96 dpi). Rendering them correctly may result in crisper lines and slight, but intended, changes in letter forms to better accommodate lower resolution displays. If "CaskaydiaCove" has removed this hinting information, it could very well explain why it looks worse at 12.

@lhecker
Copy link
Member Author

lhecker commented Sep 19, 2022

If this truly is 12 point text, and Atlas is rendering it properly as 12 points... that suggests that the font is quite terribly broken.

It's not that the font is broken, but rather that:

  • (Caskaydia Cove) Larger font sizes are mistaken for "heavier" text, as opposed to simply larger text.
    This is supported by the fact that every report we've gotten for this was on low DPI, 100% scale displays, where this issue is more pronounced (since there's less pixels to snap to).
  • (Ubuntu Mono) Due to DxEngine rounding a given 14pt to 13.5pt, it actually effectively rendered text at 13pt, which allowed hinting to kick in, while it won't do that at 14pt.

In short, I believe we'll simply have to tell users that they need to edit their settings file to get the old behavior back, because the existing DxEngine behavior is just untenable unfortunately. 😔
The previous behavior made font sizes behave inconsistent and unpredictable, and sometimes certain font features outright impossible to use. For instance the pure bitmap font "Terminus TTF" was practically impossible to use on my 150% scale display initially, because certain font sizes simply didn't map to the correct bitmap size at all using DxEngine.

@nikkoaws
Copy link

nikkoaws commented Sep 20, 2022

In short, I believe we'll simply have to tell users that they need to edit their settings file to get the old behavior back

@lhecker I'm not sure how viable it is, but ideally in the "Font size" section of settings it would be nice to place a warning that the current font size does not support hinting, and suggest the next nearest smaller and bigger font size that will render nicely.

@FWest98
Copy link
Contributor

FWest98 commented Sep 20, 2022

I have the same thing for me with any variant of the Cascadia font (not just the edited Nerd Font but also 'pure' Mono/Code). Indeed, the letters are (vertically) bigger in Atlas, and it causes the entire font to have a completely different feel. The strange thing is that the old feel matched very well with how the font feels in other tools like VS Code, Visual Studio 2022, Notepad, etc; while the new rendering feels very out-of-place compared to the other tools.

Do all tools do this font rendering wrong then?

@lhecker
Copy link
Member Author

lhecker commented Sep 21, 2022

The strange thing is that the old feel matched very well with how the font feels in other tools like VS Code, Visual Studio 2022, Notepad, etc; while the new rendering feels very out-of-place compared to the other tools.

If that's true, then I think we need to revert my change. While I think a given 12pt should be rendered as 12pt and not as "11.8938523pt" or something, I think that it's better to be consistent.

But... I can't speak of every editor you listed, but VS Code for instance configures its font size in CSS pixels (px = 96 DPI), whereas our setting uses CSS points (pt = 72 DPI). VS Code's default 14px font size maps to 10.5pt in Windows Terminal, which is something you can't actually configure. If it ever looked similar it was just by sheer coincidence and would break on any other display scale if you'd tried.

Edge and Chrome use the approximately same text rendering stack we use: DirectWrite/Direct2D, which is also used in most other Windows applications. But it's not the same text renderer as Visual Studio which uses WPF, which has its own text rendering algorithm. In either case, here's a comparison with Edge at 100% scale, 12pt Cascadia Code:

atlas_engine_edge

The only difference is in the glyph advance (= the spacing between the characters), which is because Edge/Chrome use a technique called "subpixel text positioning". If you're curious about it, here's a good overview that I read just earlier today.
This is something we should 100% also support in the future, but I don't think we're in a position to do that just yet. It makes performance optimizations harder and requires a number of refactorings around our cursor handling, etc.

But clearly the line height and x-height, as well as the overall look of the characters are identical. VS Code produces the exact same result as Edge, but with a larger/custom default line height of 1.4.

In other words, I at least can't reproduce any unexpected differences between us and VS Code, nor Chrome and it's derivatives, which makes me certain that we do at least something right...

DHowett pushed a commit that referenced this issue Sep 21, 2022
With AtlasEngine being enabled by default in 1.16 Preview it would look weird
if the `useAtlasEngine` option would still be called "experimental".
Additionally we're interested in how many users opt out of useAtlasEngine,
indicating major issues that would require us to disable it by default again.

Related to #13936

* Toggling `useAtlasEngine` works as expected ✅
* Observe new event with TVPP ✅

(cherry picked from commit 6816620)
@FWest98
Copy link
Contributor

FWest98 commented Sep 21, 2022

I took some time to collect some samples of the same content:
Font Renderings.zip

This includes old Windows CMD, Firefox, VS Code (and its terminal), and old/new WT behaviour. To me, the old WT and all other renderings all have in common that Cascadia Code appears as a "friendly" and "soft" font, with clear character separation. The old WT was already standing out a bit with a slightly "wider" and "softer" rendering.

To me, the Atlas renderer changes the impression of the font to a more "stern"/"serious" one. The character separation of e.g. the MM combinations is much less clear: in general the characters seem less well-separated. This too gives the impression of the font being "bolder", with higher weight. The glyph cell size stays constant, but the glyph seems to be "stretched" more with less margin - less than the other tools included in the zip. This is especially visible when you zoom in on the pixels of these characters in both renderers:
pixels

This might all be an unfortunate consequence of the specific font size ('12') and font and DPI (100%), but to me, it has quite a significant impact. More clever subpixel rendering might alleviate the issue a bit - from my sample WT is the only one without the clever subpixel rendering.

Another, more concrete thing is that glyphs are rendered differently between the two renderers, with a 'vertical-middle dot' being no longer in the middle as a noticeable issue (left-hand is Atlas). The size difference between glyphs in both versions is also big.
WT Glpyhs Comparison 2

@DHowett
Copy link
Member

DHowett commented Sep 21, 2022

There's definitely something to this. I direct-compared notepad and Terminal at the same font size (8pt at 150% scale, which should map to 12 at 100), and my initial thought was that we looked thicker because we were using grayscale AA instead of subpixel. On closer inspection, though, it looks like it's something else. Even with subpixel rendering, there's a difference in what I think is hinting.

this_was_so_frustratingly_difficult_to_make

@lhecker
Copy link
Member Author

lhecker commented Sep 21, 2022

@FWest98 Those Ms look really bad! I'm actually working on exactly that right now. An upcoming version of 1.16 should include two fixes in that regard which should add a 1px gap between those:
image

But even that isn't particularly perfect and I'm continuing to work on it.


I've joined up with @DHowett to look at our font rendering. He put Cascadia Code into Visual TrueType (VTT) to get the expected reference rendering and we basically match the green (correct) variant, whereas Notepad looks more like the red/incorrect one:
image

For instance look at this "u" character in AtlasEngine (12pt Cascadia Code):
image

Compared to DxEngine which looks crispier and is 1px less tall which makes it look more "rounded":
image

But plugging it into VTT shows that what AtlasEngine does is correct:
image

In case of Notepad I suspect the difference is due to it not calling IDWriteFontFace::GetRecommendedRenderingMode and always using either of the two DWRITE_MEASURING_MODE_GDI_* variants. In case of the older DxEngine in Windows Terminal I'm not entirely sure. But reviewing the existing code more closely already reveals that it doesn't call IDWriteFontFace::GetRecommendedRenderingMode either and doesn't provide Direct2D with the display DPI, which means it can't correctly infer the correct vertical anti-aliasing/hinting strength.

That said, I don't want to understate the severity of blurriness in our text renderer. Please don't take my above comments as "I don't want to improve the status quo". I'm going to try and improve here, but I currently, at the moment still want to follow the supposed "perfect" rendering of fonts as closely as possible, which includes not changing the given font size, nor using crispier GDI rendering.

@lhecker
Copy link
Member Author

lhecker commented Sep 21, 2022

Oh right I should mention that enabling ClearType on a low DPI (100% scale) monitor might improve the perceived text clarity. In my testing it removes a lot of the "fuzziness" for Cascadia Code especially for "dense" characters like "w" or "m". It's also enabled by default for VS Code and Notepad.

You can enable it for all profiles in the settings menu under Defaults > Advanced:
image

@lhecker
Copy link
Member Author

lhecker commented Apr 12, 2023

#14959 will allow anyone to arbitrarily override the cell size of the terminal. It's text rendering closely matches the latest version of Notepad too, which now doesn't appear to use GDI rendering anymore either. I would strongly recommend anyone using a 100% scale display to enable ClearType like mentioned above. With #14959 in mind I'll close the issue for now. Thank you for your feedback everyone! It helped me fix a lot of bugs. 🙂

@lhecker lhecker closed this as completed Apr 12, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 12, 2023
@FWest98
Copy link
Contributor

FWest98 commented May 25, 2023

The rewrite seems to have solved the issue for me! I had to tweak my settings a bit (font size 12->11 and line height 1.2->1.3), but the rendering is now (as far as I can see) identical to the old renderer!

Thanks for the hard work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Gathering-Data We'd like to gather telemetry to influence our decision Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

9 participants