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.

Sign up for GitHub

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

Jump to bottom

Adjust Opacity to 100% with Color Scheme in Unfocused Appearance that has colour other than black gives black outline #15913

Open
Jaswir opened this issue Aug 31, 2023 · 5 comments
Open

Adjust Opacity to 100% with Color Scheme in Unfocused Appearance that has colour other than black gives black outline #15913

Jaswir opened this issue Aug 31, 2023 · 5 comments
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Milestone
Backlog

Comments

@Jaswir
Copy link
Contributor

Jaswir commented Aug 31, 2023 •
edited
Loading

Windows Terminal version

1.18.1462.0

Windows build number

10.0.22621.0

Other Software

No response

Steps to reproduce

image
new_bug_discovered

Expected Behavior

image

Actual Behavior

actual behaviour
@Jaswir Jaswir 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 Aug 31, 2023
@Jaswir
Copy link
Contributor Author

Jaswir commented Sep 1, 2023 •
edited
Loading

Clue: adding retroTerminalEffect to unfocusedAppearance gives a different behaviour

image

@Jaswir Jaswir mentioned this issue Sep 2, 2023
Merged
4 tasks
@carlos-zamora carlos-zamora added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 6, 2023
@carlos-zamora carlos-zamora added this to the Backlog milestone Sep 6, 2023
@carlos-zamora carlos-zamora added the Help Wanted We encourage anyone to jump in on these. label Sep 6, 2023
@carlos-zamora
Copy link
Member

Thanks for finding this! If you're interested in working on it, please be our guest. 😊

zadjii-msft added a commit that referenced this issue Sep 7, 2023
@Jaswir @zadjii-msft
Enable unfocused acrylic (#15923)
2f7f759 
## 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

![A](https://github.com/microsoft/terminal/assets/15957528/c43965f2-f458-46ae-af1c-a2107dac981a)

#### B

![B](https://github.com/microsoft/terminal/assets/15957528/e84ef1a8-8f4c-467a-99c2-9427061b3e3e)

#### C

![C](https://github.com/microsoft/terminal/assets/15957528/63fd35ba-a55a-4c1a-8b1c-5b571aa902ed)

#### D

![D](https://github.com/microsoft/terminal/assets/15957528/05108166-1c6e-406e-aec0-80023fc3f57c)

``` json
"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:

![256926990-3c42d99a-67de-4145-bf41-ce3995035136](https://github.com/microsoft/terminal/assets/15957528/eef62c14-d2bd-4737-b86e-dcb3588eb8f7)

### Alternate approaches I considered: 
Using `_InitializeBackgroundBrush` call instead of
`_changeBackgroundColor(bg) in
``TermControl::_UpdateAppearanceFromUIThread`. Comments in this function
mentioned:

``` *.cs'
// 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

![edgecase_scrollwheel](https://github.com/microsoft/terminal/assets/15957528/29a1b11e-05b8-4626-abd2-4f084ae94a8d)


![image](https://github.com/microsoft/terminal/assets/15957528/c05ea435-8867-4804-bcdc-2074df08cec1)

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

![edgecase_adjustopacity](https://github.com/microsoft/terminal/assets/15957528/a59b4d6d-f12e-48da-96bb-3eb333ac4637)


![image](https://github.com/microsoft/terminal/assets/15957528/c05ea435-8867-4804-bcdc-2074df08cec1)

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

![edge_case_command_palette](https://github.com/microsoft/terminal/assets/15957528/ec0cd8b5-676e-4235-8231-a10a5689c0b8)


![image](https://github.com/microsoft/terminal/assets/15957528/4300df70-f64b-4001-8731-b3b69471ea78)

#### Stumbled upon a new bug when performing validation steps #15913

![264637964-494d4417-6a35-450a-89f7-52085ef9b546](https://github.com/microsoft/terminal/assets/15957528/fee59c4a-400b-4e40-912b-ea8c638fc979)

## PR Checklist

- [x] Closes #7158
- [X] Tests added/passed
- [X] Documentation updated
- If checked, please file a pull request on [our docs
repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
- [x] Schema updated (if necessary)

---------

Co-authored-by: Mike Griese <migrie@microsoft.com>
@Jaswir
Copy link
Contributor Author

Jaswir commented Oct 14, 2023 •
edited
Loading

@lhecker

Hi Leonard,

Are you reading this?

I would feel uncomfortable talking in the chat of 190 or 2664 cause lots of people. Feel more comfortable talking here with you.

I read you're the expertise person when it comes to DXEngine, AtlasEngine

In all honesty, the renderer code is probably not my area of expertise. The best I could likely do is point you at DxEngine and AtlasEngine, which are the two different text renderers in the Terminal.

@lhecker might be able to point you to a more specific method to look at.

-- @zadjii-msft

I was wondering if 190 and 2664 are both related to DXEngine, AtlasEngine?

@Jaswir
Copy link
Contributor Author

Jaswir commented Oct 14, 2023

@zadjii-msft

What is your expertise then?

@zadjii-msft
Copy link
Member

My expertise at this point is pretty much the whole project, save for the renderer code, and the UIA code. Pretty much everything else I've had my paws on. The VT code has certainly evolved dramatically since I first owned that, but it's a lot of the same shape more or less. I've at various points owned:

  • The VT parser
  • Console settings
  • ConPTY
  • the whole Windows Terminal architecture, from the core and connection layers up to the control, the settings model, the app layer, the windowing, the process model.

Rendering I've mostly kept my head out of, except a couple quick forays to help here and there. I'm honestly more familiar with OpenGL than I am directx 😄 There's a lot to mentally cache in for working with the renderer, so I try to leave that cached out of my brain most of the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@carlos-zamora @Jaswir @zadjii-msft