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.

Sign up for GitHub

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

Jump to bottom

adding transparency to window builder #3033

Closed
wants to merge 10 commits into from
Closed

adding transparency to window builder #3033

wants to merge 10 commits into from

Conversation

louiidev
Copy link
Contributor

Objective

Fixes #3032

Allowing a user to create a transparent window

Solution

I've allowed the transparent bool to be passed to the winit window builder

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Oct 27, 2021
@louiidev louiidev mentioned this pull request Oct 27, 2021
Closed
alice-i-cecile
alice-i-cecile reviewed Oct 27, 2021
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Awesome, that was much easier than I expected! Some nits on the example, to better communicate to new users, but the actual engine changes LGTM.

@alice-i-cecile alice-i-cecile added A-Windowing Platform-agnostic interface layer to run your app in C-Feature A new feature, making something new possible and removed S-Needs-Triage This issue needs to be labelled labels Oct 27, 2021
@mockersf
Copy link
Member

On macOS, the window is not transparent in this example...
I tried running the transparent example from winit and it's working

@alice-i-cecile
Copy link
Member

Great, I'm happy with the changes to improve clarity / comments :) We should verify / fix that this works on all major platforms (or leave some very heavy notes in the example and docstrings) before merging this.

@louiidev
Copy link
Contributor Author

On macOS, the window is not transparent in this example... I tried running the transparent example from winit and it's working

Thanks for finding that. Yeah I just tested it myself and tried to implement it through wgpu and saw the same problem, so I'm wondering if this is the same issue: gfx-rs/wgpu#687

@mockersf
Copy link
Member

I'm wondering if this is the same issue: gfx-rs/wgpu#687

Yup that looks like it, but it doesn't seem specific to macOS, so it's not working on all platform?

@louiidev
Copy link
Contributor Author

Yup that looks like it, but it doesn't seem specific to macOS, so it's not working on all platform?

working on Windows 10 but I don't have a Linux install that I can test

@alice-i-cecile alice-i-cecile added the S-Needs-Investigation This issue requires detective work to figure out what's going wrong label Oct 28, 2021
@louiidev
Copy link
Contributor Author

Great, I'm happy with the changes to improve clarity / comments :) We should verify / fix that this works on all major platforms (or leave some very heavy notes in the example and docstrings) before merging this.

Left some comments at the top of the example, let me know if it still needs more. It would be nice to test this on Linux as well. I'm wondering if changing wgpu backends has any effect as well?

IceSentry
IceSentry reviewed Oct 28, 2021
View reviewed changes
Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

You need to add the example link in the examples README

@louiidev
Copy link
Contributor Author

You need to add the example link in the examples README

added.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Oct 29, 2021 •
edited
Loading

I left some nits on the commenting, but I'm now happy with the quantity and quality (other than the nits) of the documentation for this PR :)

@louiidev
Copy link
Contributor Author

louiidev commented Nov 1, 2021

I left some nits on the commenting, but I'm now happy with the quantity and quality (other than the nits) of the documentation for this PR :)

Whoops, fixed up the typo's

@alice-i-cecile
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Nov 9, 2021
@bors
Try #3033:
0bc7e42
@alice-i-cecile alice-i-cecile added S-Needs-Review and removed S-Needs-Investigation This issue requires detective work to figure out what's going wrong labels Nov 9, 2021
@bors
Copy link
Contributor

bors bot commented Nov 9, 2021

try

Build failed:

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review labels Nov 10, 2021
@louiidev louiidev mentioned this pull request Nov 10, 2021
Closed
@louiidev louiidev closed this Nov 10, 2021
@inodentry
Copy link
Contributor

Why was this PR closed?

@alice-i-cecile
Copy link
Member

@inodentry, this was closed in favor of #3105, which is just a branch-fixed version of this PR.

bors bot pushed a commit that referenced this pull request Dec 8, 2021
@louiidev
Added transparency to window builder (#3105)
3ca8844 
Applogies, had to recreate this pr because of branching issue.
Old PR: #3033

# Objective

Fixes #3032

Allowing a user to create a transparent window 

## Solution

I've allowed the transparent bool to be passed to the winit window builder
Subserial pushed a commit to Subserial/bevy_winit_hook that referenced this pull request Jan 24, 2024
@louiidev
Added transparency to window builder (#3105)
e28b6c6 
Applogies, had to recreate this pr because of branching issue.
Old PR: bevyengine/bevy#3033

# Objective

Fixes #3032

Allowing a user to create a transparent window 

## Solution

I've allowed the transparent bool to be passed to the winit window builder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@IceSentry IceSentry IceSentry approved these changes

@TheRawMeatball TheRawMeatball Awaiting requested review from TheRawMeatball

Assignees
No one assigned
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow window to be transparent
6 participants
@louiidev @mockersf @alice-i-cecile @inodentry @TheRawMeatball @IceSentry