Skip to content

feat: support Mica/Acrylic on Windows - #38163

Merged
jkleinsc merged 2 commits into
mainfrom
windows-backdrop
May 15, 2023
Merged

feat: support Mica/Acrylic on Windows#38163
jkleinsc merged 2 commits into
mainfrom
windows-backdrop

Conversation

@codebytere

@codebytere codebytere commented May 3, 2023

Copy link
Copy Markdown
Member

Description of Change

Supersedes #30298.

Closes #16391.
Closes #29937.

Adds support for background material effects on Windows with win.setBackgroundMaterial(material).

Checklist

Release Notes

Notes: Add support for Mica and Acrylic background effects on Windows.

@codebytere codebytere added semver/minor backwards-compatible functionality target/24-x-y PR should also be added to the "24-x-y" branch. target/25-x-y PR should also be added to the "25-x-y" branch. labels May 3, 2023
@codebytere
codebytere requested review from a team, deepak1556 and mlaurencin May 3, 2023 10:49
@felixrieseberg

Copy link
Copy Markdown
Member

I can't believe you did this in like 30 LoC, this is so much simpler than I thought it would have been.

Comment thread shell/browser/native_window.cc

@deepak1556 deepak1556 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How does this feature work together with our custom titlebar implementation where we basically extend into the non-client area ?

#if BUILDFLAG(IS_WIN)
// DWMWA_USE_HOSTBACKDROPBRUSH is only supported on Windows 11 22H2 and up.
if (base::win::GetVersion() < base::win::Version::WIN11_22H2)
return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this surface an error to the user rather than silent termination ?

@codebytere codebytere May 4, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't for comparable version-gated features like this - historically it's just been clearly noted in docs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think silently failing is fine here

DwmSetWindowAttribute(GetAcceleratedWidget(), DWMWA_SYSTEMBACKDROP_TYPE,
&backdrop_type, sizeof(backdrop_type));
if (FAILED(result))
LOG(WARNING) << "Failed to set background material to " << material;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

User facing error instead of LOG ?

@codebytere codebytere May 4, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm also open to that, but we don't have a clear convention there either really 🤔 for dark mode for example we don't surface a user facing error:

TrySetWindowTheme(hWnd, dark);

Beyond that - we don't really get a good error message from the call sadly, so all we'd really be able to say is "it failed"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Because we set this in the constructor we can't safely have a user facing error afaik anyway. I think the LOG is fine here. We shouldn't fail to construct a window just because the DWM call got lost

Comment thread shell/browser/api/electron_api_base_window.cc Outdated
@codebytere
codebytere requested a review from deepak1556 May 8, 2023 07:47

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

API LGTM

Comment thread shell/browser/api/electron_api_base_window.h Outdated
Comment thread docs/api/browser-window.md

@samuelmaddock samuelmaddock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

API LGTM

@jkleinsc
jkleinsc merged commit e19500f into main May 15, 2023
@jkleinsc
jkleinsc deleted the windows-backdrop branch May 15, 2023 20:31
@release-clerk

release-clerk Bot commented May 15, 2023

Copy link
Copy Markdown

Release Notes Persisted

Add support for Mica and Acrylic background effects on Windows.

@trop

trop Bot commented May 15, 2023

Copy link
Copy Markdown
Contributor

I was unable to backport this PR to "24-x-y" cleanly;
you will need to perform this backport manually.

@trop trop Bot removed the target/24-x-y PR should also be added to the "24-x-y" branch. label May 15, 2023
@trop

trop Bot commented May 15, 2023

Copy link
Copy Markdown
Contributor

I was unable to backport this PR to "25-x-y" cleanly;
you will need to perform this backport manually.

@trop

trop Bot commented May 17, 2023

Copy link
Copy Markdown
Contributor

@codebytere has manually backported this PR to "25-x-y", please check out #38357

@trop

trop Bot commented May 17, 2023

Copy link
Copy Markdown
Contributor

@codebytere has manually backported this PR to "24-x-y", please check out #38361

@trop trop Bot added in-flight/24-x-y merged/24-x-y PR was merged to the "24-x-y" branch merged/25-x-y PR was merged to the "25-x-y" branch. and removed needs-manual-bp/24-x-y in-flight/24-x-y labels May 17, 2023
theolivenbaum added a commit to theolivenbaum/electron-sharp that referenced this pull request May 25, 2023
@michalzaq12

michalzaq12 commented May 28, 2023

Copy link
Copy Markdown
Contributor

@codebytere Is this the intended behavior? The effect only applies to the title bar?
image

@ghost

ghost commented May 28, 2023

Copy link
Copy Markdown

Interesting, with Glasstron this was a bug, not sure if that is suppose to happen.

@kochie

kochie commented May 28, 2023

Copy link
Copy Markdown

For vibrancy on macos you need to have no background color in the div. Maybe double check that your div is transparent? Nope, I get the above as well.

@ghost

ghost commented May 30, 2023

Copy link
Copy Markdown

So sorry for all you that are suscribed but this is SO cool! I'm surprised how little lines it takes to do something like this

@blueset

blueset commented Jun 6, 2023

Copy link
Copy Markdown

@codebytere Is this the intended behavior? The effect only applies to the title bar? image

Having the same issue. Is this expected? Is it possible to apply also Mica to the web content background?

@ghost

ghost commented Jun 6, 2023

Copy link
Copy Markdown

Have you tried with background: none;, background-color: none;, background: transparent;, background-color: transparent; (to <body>)?

@blueset

blueset commented Jun 6, 2023

Copy link
Copy Markdown

@aitor-gomila:
Have you tried with background: none;, background-color: none;, background: transparent;, background-color: transparent; (to <body>)?

image

Yes, I’ve tried all 4 combinations on both <body> and <html>, and it still produces the same result.

Here’s my Electron Fiddle code to repro this: https://gist.github.com/blueset/b406e2d1bd9218a16b599e72a3ee82c7

@oliverschwendener

Copy link
Copy Markdown

@blueset @michalzaq12 I had the same issue, but it worked when I added transparent: true to the BrowserWindow constructor options. Using electron 25.1.0

@blueset

blueset commented Jun 9, 2023

Copy link
Copy Markdown

@oliverschwendener
I had the same issue, but it worked when I added transparent: true to the BrowserWindow constructor options. Using electron 25.1.0

Setting transparent: true partially solves the problem, but the window would become unresizable. See #38454.

@gaetandezeiraud

Copy link
Copy Markdown

I confirm, same problem.

@JerryI

JerryI commented Oct 24, 2023

Copy link
Copy Markdown

I confirm, same problem.

just bump into the same issue.
Confirmed on Win11

@YeesterPlus

Copy link
Copy Markdown

The window becomes clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-review/approved ✅ merged/24-x-y PR was merged to the "24-x-y" branch merged/25-x-y PR was merged to the "25-x-y" branch. semver/minor backwards-compatible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Use new 'Mica' material for BrowserWindow vibrancy in Windows 11 Vibrancy Support for Windows