-
Notifications
You must be signed in to change notification settings - Fork 903
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
Restore the ability to have fully transparent windows on Windows #1815
Conversation
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.
This PR looks like it's mostly a revert of 6343059, but without the following "hack": // HACK: When opaque (opacity 255), there is a trail whenever
// the transparent window is moved. By reducing it to 254,
// the window is rendered properly.
let opacity = 254;
// The color key can be any value except for black (0x0).
let color_key = 0x0030c100;
winuser::SetLayeredWindowAttributes(
real_window.0,
color_key,
opacity,
winuser::LWA_ALPHA,
); The comment in this snippet claims that it solves an issue observed during the development of #675, and it seems like this is what caused alacritty/alacritty#1927, which got fixed in #1621. This PR does seem to fix the issue described in #1814, but I'm not too sure if it's the best way to do it. Commenting out this call to |
Correct. As I mentioned in the commit message, the part that is being reverted here was claimed to be essentially a no-op, which it is not. I guess the author thought that the region covers the whole client area when it actually covers none of it. So there was a false premise for this change.
I suppose that only works for the
Without decorations, just the blur behind call with the empty regions works for me, but when I enable decorations, the call to I'm not really sure why that style is required. It's from 28775be, which is yet another commit that claims to fix some issue with a trail being left behind (only in the changelog, not even in the commit message itself) with no further explanation being given. :-( Removing both the code that sets the For per-pixel transparency, the documentation also says that you need to use
I have a branch at https://github.com/dotdash/winit/tree/test that has a commit that removes the On a side note, to whom it may concern, and (mostly) JFYI: You seem to be using the "rebase and merge" option for pull requests, and as a side effect of that, commit ids between the master branch and the pull requests differ, which made it quite a bit harder for me to correlate things between the master branch and github, and looking up where changes came from. It also makes it way harder to identify whether things happened on indepedent branches independently, and each branch was correct on its own, but the merge results is invalid not because of textual conflicts but because of semantic conflicts. The rebase approach completely dismisses that information. It also makes the automated checks a bit less useful. If there are changes on master that semantically break the changes in the pull request, then the rebased commits will be broken and e.g. For example, for #1740 you can find the information by digging through the logs of the checks, seeing that it was tested by merging into 96809ac but was then rebased onto ee3996c, so the test results are meaningless for the commit that ended up in the master branch, while with a true merge, you'd at least know that the merged commits are fine, and were compatible with master at the time of the check. Ok, that got way longer than I anticipated. I guess I just got a bit frustrated because analyzing this was made so much harder by this. What you make of this is up to you of course. |
If it's back in 2014, then |
Thanks! Just in case, do you know whether it would be possible or a good idea to remove the |
Honestly I haven't touched anything related to Windows windowing for several years, and I don't remember at all what |
It might be possible to use |
I tried your changes with Glutin's For reference: I also used the following here so I could drag the window around without decorations: winuser::WM_NCHITTEST => winuser::HTCAPTION, It would be nice to have someone with an older version of Windows test this since I only have Windows 10. cc @0xpr03, since you've listed yourself as a tester for Windows 7 |
Hi and thanks for investigating! The initial PR for this was #675, which seems to initially be added for win10 (#260 (comment)) I'm in general in favor of removing the |
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.
Sorry, took me a bit to finally try it out.
Tried it with an OpenGL application as well and works fine!
Same behavior when running with the additional change for removing SetLayeredWindowAttributes
.
Please, add a short entry to the CHANGELOG and I think adding the other commit as well would good.
Thanks!
Just in case it was unclear what it was that I wanted you to test (on Windows 7), @0xpr03:
[patch.'crates-io'.winit]
git = "https://github.com/dotdash/winit"
branch = "fix_transparency"
I honestly don't expect to see any issues here, but it would be nice to have it confirmed. |
Ah thanks, it looked like this already got approved based on the comment of msiglreith |
Yeah, I just want to have some form of confirmation that it actually works on Windows 7, since |
I'm sorry but it looks like my windows 7 VM can't do this.. Err value: NoAvaila
blePixelFormat', glutin_examples\examples\transparent.rs:15:74 It's already a pain to get the right toolchains for win7. rust-msvc won't work anymore, it requires a newer .NET that won't install on win7 or is straight up "unsupported", even if you manage to get license access to msvc 2013-2017 |
Update: There is hope, my Linux system has a working windows 7 VM with transparency. Seems my windows box has a windows 7 that is stuck with outdated stuff. |
Sorry for the noise but nope.. I can have transparent windows and run games in that VM but apparently not this pixel format. |
`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.
Sorry for letting this lay around for so long, I've now added the other commit with a proper commit message. |
I just did similar thing few weeks ago (transparency to druid) on Windows side. I got an answer from Microsoft devs that the most performant way to do is with If you use EDIT I'm reading the logic here, maybe the idea is that I can use |
Thanks for the tutorial link, it was an interesting read :) If I read that correctly, then all you should need to do is to call |
The black art of Windows transparency! I know one more way (that doesn't involve legacy layered windows): // Copy pasted from C++
// Creates DWM surface
MARGINS m = { -1 };
if (!SUCCEEDED(DwmExtendFrameIntoClientArea(hwnd, &m))) {
throw runtime_error("Unable to extend");
} I used it once in my C++ code. Maybe it's similar to used It works only right after
DwmExtendFrameIntoClientArea trick doesn't require using that Blur feature at all, it just makes it transparent for some reason. |
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:
CreateRectRgn
call, since we want the entire window's region tohave 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.
cargo fmt
has been run on this branchcargo doc
builds successfullyCHANGELOG.md
if knowledge of this change could be valuable to users