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

Is it possible to decouple the Icon functionality from a specific version of the image crate? #661

Closed
icefoxen opened this issue Sep 23, 2018 · 3 comments

Comments

@icefoxen
Copy link
Contributor

As it is now, WindowBuilder::with_window_icon() and winit::Icon have API's that use various types from the image crate: image::image::ImageError, image::image::ImageFormat, some of the From impl's for Icon, etc.

These all use the version of image that winit uses, 0.19, and so if you want to use that functionality you have to use the same version of image. This isn't really a huge problem, but is kind of inconvenient. Perhaps winit could at least re-export that version of the image crate? I dunno what the best solution is without just newtype'ing all the types to hide them.

@icefoxen
Copy link
Contributor Author

Bitten by this again. Even more mysteriously, Icon::from_rgba() returns Result<Self, winit::BadIcon>, while every other Icon thing returns Result<Self, image::ImageError>. Heck!

A re-export would be real nice.

@Osspial
Copy link
Contributor

Osspial commented Feb 13, 2019

The reason Icon::from_rgba() doesn't return image::ImageError is because none of its code actually goes through the image crate - it does a couple of internal integrity checks then passes the raw data directly to the active windowing backend. I'm pretty sure the image functionality just exists for convenience, though I could be wrong about that (cc @francesca64).

Honestly, I'm kinda tempted to remove the image integration altogether, as icon setting works perfectly fine without it through from_rgba(). Additionally, publicly exposing an image dependency means we can't stabilize until image can, and we've gotten so many issues about having an out-of-date image version that I can't image it's more convenient than having people just write a couple lines of file-loading code themselves.

@francesca64
Copy link
Member

@Osspial correct, the image functionality is completely decoupled. I also agree about removing it.

@Osspial Osspial closed this as completed Feb 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants