Implement a relationship between Windows and Monitors.#23249
Implement a relationship between Windows and Monitors.#23249Sapein wants to merge 6 commits intobevyengine:mainfrom
Conversation
This implements a relationship between Windows and Monitors, along with providing an example to let people see what's going on there. ATM there is a small window of time, where the Window Exists, but does not have a Monitor linked to it, I'm not quite sure what the best solution is to fix this as Monitors haven't spawned in yet when we spawn the window...
|
You added a new example but didn't add metadata for it. Please update the root Cargo.toml file. |
kfc35
added
A-Windowing
There was a problem hiding this comment.
Just a suggestion to improve the example. Logic looks good to me (unsure if there is a performance implication between inserting to overwrite a component vs mutating the value of OnMonitor directly when it needs to be changed)
I checked out the branch and verified that the example prints out Some(monitor). However, I don’t have an extensive setup where I can drag the window to a different monitor to see that it updates the value there as well. Might be useful for someone else to test with a multi monitor setup.
Also, CI is failing because the PR needs to be cargo fmt --all’d
Edit: about the initialization of the relationship issue: I’m not helpful here 😅 I at least think it’s worth adding a note in your code about that in the documentation for the relationship if it remains unsolved (and other reviewers are OK with it being unsolved)
…UI to display the monitor.
Yeah I'm not 100% sure about the performance implications and I'm more than willing to change it, this code is mostly just an integrated version of code I already had, where I just did that. I also went ahead and did the As for the relationship issue, yeah I'm kinda hoping someone more familiar with winit and how it works in this context might have a solution there. If the resolution ends up being: "There's not really a good solution and this is an edge case we can have for now." then I'd definitely make sure to add a note. |
|
Works for me when changing the window between monitors on macOS 👍 I don't think this needs a dedicated example, could you add it to another like |
As for the example, I do somewhat agree, as the main reason for the example was to make sure my implementation worked when I was implementing it. So putting it to another example would be perfectly fine by me, although I'm not 100% certain |
Yep, I don't think this can be resolved. |
alice-i-cecile
left a comment
alice-i-cecile
left a comment
There was a problem hiding this comment.
I took a look at the examples folder and monitor_info seems correct. Doesn't need to be fancy, just breadcrumbs to make sure people can discover this. Once that's done this LGTM and I'll merge it. Thanks!
4 participants
Objective
Currently there is no easy way to tell what Monitor a Window is on, which is sometimes required for this like settings menus, without having to tie yourself to
bevy_winit.This also fixes #19169
Solution
This implements a Many-to-One relationship in
bevy_window, which is theHasWindowsandOnMonitorrelationship, which connects Monitors and Windows together.As a note there is currently one potential issue, in that the relationship does not exist immediately when the Window is made, but instead exists shortly after when the system to sync changes between winit and the window component occurs. I'm not sure how to fix this as, due to ordering, the Window technically exists prior to any Monitor Entities being made at times.
Testing
I've tested this on Void Linux X86-64, using the provided example. I have not tested anything else.