Motivation
Cursor icons (#9557) and window icons (#1031) are highly requested, and seem like they should be easy.
Ideally, we should:
- Load an image (defined in
bevy_render) using bevy_asset.
- Create a handle to the image.
- Feed the data to
winit via bevy_window and then bevy_winit.
- Store this information as a field on
Window, or create an equivalent Cursor component and entity,
Straightforward, idiomatic. Like @cart said in #1163 (comment)
If possible, I think it would be nice if we could load icons using the asset system (ex: make the window descriptor take a Handle). But imo thats only really an option if we can set the winit icon after creating the window. Otherwise we're blocking window construction (and various app startup / gpu allocations) on an image being loaded.
The problem
Unfortunately, Bevy's multi-crate, modular nature makes this quite painful.
This proposed workflow means:
bevy_window and bevy_winit need to be aware of the Image type.
bevy_window and bevy_winit need to be aware of Handle, and thus all of bevy_asset.
Right now though, the dependencies are reversed:
bevy_render depends on bevy_window, because it needs to draw to it.
bevy_asset depends on bevy_winit to handle an Android edge case.
Making bevy_window depend on bevy_render is impossible, since that edge is always required.
Making bevy_winit depend on bevy_asset is undesired, since that prevents users from using their own asset-management solution.
This was discussed at length in #14284, but has come up in various forms every time a contributor attempts to implement one of these features.
How can we avoid coupling these crates tightly without creating a terrible UX?
Caveat around window icons
As discussed in #8130 (comment) and other comments in that thread, it's not clear that we can / should be setting the window's icon in Bevy at all. That may be a documentation / build-time template problem.
Solution 1: just use image bytes
Rather than passing in an Image type, just pass in a string of bytes in the format that winit expects, as defined by [Icon::from_rgba].
This approach works, and would unlock the features, but ends up feeling very half-baked and unidiomatic. We have an Image type, surely we should use that to set images?
This solution is the approach taken by #14196, #14284, and #2268. No one seems particularly pleased with the end result, so these PRs stall out repeatedly.
Solution 2: Synchronize with systems
Now that windows are entities, we could probably work around the crate structure issue by adding a WindowIcon(Handle) component to bevy_render. I think our current crate structure is close to being optimal already here and I definitely don't want to refactor a bunch of stuff just for this. Definitely little bit odd, but imo its still reasonably logically consistent (from a dep perspective).
We could then write a winit-specific system in bevy_render to synchronize the icon bytes with winit.
The winit API should just accept image bytes for the Custom variant. We just need a layer of indirection on the current Window cursor setting that accepts an asset handle once winit supports the feature, and a plugin that updates it with the data in the Handle
.
The basic idea here is that we implement a solution like above that just accepts a string of bytes. Then, somewhere in a different bevy crate that can see bevy_window, bevy_render and bevy_asset, we run a system that tracks a configuration resource or component which stores a Handle<Image>, and then calls into the bevy_window API with the new data.
We should continue the windows-as-entity pattern here, and use components for this data.
Solution 3: split Image into a dedicated crate
As discussed in #11888, Image is a type with complex behavior and possibly wide-spread value. If we were to pull it into its own crate, the bevy_window (and thus bevy_winit) API could accept a strongly typed Image, improving the robustness.
Unfortunately, Image must implement Asset to be used in Bevy's asset pipeline, and as a result, would end up pulling in bevy_asset to the bevy_window tree.
This could be avoided using a feature flag, but still means that there's nasty indirection. Without pulling a direct dependency from bevy_window to bevy_asset, this solution is still left with the indirection involved in 2.
Analysis
Solution 1 is quick and dirty. The final API is pretty terrible, but it does technically work. This low-level, direct-from-Winit API should probably be exposed no matter what.
Solution 3 doesn't have any serious benefits. To actually get to the ideal solution outlined at the top of this excessively long issue, we would need to pull in bevy_asset into bevy_window, which we don't want to do. Splitting out Image is also likely to be super controversial and derail things.
Solution 2 is weird (why are we updating windowing stuff from bevy_render?), but gives a near-ideal API that can use handles to images, and handles the gnarly conversion silently under the hood. It's unfortunate that we can't just have another field on Window, or a bevy_window::Cursor component, but without mucking with our crate structure I think this is the best we can do.
In theory we could make another crate which depends on everything and stick just the window and cursor handling code in there, but that doesn't make the API any nicer and is more complex and controversial. Simply feature-flagging the section of bevy_render seems better.
Motivation
Cursor icons (#9557) and window icons (#1031) are highly requested, and seem like they should be easy.
Ideally, we should:
bevy_render) usingbevy_asset.winitviabevy_windowand thenbevy_winit.Window, or create an equivalentCursorcomponent and entity,Straightforward, idiomatic. Like @cart said in #1163 (comment)
The problem
Unfortunately, Bevy's multi-crate, modular nature makes this quite painful.
This proposed workflow means:
bevy_windowandbevy_winitneed to be aware of theImagetype.bevy_windowandbevy_winitneed to be aware ofHandle, and thus all ofbevy_asset.Right now though, the dependencies are reversed:
bevy_renderdepends onbevy_window, because it needs to draw to it.bevy_assetdepends onbevy_winitto handle an Android edge case.Making
bevy_windowdepend onbevy_renderis impossible, since that edge is always required.Making
bevy_winitdepend onbevy_assetis undesired, since that prevents users from using their own asset-management solution.This was discussed at length in #14284, but has come up in various forms every time a contributor attempts to implement one of these features.
How can we avoid coupling these crates tightly without creating a terrible UX?
Caveat around window icons
As discussed in #8130 (comment) and other comments in that thread, it's not clear that we can / should be setting the window's icon in Bevy at all. That may be a documentation / build-time template problem.
Solution 1: just use image bytes
Rather than passing in an
Imagetype, just pass in a string of bytes in the format thatwinitexpects, as defined by [Icon::from_rgba].This approach works, and would unlock the features, but ends up feeling very half-baked and unidiomatic. We have an
Imagetype, surely we should use that to set images?This solution is the approach taken by #14196, #14284, and #2268. No one seems particularly pleased with the end result, so these PRs stall out repeatedly.
Solution 2: Synchronize with systems
The basic idea here is that we implement a solution like above that just accepts a string of bytes. Then, somewhere in a different bevy crate that can see
bevy_window,bevy_renderandbevy_asset, we run a system that tracks a configuration resource or component which stores aHandle<Image>, and then calls into thebevy_windowAPI with the new data.We should continue the windows-as-entity pattern here, and use components for this data.
Solution 3: split
Imageinto a dedicated crateAs discussed in #11888,
Imageis a type with complex behavior and possibly wide-spread value. If we were to pull it into its own crate, thebevy_window(and thusbevy_winit) API could accept a strongly typedImage, improving the robustness.Unfortunately,
Imagemust implementAssetto be used in Bevy's asset pipeline, and as a result, would end up pulling inbevy_assetto thebevy_windowtree.This could be avoided using a feature flag, but still means that there's nasty indirection. Without pulling a direct dependency from
bevy_windowtobevy_asset, this solution is still left with the indirection involved in 2.Analysis
Solution 1 is quick and dirty. The final API is pretty terrible, but it does technically work. This low-level, direct-from-Winit API should probably be exposed no matter what.
Solution 3 doesn't have any serious benefits. To actually get to the ideal solution outlined at the top of this excessively long issue, we would need to pull in
bevy_assetintobevy_window, which we don't want to do. Splitting outImageis also likely to be super controversial and derail things.Solution 2 is weird (why are we updating windowing stuff from
bevy_render?), but gives a near-ideal API that can use handles to images, and handles the gnarly conversion silently under the hood. It's unfortunate that we can't just have another field onWindow, or abevy_window::Cursorcomponent, but without mucking with our crate structure I think this is the best we can do.In theory we could make another crate which depends on everything and stick just the window and cursor handling code in there, but that doesn't make the API any nicer and is more complex and controversial. Simply feature-flagging the section of
bevy_renderseems better.