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

Export the needed parts of Winit or add better utilities for centering a window. #4993

Closed
CalinZBaenen opened this issue Jun 12, 2022 · 9 comments
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Usability A simple quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@CalinZBaenen
Copy link

What problem does this solve or what need does it fill?

The problem this proposition intents to solve is making the task of centering a window easier.
It intents to remove the need to have dependencies for both bevy and winit.

What solution would you like?

There's multiple ways to o about this.
First, the way I would not like to see, by re-exporting the required parts of winit to center a window: winit::dpi::(::PhysicalPosition) and winit::event_loop(::EventLoop).

Second, the solution I'd like to see implemented, WindowDescriptor.position when set to None should say that the window should be centered, then Bevy can take care of everything behind the scenes.
This also allows all windows to be centered by default.

What alternative(s) have you considered?

The alternative I'm using is to have dependencies for both winit and bevy in my Cargo.toml file:

[[bin]]
name = "bevytest"
path = "./src/main.rs"

[package]
edition = "2021"
version = "0.1.0"
name = "bevy_test"

[profile.dev]
opt-level = 1

[profile.dev.package."*"]
opt-level = 3

[dependencies]
winit = { version = "0.26.0", default-features = false }
bevy = { version="0.7" }

However, this seems inconvenient.

Additional Context

I believe centering the window could be delegated to WindowDescriptor.position = None or a center() method on a Window.
Check out the current solution for centering a window in a Discord-thread on the Bevy Discord guild.

@CalinZBaenen CalinZBaenen added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Jun 12, 2022
@CalinZBaenen CalinZBaenen changed the title Export parts the needed parts of Winit or add better utilities for centering a window. Export the needed parts of Winit or add better utilities for centering a window. Jun 12, 2022
@alice-i-cecile alice-i-cecile added A-Windowing Platform-agnostic interface layer to run your app in C-Usability A simple quality-of-life change that makes Bevy easier to use and removed C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Jun 12, 2022
@alice-i-cecile
Copy link
Member

This is an excellent write-up, thanks! I agree with your analysis, and would like this to be simple. We get this question pretty regularly!

@alice-i-cecile alice-i-cecile added the D-Trivial Nice and easy! A great choice to get started with Bevy label Jun 12, 2022
@CalinZBaenen
Copy link
Author

@alice-i-cecile Thank you for the compliment. I feel honored to have gotten the "Good-First-Issue" tag, even for as little as its worth.

@CalinZBaenen
Copy link
Author

I just had an afterthought, and this may or may not be a good idea, but it would be cool to control the window's visibility in a WindowDescriptor as a visibility property, this would theoretically allow you to center the window perfectly accounting for the border without anyone seeing, then you can unhide the window once it's in place.

@alice-i-cecile
Copy link
Member

Ideally we reuse our standard visibility component for that, by making windows entities in #4530 :)

@bjorn3
Copy link
Contributor

bjorn3 commented Jun 12, 2022

Second, the solution I'd like to see implemented, WindowDescriptor.position when set to None should say that the window should be centered

I think that should use the default position assigned by the window manager, not use a centered position.

@CalinZBaenen
Copy link
Author

@bjorn3 Then how would one make the window centered using a WindowDescriptor? Would you suggest a centered:bool property?

@bjorn3
Copy link
Contributor

bjorn3 commented Jun 13, 2022

Maybe make position an enum between fixed absolute position, centered and default position?

@LoipesMas
Copy link
Contributor

That's exactly what mockersf suggested in #4999 (and I now implemented)

@bjorn3
Copy link
Contributor

bjorn3 commented Jun 13, 2022

I didn't see PR until after I commented.

@bors bors bot closed this as completed in 49da4e7 Jul 4, 2022
inodentry pushed a commit to IyesGames/bevy that referenced this issue Aug 8, 2022
# Objective
- Fixes bevyengine#4993 

## Solution

- ~~Add `centered` property to `WindowDescriptor`~~
- Add `WindowPosition` enum
- `WindowDescriptor.position` is now `WindowPosition` instead of `Option<Vec2>`
- Add `center_window` function to `Window`

## Migration Guide
- If using `WindowDescriptor`, replace `position: None` with `position: WindowPosition::Default` and `position: Some(vec2)`  with `WindowPosition::At(vec2)`.

I'm not sure if this is the best approach, so feel free to give any feedback.
Also I'm not sure how `Option`s should be handled in `bevy_winit/src/lib.rs:161`.

Also, on window creation we can't (or at least I couldn't) get `outer_size`, so this doesn't include decorations in calculations.
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective
- Fixes bevyengine#4993 

## Solution

- ~~Add `centered` property to `WindowDescriptor`~~
- Add `WindowPosition` enum
- `WindowDescriptor.position` is now `WindowPosition` instead of `Option<Vec2>`
- Add `center_window` function to `Window`

## Migration Guide
- If using `WindowDescriptor`, replace `position: None` with `position: WindowPosition::Default` and `position: Some(vec2)`  with `WindowPosition::At(vec2)`.

I'm not sure if this is the best approach, so feel free to give any feedback.
Also I'm not sure how `Option`s should be handled in `bevy_winit/src/lib.rs:161`.

Also, on window creation we can't (or at least I couldn't) get `outer_size`, so this doesn't include decorations in calculations.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective
- Fixes bevyengine#4993 

## Solution

- ~~Add `centered` property to `WindowDescriptor`~~
- Add `WindowPosition` enum
- `WindowDescriptor.position` is now `WindowPosition` instead of `Option<Vec2>`
- Add `center_window` function to `Window`

## Migration Guide
- If using `WindowDescriptor`, replace `position: None` with `position: WindowPosition::Default` and `position: Some(vec2)`  with `WindowPosition::At(vec2)`.

I'm not sure if this is the best approach, so feel free to give any feedback.
Also I'm not sure how `Option`s should be handled in `bevy_winit/src/lib.rs:161`.

Also, on window creation we can't (or at least I couldn't) get `outer_size`, so this doesn't include decorations in calculations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Usability A simple quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants