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.

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

Fix Windows transparency behavior to support fully-opaque regions #1621

Merged
merged 1 commit into from
Oct 14, 2020
Merged

Fix Windows transparency behavior to support fully-opaque regions #1621

merged 1 commit into from
Oct 14, 2020

Conversation

jimporter
Copy link
Contributor

@jimporter jimporter commented Jul 9, 2020

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • cargo doc builds successfully
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

This PR is primarily here to fix a downstream bug (alacritty/alacritty#1927). Testing the transparency feature out on Windows (both via examples/transparency.rs and with a build of alacritty), I was unable to reproduce the issue described in the old comment in this code. I suppose it's possible that this only manifests on some configurations though (for completeness, I'm testing on a Windows 10 environment, though I cross-compiled things on a Linux box using GNU tools). To be on the safe side, it might make sense for other people with access to a Windows environment to double check that this doesn't regress anything.

In addition, I removed a couple of redundant bits in the Windows API calls (see the description below).


Original commit message follows:

This patch removes an unneeded workaround for transparent windows on the Windows platform. In addition, it simplifies a couple of related API calls:

  • Remove the CreateRectRgn call, since we want the entire window's region to have blur behind it, and DwnEnableBlurBehindWindow does that by default.
  • Remove the color_key for SetLayeredWindowAttributes, since it's not used (we're not passing winuser::LWA_COLORKEY to the flags).

@jimporter
Copy link
Contributor Author

@Osspial Does this look ok? (Note: I rebased this onto the latest master revision and Android tests are failing now, but looking at the CI logs, this is happening for other builds too and doesn't have anything to do with my patch, which is Windows-only anyway.)

@jimporter
Copy link
Contributor Author

@Osspial, @kchibisov: I rebased this again onto the latest master and the tests look good again. Just let me know if there's anything I need to change so that this can merge. Thanks!

@msiglreith msiglreith added DS - windows C - waiting on maintainer A maintainer must review this code labels Oct 13, 2020
Copy link
Member

@msiglreith msiglreith left a comment

Choose a reason for hiding this comment

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

Works fine for me, tested with an OpenGL application - 1.0 and non-0.0 showed the desired results.
Could you updated to latest master and adjust the changelog entry please?

This patch removes an unneeded workaround for transparent windows on the
Windows platform. In addition, it simplifies a couple of related API calls:

* Remove the `CreateRectRgn` call, since we want the entire window's region to
  have blur behind it, and `DwnEnableBlurBehindWindow` does that by default.
* Remove the `color_key` for `SetLayeredWindowAttributes`, since it's not used
  (we're not passing `winuser::LWA_COLORKEY` to the flags).
@jimporter
Copy link
Contributor Author

@msiglreith Ok, I updated my branch and moved the relnote to the right section (and rephrased it a bit so it's less hard to understand). Thanks for taking a look!

Copy link
Member

@msiglreith msiglreith left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@msiglreith msiglreith merged commit 6343059 into rust-windowing:master Oct 14, 2020
msiglreith pushed a commit that referenced this pull request Feb 17, 2021
* Restore the ability to have fully transparent windows on Windows

Besides its original purpose, commit 6343059 "Fix Windows transparency
behavior to support fully-opaque regions (#1621)" also included some
changes considered cleanups, one of them was:

* Remove the `CreateRectRgn` call, since we want the entire window's region to
  have blur behind it, and `DwnEnableBlurBehindWindow` does that by default.

But the original code actually disabled the blur effect for the whole
window by creating an empty region for it, because that allows for the
window to be truely fully transparent. With the blur effect in place,
the areas meant to be transparent either blur the things behind it
(until Windows 8) or are darkened (since Windows 8). This also means
that on Windows 8 and newer, the resulting colors are darker than
intended in translucent areas when the blur effect is enabled.

This restores the behaviour from winit <0.24 and fixes #1814.

Arguably, one might want to expose the ability to control the blur
region, but that is outside the scope of this commit.

* Remove useless WS_EX_LAYERED from transparent windows on Windows

`WS_EX_LAYERED` is not supposed to be used in combination with
`CS_OWNDC`. In winit, as it is currently used, `WS_EX_LAYERED` actually
has no effect at all. The only relevant call is to
`SetLayeredWindowAttributes`, which is required to make the window
visible at all with `WS_EX_LAYERED` set, but is called with full
opacity, i.e. there's no transparency involved at all.

The actual transparency is already achieved by using
`DwmEnableBlurBehindWindow`, so `WS_EX_LAYERED` and the call to
`SetLayeredWindowAttributes` can both be removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - waiting on maintainer A maintainer must review this code DS - windows
Development

Successfully merging this pull request may close these issues.

2 participants