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 transparency on X11 #2006

Merged
merged 4 commits into from
Aug 22, 2021
Merged

Fix transparency on X11 #2006

merged 4 commits into from
Aug 22, 2021

Conversation

sandmor
Copy link
Contributor

@sandmor sandmor commented Aug 22, 2021

This PR fixes the broke transparency feature on X11.

Note that for this feature matrix said that this already worked, but it was code as TODO and only would work if someone provide a custom visual, which is not documented. This PR fixes that, allowing winit to find an appropriate visual for transparency.

This also fix this issue in wgpu for X11 allowing to create transparent background(I just tried that, I didn't do extensive testing).

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • 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

@coderedart
Copy link

does this mean that we can finally have transparent windows in winit with vulkan/wgpu?

@sandmor
Copy link
Contributor Author

sandmor commented Aug 22, 2021

No, this only affects X11(so a specific subset of Linux) it will don't do anything for any other platform furthermore I just tested the hello-triangle example with transparent background using a local clone of wgpu repo with the winit dependency pointing to my fork, and it worked smoothly contrary to before where you only saw a black background where it should be transparent.

As I said this only affects X11, that issue seems present on multiple platforms.

@maroider maroider added DS - x11 C - waiting on maintainer A maintainer must review this code labels Aug 22, 2021
Copy link
Contributor

@ArturKovacs ArturKovacs left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the contribution!

I think there are only two small improvements that could be made, but otherwise this looks ready to be merged.

src/platform_impl/linux/x11/window.rs Outdated Show resolved Hide resolved
Comment on lines 201 to 205
} else {
// There is not a visual that matches the criteria of being `TrueColor` and 32 bits depth,
// and panic or error is not appropiate so just ignore it and continue
(ffi::CopyFromParent, ffi::CopyFromParent as *mut ffi::Visual)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a good idea to log a debug message here like

debug!("Could not set transparency, because XMatchVisualInfo returned zero for the required parameters");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed, thank you

@ArturKovacs ArturKovacs merged commit b54d477 into rust-windowing:master Aug 22, 2021
@coderedart
Copy link

Thank you for this pr. i have been waiting for wgpu transparency for months. i thought the issue was with wgpu all this time. <3

@sandmor
Copy link
Contributor Author

sandmor commented Aug 24, 2021

It is okay I was building a library to use winit with tiny-skia after realize that wgpu don't support transparency, then it doesn't worked as I expected am I println the depth of the window that was using when it was 24 I was to check the winit window creation code and then solved the issue 🙂

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 - x11
Development

Successfully merging this pull request may close these issues.

Window alpha transparency support
4 participants