Skip to content

Add support for cleartype text antialiasing#4711

Merged
4 commits merged intomasterfrom
dev/migrie/f/1298-AA-setting
Feb 25, 2020
Merged

Add support for cleartype text antialiasing#4711
4 commits merged intomasterfrom
dev/migrie/f/1298-AA-setting

Conversation

@zadjii-msft
Copy link
Member

Summary of the Pull Request

I needed to do something to keep sane so today I day of learned about antialiasing. This PR adds the ability to specify the "antialiasingMode" as a setting.

  • "antialiasingMode": "grayscale": the current behavior, D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE
  • "antialiasingMode": "cleartype": use D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE instead

PR Checklist

Detailed Description of the Pull Request / Additional comments

Grayscale:

image

Cleartype:
image

Side-by-side (can you tell which is which?)

image

@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Feb 24, 2020
@zadjii-msft zadjii-msft requested a review from miniksa February 24, 2020 17:03
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Straightforward. Most of it is just piping the enum all the way down to the renderer.

Considering that D2D1_TEXT_ANTIALIAS_MODE_ALIASED is basically the only one left, should we just yeet it into this PR too?

@msftbot make sure @miniksa signs off on this

@miniksa
Copy link
Member

miniksa commented Feb 25, 2020

Please do put aliased in as an option.

@DHowett-MSFT
Copy link
Contributor

Should this be the beginning of the font object? I am envisioning a future where we...

"font": {
    "family": "Cascadia Code",
    "weight": 400,
    "features": ["dlig", "ss01"]
}

but of course, maybe it chould also support a string ("font": "Cascadia Code")

@zadjii-msft
Copy link
Member Author

@DHowett-MSFT I'm gonna argue no, because I want this for 1.0 😄

That's a fine idea for 1.x/backlog IMO. If "font" is a string, use that. If it's a object, try parsing other bits out of it. That'll work well enough, but won't land this side of 1.0

@zadjii-msft
Copy link
Member Author

image

@zadjii-msft zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Feb 25, 2020
@ghost ghost requested review from DHowett-MSFT and leonMSFT February 25, 2020 14:20
},
"antialiasingMode": {
"default": "grayscale",
"description": "Controls how text is antialiased in the renderer. Possible values are \"grayscale\", \"cleartype\" and \"aliased\". Note that changing this setting will require starting a new terminal instance.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain the default maybe? Grayscale is faster than cleartype but doesn't look as jagged as aliased. It's a compromise.

// Routine Description:
// - Changes the antialiasing mode of the renderer. This must be called before
// _PrepareRenderTarget, otherwise the renderer will default to
// D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE. This has only been tested with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is no longer true that it has been tested with only those 😄 right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof okay fine

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Feb 25, 2020
@ghost
Copy link

ghost commented Feb 25, 2020

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@zadjii-msft
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost ghost merged commit 8a5407c into master Feb 25, 2020
@ghost ghost deleted the dev/migrie/f/1298-AA-setting branch February 25, 2020 22:20
ghost pushed a commit that referenced this pull request Apr 24, 2020
## Summary of the Pull Request

When we're on acrylic, we can't have cleartype text unfortunately. This PR changes the DX renderer to force cleartype runs of text that are on a non-opaque background to use grayscale AA instead.

## References

Here are some of the URLS I was referencing as writing this:

* https://stackoverflow.com/q/23587787
* https://docs.microsoft.com/en-us/windows/win32/direct2d/supported-pixel-formats-and-alpha-modes#cleartype-and-alpha-modes
* https://devblogs.microsoft.com/oldnewthing/20150129-00/?p=44803
* https://docs.microsoft.com/en-us/windows/win32/api/d2d1/ne-d2d1-d2d1_layer_options
* https://docs.microsoft.com/en-us/windows/win32/direct2d/direct2d-layers-overview#d2d1_layer_parameters1-and-d2d1_layer_options1
* https://docs.microsoft.com/en-us/windows/win32/api/dcommon/ne-dcommon-d2d1_alpha_mode?redirectedfrom=MSDN#cleartype-and-alpha-modes
* https://stackoverflow.com/a/26523006

Additionally:
* This was introduced in #4711 

## PR Checklist
* [x] Closes #5098
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

Basically, if you use cleartype on a light background, what you'll get today is the text foreground color _added_ to the background. This will make the text look basically invisible. 

So, what I did was use some trickery with `PushLayer` to basically create a layer where the text would be forced to render in grayscale AA. I only enable this layer pushing business when both:
  * The user has enabled cleartype text
  * The background opacity < 1.0

This plumbs some information through from the TermControl to the DX Renderer to make this smooth.

## Validation Steps Performed

Opened both cleartype and grayscale panes SxS, and messed with the opacity liberally.
This pull request was closed.
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 Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Antialiasing mode options

4 participants