-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Enable Vintage Opacity #11180
Enable Vintage Opacity #11180
Conversation
This reverts commit cb4f58d.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
21/29
src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h
Outdated
Show resolved
Hide resolved
@@ -84,7 +84,7 @@ namespace winrt::TerminalApp::implementation | |||
TraceLoggingBool(usedManualProfile, "ProfileSpecified", "Whether the new tab specified a profile explicitly"), | |||
TraceLoggingGuid(profile.Guid(), "ProfileGuid", "The GUID of the profile spawned in the new tab"), | |||
TraceLoggingBool(settings.DefaultSettings().UseAcrylic(), "UseAcrylic", "The acrylic preference from the settings"), | |||
TraceLoggingFloat64(settings.DefaultSettings().TintOpacity(), "TintOpacity", "Opacity preference from the settings"), | |||
TraceLoggingFloat64(settings.DefaultSettings().Opacity(), "TintOpacity", "Opacity preference from the settings"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"TintOpacity" -> "Opacity"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
@@ -685,9 +685,18 @@ SIZE NonClientIslandWindow::GetTotalNonClientExclusiveSize(UINT dpi) const noexc | |||
// - the HRESULT returned by DwmExtendFrameIntoClientArea. | |||
void NonClientIslandWindow::_UpdateFrameMargins() const noexcept | |||
{ | |||
MARGINS margins = {}; | |||
MARGINS margins = { 0, 0, 0, 0 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part didn't seem necessary 🤔
@@ -896,6 +905,10 @@ void NonClientIslandWindow::_SetIsBorderless(const bool borderlessEnabled) | |||
_titlebar.Visibility(_IsTitlebarVisible() ? Visibility::Visible : Visibility::Collapsed); | |||
} | |||
|
|||
// Update the margins when entering/leaving focus mode, so we can prevent | |||
// the titlebar from showing through transparent terminal controls | |||
_UpdateFrameMargins(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw the titlebar now appears behind all terminals on resize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not just fomo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea. Couldn't find a way to suppress that. Before the island would paint on top of where the win32 titlebar is, so it wouldn't be visible. Now the island is transparent, so there's nothing to obscure the titlebar.
We could maybe paint the region that's between the side of the window and the island with some color, then paint over it with BLACK_BRUSH
once the island resizes, but I'm not sure if that'll work, or if we get the right paint messages to enable that.
@@ -896,6 +905,10 @@ void NonClientIslandWindow::_SetIsBorderless(const bool borderlessEnabled) | |||
_titlebar.Visibility(_IsTitlebarVisible() ? Visibility::Visible : Visibility::Collapsed); | |||
} | |||
|
|||
// Update the margins when entering/leaving focus mode, so we can prevent | |||
// the titlebar from showing through transparent terminal controls | |||
_UpdateFrameMargins(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not just fomo
if (newOpacity >= 1.0) | ||
{ | ||
_settings.UseAcrylic(false); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're putting a lot of trust in the platform that it will "do the right thing" when acrylic is enabled and set to 1.0.
} | ||
else if (adjustment < 0) | ||
{ | ||
_settings.UseAcrylic(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I had to guess, this is part of what breaks the "go from solid to acrylic" wheel action on Windows 10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooof. Might need to add this back to fix #11285, to make sure that we have some acrylic on win10. Gross.
) This logic was seemingly redundant. There's two cases I'm looking at here: #### Case 1 ```jsonc "defaults": { "opacity": 35 }, "list": [ { "commandline": "cmd.exe", "name": "Command Prompt" }, ``` In this case, we wouldn't set the `TerminalSettings` Opacity to .35, we'd set it to 1.0, because the profile didn't have an `opactity`. #### Case 2 ```jsonc "defaults": { "useAcrylic": true }, "list": [ { "commandline": "cmd.exe", "name": "Command Prompt" }, ``` In this case we still want to have an acrylic effect. Previously, we'd default this effect to 50% opaque. I'm not sure that we can actually get that anymore. BUT it turns out, we _can_ have 100% opacity and HostBackdropAcrylic. It is very subtle, but is maybe something we should be allowing anyways. It kinda looks like: ![image](https://user-images.githubusercontent.com/18356694/135168469-35d1f55b-58d1-4ee3-a717-76000c2574b9.png) * [x] Fixes #11355 * [x] Regressed in #11180 * [x] I work here
Missed this in #11180. I forgot to init the BG opacity with the renderer on startup, because that matters when you have `"antialiasingMode": "cleartype",`. Repro json ```json { "commandline": "cmd.exe", "guid": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}", "hidden": false, "opacity": 35, "antialiasingMode": "cleartype", "padding": "0", "name": "Command Prompt" }, ``` * [x] Fixes #11315
In #11180 we made `opacity` independent from `useAcrylic`. We also changed the mouse wheel behavior to only change opacity, and not mess with acrylic. However, on Windows 10, vintage opacity doesn't work at all. So there, we still need to manually enable acrylic when the user requests opacity. * [x] Closes #11285
Missed this in #11180. I forgot to init the BG opacity with the renderer on startup, because that matters when you have `"antialiasingMode": "cleartype",`. Repro json ```json { "commandline": "cmd.exe", "guid": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}", "hidden": false, "opacity": 35, "antialiasingMode": "cleartype", "padding": "0", "name": "Command Prompt" }, ``` * [x] Fixes #11315
So to be clear there are currently no plans to backport this to windows 10? Or is it just waiting for XAML Islands? |
To be totally correct:
|
In #11180 we made `opacity` independent from `useAcrylic`. We also changed the mouse wheel behavior to only change opacity, and not mess with acrylic. However, on Windows 10, vintage opacity doesn't work at all. So there, we still need to manually enable acrylic when the user requests opacity. * [x] Closes #11285 SUI changes in action: ![auto-acrylic-win10](https://user-images.githubusercontent.com/18356694/136281935-db9a10f4-e0ad-4422-950b-0a01dc3e12c0.gif)
🎉 Handy links: |
This reverts #11372 and #11285, and brings #11180 to everyone, now that MSFT:37879806 has been serviced to everyone in [KB5011831](https://support.microsoft.com/en-gb/topic/april-25-2022-kb5011831-os-builds-19042-1682-19043-1682-and-19044-1682-preview-fe4ff411-d25a-4185-aabb-8bc66e9dbb6c)[1]. I tested this on my home Win10 laptop that's super old and doesn't have a functioning clock, but it does have that update at the very least. I don't think we had an issue tracking this? [1]: I'm pretty sure about this at least
This reverts #11372 and #11285, and brings #11180 to everyone, now that MSFT:37879806 has been serviced to everyone in [KB5011831](https://support.microsoft.com/en-gb/topic/april-25-2022-kb5011831-os-builds-19042-1682-19043-1682-and-19044-1682-preview-fe4ff411-d25a-4185-aabb-8bc66e9dbb6c)[1]. I tested this on my home Win10 laptop that's super old and doesn't have a functioning clock, but it does have that update at the very least. I don't think we had an issue tracking this? [1]: I'm pretty sure about this at least (cherry picked from commit a5c5b8a) Service-Card-Id: 87176746 Service-Version: 1.16
This reverts #11372 and #11285, and brings #11180 to everyone, now that MSFT:37879806 has been serviced to everyone in [KB5011831](https://support.microsoft.com/en-gb/topic/april-25-2022-kb5011831-os-builds-19042-1682-19043-1682-and-19044-1682-preview-fe4ff411-d25a-4185-aabb-8bc66e9dbb6c)[1]. I tested this on my home Win10 laptop that's super old and doesn't have a functioning clock, but it does have that update at the very least. I don't think we had an issue tracking this? [1]: I'm pretty sure about this at least (cherry picked from commit a5c5b8a) Service-Card-Id: 87176745 Service-Version: 1.15
Summary of the Pull Request
Adds support for vintage style opacity, on Windows 11+. The API we're using for this exists since the time immemorial, but there's a bug in XAML Islands that prevents it from working right until Windows 11 (which we're working on backporting).
Replaces the
acrylicOpacity
setting withopacity
, which is a uint between 0 and 100 (inclusive), default to 100.useAcrylic
now controls whether acrylic is used or not. Setting an opacity < 100 with"useAcrylic": false
will use vintage style opacity.Mouse wheeling adjusts opacity. Whether acrylic is used or not is dependent upon
useAcrylic
.opacity
will stealthily default to 50 ifuseAcrylic:true
is set.PR Checklist
Detailed Description of the Pull Request / Additional comments
Opacity was moved to AppearanceConfig. In the future, I have a mind to allow unfocused acrylic, so that'll be important then.
Validation Steps Performed
just look at it