Enable unfocused acrylic

[Jaswir]

Summary of the Pull Request

Closes #7158

Enabling Acrylic as both an appearance setting (with all the plumbing), allowing it to be set differently in both focused and unfocused terminals. EnableUnfocusedAcrylic Global Setting that controls if unfocused acrylic is possible so that people can disable that behavior.

References and Relevant Issues

#7158 , references: #15913 , #11092

Detailed Description of the Pull Request / Additional comments

Allowing Acrylic to be set differently in both focused and unfocused terminals:

A

B

C

D

"profiles":
{
    "list":
    [
        {
            "commandline": "pwsh.exe",
            "name": "A",
            "unfocusedAppearance":
            {
                "useAcrylic": true,
            },
            "useAcrylic": true,
        },
        {
            "commandline": "pwsh.exe",
            "name": "B",
            "unfocusedAppearance":
            {
                "useAcrylic": false,
            },
            "useAcrylic": true,
        },
        {
            "commandline": "pwsh.exe",
            "name": "C",
            "unfocusedAppearance":
            {
                "useAcrylic": true,
            },
            "useAcrylic": false,
        },
        {
            "commandline": "pwsh.exe",
            "name": "D",
            "unfocusedAppearance":
            {
            },
            "useAcrylic": false,
        },
    ]
}

• A: AcrylicBlur always on
• B: Acrylic when focused, not acrylic when unfocused
• C: Why the hell not. Not Acrylic when focused, Acrylic when unfocused.
• D: Possible today by not using Acrylic.

EnableUnfocusedACrylic global setting that controls if unfocused acrylic is possible

So that people can disable that behavior:

Alternate approaches I considered:

Using _InitializeBackgroundBrush call instead of _changeBackgroundColor(bg) in ``TermControl::_UpdateAppearanceFromUIThread. Comments in this function mentioned:

// In the future, this might need to be changed to a
// _InitializeBackgroundBrush call instead, because we may need to
// switch from a solid color brush to an acrylic one.

I considered using this to tackle to problem, but don't see the benefit. The only time we need to update the brush is when the user changes the EnableUnfocusedAcrylic setting which is already covered by fire_and_forget TermControl::UpdateControlSettings

Supporting different Opacity in Focused and Unfocused Appearance???

This PR is split up in two parts #7158 covers allowing Acrylic to be set differently in both focused and unfocused terminals. And EnableUnfocusedAcrylic Global Setting that controls if unfocused acrylic is possible so that people can disable that behavior.

#11092 will be about enabling opacity as both an appearance setting, allowing it to be set differently in both focused and unfocused terminals.

Skipping the XAML for now:

“I actually think we may want to skip the XAML on this one for now. We've been having some discussions about compatibility settings, global settings, stuff like this, and it might be _more- confusing to have you do something here. We can always add it in post when we decide where to put it.”
-- Mike Griese

Validation Steps Performed

When Scrolling Mouse , opacity changes appropriately, on opacity 100 there are no gray lines or artefacts

When Adjusting Opacity through command palette, opacity changes appropriately, on opacity 100 there are no gray lines or artefacts

When opening command palette state goes to unfocused, the acrylic and color change appropriately

Stumbled upon a new bug when performing validation steps #15913

PR Checklist

• Closes Investigate: AcrylicBlur always on in an inactive window #7158
• Tests added/passed
• Documentation updated
  • If checked, please file a pull request on our docs repo and link it here: #xxx
• Schema updated (if necessary)

[Jaswir]

There were a lot of commits let me know if you want me to squash them

[lhecker]

There were a lot of commits let me know if you want me to squash them

All your commits will be squashed automatically when the pull request merges into main. 🙂

[DHowett]

This is very nice, thank you for doing it! We'll review it over the next few days. 😄

[lhecker]

The PR looks good to me, and I'd approve it, but I haven't understood yet why EnableUnfocusedAcrylic is necessary if we already have UseAcrylic for the unfocused appearance.

[zadjii-msft]

haven't understood yet why EnableUnfocusedAcrylic is necessary if we already have UseAcrylic for the unfocused appearance

Last time we brought this up in team sync (probably >1yr ago), there was discussion of having a global opt-out of unfocused acrylic to revert to the old behavior. Just as a potential compat setting. Or something that we could override with some sort of "battery saver mode" (ala #9600).

[Jaswir]

This is very nice, thank you for doing it! We'll review it over the next few days. 😄

Okay , thanks for the heads up!

@lhecker @zadjii-msft @DHowett
Let me know if something need to be changed.
I want to become a great Software Engineer. Would appreciate any tips/advice you can give.

[zadjii-msft]

FWIW, don't worry too much about our "battery saver" sidebar for now. That's something that needs a lot more noodling on before we start committing any code.

[Jaswir]

@zadjii-msft @lhecker
I notice that acrylic.BackgroundSource(Media::AcrylicBackgroundSource::Backdrop); is different from
acrylic.BackgroundSource(Media::AcrylicBackgroundSource::HostBackdrop);

Not sure if it's a problem, seems to be the default behaviour of Backdrop, there's a see through at the edge where it's somewhat transparent instead of acrylic.

and it seems a nit bit less bright, can you spot it? Both are at 0 Opacity btw

I can make it so that Backdrop brush is only used when Unfocused , would really miss the bright Acrylic otherwise :(
This is the result of that:

as compared to both having the Backdrop

I asked the users which one they like more.
@zadjii-msft Let me know how to proceed, I guess wait for what users like best and just make that?

[zadjii-msft]

Okay re-talked over the global setting thing with Dustin. Refreshed his memory on having one in the first place.

• Yea, sure it's partially redundant with just not putting useAcrylic in an unfocused appearance
• HostBackdrop acrylic is slightly different from Backdrop acrylic. That global compat setting does give folks a chance to opt back to the old appearance.
• We can always add an infobar in Profiles_Appearance in the future to let folks know "hey this won't work cause you have the global setting off"
• Maybe folks who have spicy pillows like me don't want to have acrylic on when the window is inactive, but want it always on if the window is active (but on even if the control isn't focused). The global setting is good for that case.
• It can later be futzed with after we refine batter saver settings

So, he's cool with it 👍

[Jaswir]

@zadjii-msft @lhecker
Awesome that it got merged! Am a bit disappointed though to be frank, had expected to get butchered more in the review process and get tons of constructive criticism and learn many things in the process and grow a lot. Like here: #15795

I would like that, why did that not happen? How can I get that? I guess PR's need to have more lines of code?
What are some issues that require lots of code to be solved?

@carlos-zamora I think battery saver mode would make it way higher quality for what it's worth.
I assume unfocused Acrylic will really drain the battery.

[Jaswir]

@lhecker

There was a bug in the PR we missed, here's a fix. #15944

[DHowett]

Am a bit disappointed though to be frank, had expected to get butchered more in the review process and get tons of constructive criticism and learn many things in the process and grow a lot. Like here: #15795

I would like that, why did that not happen? How can I get that? I guess PR's need to have more lines of code? What are some issues that require lots of code to be solved?

Not every code review is going to go the same way! As an example, James Holderness submits huge pull requests that get very few comments. It's because he is comfortable with our code, and we are comfortable with the way that he does his work. Now, not everybody has been working with the team for as long as he has--like tusharsnx (#15795). They are not familiar with our practices, but they ARE still submitting large changes.

The bottom line is:

If you submit small changes, they are less contentious. There is less stuff to talk about, or to be worried about. This is a good thing!

If you submit larger changes, there are more points for discussion. More code, more arguments, more areas. The more stuff you touch at once, the closer the team has to look to make sure it all fits together right.

Sometimes, it goes smoothly. You just do everything perfectly. Sometimes, it goes differently and you get a lot of comments. Maybe there's something you understand very well (and we won't challenge you!) or you've done a lot of research (and the PR goes smoothly)... and sometimes you need to be taught something about how the project fits together or how the language works, etc.

---

As you submit more and larger pull requests, you'll get more comments. Eventually, you will be so good at it that you will get less and less comments! 😄

[zadjii-msft]

Yea I couldn't have said anything more. If I had anything to add, it might be that we hashed out a lot of the edge cases in the side discussion in #2531 before you even got to the PR 😄 (this is a good thing)

[Jaswir]

@zadjii-msft @DHowett @lhecker Are there any relevant opportunities at Microsoft that would let me work on this project or similar ones to it?

[Jaswir]

@zadjii-msft @lhecker @DHowett

Checked it out, wanted to point out that
The doc/cascadia/profiles.schema.json changes don't appear to be updated.

https://[raw.githubusercontent.com/microsoft/terminal/release-1.18/doc/cascadia/profiles.schema.json
Doesn't have entree for enableunfocusedacrylic or the useacrylic for unfocused.