Enable creating "virtual" windows without corresponding OS window
Objective
- Enable creating windows without an associated OS window
- This will be useful for embedding games into other applications like editors
- It will also enable games to be run headless without much change for the specific game
Solution
- Make the
raw_window_handleattribute anOption - Ignore windows with a
raw_window_handleofNoneinprepare_windows- Instead, the creator of a virtual window is responsible for creating a render target texture for the window and injecting it into the
ExtractedWindow
- Instead, the creator of a virtual window is responsible for creating a render target texture for the window and injecting it into the
Changelog
- Added a
new_virtualmethod toWindow, which does not take aRawWindowHandleWrapper - Changed
Window::raw_window_handleto returnNoneif it was created usingnew_virtual - Changed
prepare_windowsto ignore windows that were created usingnew_virtual
Related to #6114, which makes this field an Option.
#6147 seems to be tackling similar problems as well. Ping @anlumo and @targrub; I'd like to talk through how all of these PRs inter-relate and how we should move forward.
Oh, I missed #6114, which seems to make almost the same changes with mainly different intention / language around them. It also adds a test example and changes the RenderApp to only run if there is a primary window present (which I'm not sure is the right behavior).
I'd suggest rebasing @targrub's changes on my implementation. My pull request seems to be more minimal and be more open to different use cases. #6114 for example says
/// During normal use, this can be safely unwrapped; the value should only be [`None`] when synthetically constructed for tests.
which rules out the use cases I am suggesting. Of course we should probably mention the use of "virtual windows" for testing and @targrub's test example is also a good addition.
I'm not sure how #6147 fits in. The windows created there do seem to always have a RawWindowHandleWrapper, but it looks like this handle is not used. Maybe it makes sense to change the handle into an enum
enum AbstractWindowHandle {
Raw(RawWindowHandleWrapper),
Virtual,
Canvas(Canvas),
}
instead.
Sounds good; I'm on board with that course of action.
I rewrote my code to use an enum instead of Option. I did not include a Canvas variant, but it should be easy for @anlumo to adapt their PR to use the same enum.
I did not include a
Canvasvariant, but it should be easy for @anlumo to adapt their PR to use the same enum.
I've updated my PR to use your enum instead.
Oh, I missed #6114, which seems to make almost the same changes with mainly different intention / language around them. It also adds a test example and changes the
RenderAppto only run if there is a primary window present (which I'm not sure is the right behavior).
I've fixed that behavior in a subsequent commit.
I'd suggest rebasing @targrub's changes on my implementation. My pull request seems to be more minimal and be more open to different use cases. #6114 for example says
I'm not familiar with that phrasing; what does that mean, "rebasing @targrub's changes on my implementation"?
/// During normal use, this can be safely unwrapped; the value should only be [
None] when synthetically constructed for tests.
That comment was written by @alice-i-cecile . :)
I'm not familiar with that phrasing; what does that mean, "rebasing @targrub's changes on my implementation"?
It just means that my PR gets merged first. Your PR would then show a message say something about "conflicts" because we are editing the same lines in the same file. You would then merge the (then) current main branch from bevy into your branch solving these conflicts (or alternatively, run git rebase main on it, that's where the name comes from).
All in all the commit history should then look like
...older commits -> my commits -> your commits
You can also do that work before my PR gets merged, but that requires some git magic.
I'm going to merge targrub's PR first to spare them the git pain :) I also think it's less complex / controversial than this change.
I'm going to merge targrub's PR first to spare them the git pain :) I also think it's less complex / controversial than this change.
Gee, thanks!
It's causing a lot of trouble for my side to not have the AbstractWindowHandle being part of WindowDescriptor, instead generating it by dedicated functions. I don't know why the RawWindowHandle wasn't part of the struct beforehand, and this PR just copies that design choice.
Wouldn't it be a good idea to include AbstractWindowHandle in that struct? The way I understand it is that the WindowDescriptor is supposed to provide the information to generate the Window, but this piece of information is added with the actual function call of creating the Window for some reason instead.
It's causing a lot of trouble for my side to not have the
AbstractWindowHandlebeing part ofWindowDescriptor, instead generating it by dedicated functions. I don't know why theRawWindowHandlewasn't part of the struct beforehand, and this PR just copies that design choice.Wouldn't it be a good idea to include
AbstractWindowHandlein that struct? The way I understand it is that theWindowDescriptoris supposed to provide the information to generate the Window, but this piece of information is added with the actual function call of creating the Window for some reason instead.
Unfortunately I don't think this is possible. As I understand the main purpose of WindowDescriptor is the CreateWindow event. Obviously the window handle cannot exist yet when this event is created. Also this is "user facing" API and should probably not contain implementation details.
For your PR it would make sense though to add the canvas as a field of WindowDescriptor, but maybe someone more qualified can answer that. :sweat_smile:
There are many types called “Window” in bevy. The Window created by CreateWindow is the one used by winit to link the actual OS window to the engine. It's possible that it already exists when this event is called, I think the OS window is just created lazily when it doesn't exist yet.
However, I didn't write that code and it's entirely undocumented, so I'm mostly guessing here based on the snippets I've read through in the course of creating my PR.
Anyways, the problem is that there's no way get the canvas to the winit initializer at the moment without having a web-only API, which I don't want to force.
Couldn't there just be a canvas attribute in WindowDescriptor that's hidden behind a feature flag or that is only active for the web platform?
That was my previous solution, but that complicates things in combination with this PR, making a few things redundant.
I'll have to take a look if I can get that working again in the new architecture.
Sure, good idea. I'm gonna try to come up with something simple.
@anlumo, can I get a review from you on this since you've been touching related code and working off this PR?
@anlumo, can I get a review from you on this since you've been touching related code and working off this PR?
So, sorry for taking so long. I already knew the code itself while adapting my PR to it, and now I've also checked out the documentation and the example. All of that seems fine to me.
@MDeiml can you rebase this?
Closed per rationale in #8278.