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

Allow users to set the window's icon #1031

Open
Azales94500 opened this issue Dec 8, 2020 · 20 comments
Open

Allow users to set the window's icon #1031

Azales94500 opened this issue Dec 8, 2020 · 20 comments
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@Azales94500
Copy link

Hello,

I just started using Bevy and I wanted to start experimenting a bit with a small project, I wanted to put an image as an icon in the game window, but I couldn't find how to do it, even searching in the documentation, I specify that I'm using version 0.3.0 of Bevy, is it me who missed something? Or is this feature not yet implemented?

If it is not implemented, would it be possible to just add a field to the WindowDescriptor resource to specify this icon as it is possible to do it for the window title ?

Thank you in advance for the answers

@mockersf
Copy link
Member

mockersf commented Dec 8, 2020

hello and welcome!

It has not yet been implemented, but it should be possible using winit with_window_icon when building the window.

If you're interested in doing the PR, you could add a field to WindowDescriptor for the icon, with a type copying the Icon type from winit (there shouldn't be a dependency in bevy_windows to winit, so the type has to be copied there). And then when executing create_window , check if an icon has been set in the WindowDescriptor and call with_window_icon from winit on the builder.

Ideally it should be available only when running on windows or with X11 as it won't do anything otherwise, but I'm not sure it's possible to select X11 with a config. You could at least only enable it on windows and linux.

@Azales94500
Copy link
Author

Thanks for the explanation, I'm not sure I have the skills to add it myself, I think it would be better if someone more experienced would do it

@smokku
Copy link
Member

smokku commented Dec 8, 2020

This is a good first-time-issue for Bevy development.

@memoryruins memoryruins added C-Feature A new feature, making something new possible D-Trivial Nice and easy! A great choice to get started with Bevy A-Windowing Platform-agnostic interface layer to run your app in labels Dec 8, 2020
@goodbadwolf
Copy link

Is anyone working on this? If not, I would like to take a stab at this.

@Azales94500
Copy link
Author

Is anyone working on this? If not, I would like to take a stab at this.

Sorry for the late answer, no one claims it, so i think you can work on it ^^

@alice-i-cecile
Copy link
Member

Will be closed by #2268.

@Azales94500
Copy link
Author

Oh, should i close this issue then ?

@NiklasEi
Copy link
Member

NiklasEi commented May 28, 2021

Oh, should i close this issue then ?

You can leave it open. Posting the PR number just creates a link between the issue and the PR. When it gets merged the issue will probably be closed 🙂

@Azales94500
Copy link
Author

Thank you, i didn't know that 👍

@alice-i-cecile
Copy link
Member

This is still incomplete. The work done in #2268 was good, but ended up abandoned without relicensing. When tackling this, you cannot directly reuse that code.

@emwalker
Copy link

Just looking through the history of this story —

  1. From May 28, 2021, to June 19, 2021, @KirmesBude and @NathanSWard worked on Feature/issue 1031 window icon #2268, and then PR went dormant. A challenge here was that there was a licensing change, apparently from MIT to MIT/Apache-2.0 (dual license), and @KirmesBude didn't see the request to re-license the work until significantly later, so at the time people proceeded under the assumption that the PR should be avoided in order to prevent unintentional copying.
  2. From July 29, 2022, to August 29, 2022, @alt0173 and @alice-i-cecile worked on https://github.com/bevyengine/bevy/pull/5488. The PR became blocked on another dependency, and @alt0173 eventually deleted the repo, at which point the PR was automatically closed.

The first PR is now re-licensed, and an attempt to rebase it off of latest main could be made. Not sure which of the two implementations would be preferable.

@alice-i-cecile
Copy link
Member

I think the latter is a better start, but the most relevant insight from those threads was:

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.

#1163 (comment)

@emwalker
Copy link

Looks like there's a dependency for this story on refactoring out Image and Color into their own crate. (I was running into the cyclical dependency between bevy_asset and bevy_window as well.)

What would the crate be called? What would go into it?

@alice-i-cecile
Copy link
Member

bevy_rendering_primitives maybe?

@emwalker
Copy link

That works. This is what the crate ended up looking like:

$ tree bevy_render_primitives 
bevy_render_primitives
├── Cargo.toml
└── src
    ├── color
    │   ├── colorspace.rs
    │   └── mod.rs
    ├── lib.rs
    ├── render_resource
    │   ├── mod.rs
    │   ├── pipeline.rs
    │   ├── resource_macros.rs
    │   └── texture.rs
    └── texture
        ├── basis.rs
        ├── dds.rs
        ├── image.rs
        ├── image_texture_conversion.rs
        ├── ktx2.rs
        └── mod.rs

4 directories, 14 files

To load the image in a background thread, will I need to add an argument for the asset server here?

If so, there will be another circular dependency to untangle, between bevy_winnit and bevy_asset, possibly to be fixed by moving the asset server code to another crate. But it looks like there's a substantial amount of code that would travel with it. Does this sound right? Where would the asset server code be moved?

@alice-i-cecile
Copy link
Member

Yeah, that's starting to get real messy. @cart, opinions? Using assets was your idea originally, and you've been mucking around with asset architecture.

@cart
Copy link
Member

cart commented Mar 18, 2023

Now that windows are entities, we could probably work around the crate structure issue by adding a WindowIcon(Handle<Texture>) 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.

If we later find a crate structure we like better, we can revisit.

@emwalker
Copy link

@cart, @alice-i-cecile a lot of Bevy's concepts are new to me, and I wasn't sure whether I fully understood the proposal. I've put a draft PR together and wanted to double-check that the basic idea is right before attempting to add a unit test and submitting: #8130.

@Agrabski
Copy link

If anyone is interested in a temporary solution, I've created a crate for Bevy 0.11.3, for my own use, that allows for updating window icon, using bevy's assets.
https://crates.io/crates/bevy_window_management

@alice-i-cecile alice-i-cecile added D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed D-Trivial Nice and easy! A great choice to get started with Bevy labels Jul 14, 2024
@alice-i-cecile alice-i-cecile changed the title A field in WindowDescriptor resource to set the window's icon ? Allow users to set the window's icon Jul 14, 2024
@alice-i-cecile
Copy link
Member

Note that the correct approach for this is very OS specific. See #8130 (comment)

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-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
10 participants