minimise features imported from image crate.

[JAicewizard]

Right now using images in druid causes all default features of image to be imported.
This means that I cannot disable features on my end, because druid will "need" them. I am sure druid doesn't need to encode/decode in any image format for me.
The biggest one is "jpeg/rayon" which imports 20 crates or so, which is really not needed for druid.
If this is something you agree with I can minimize the amount of features enabled for image and send in a PR.

[raphlinus]

This sounds like a pragmatic solution to the problem of the image crate dep being too heavyweight. There are other solutions, like being able to use codecs directly (I often just bring in the png crate when I need that), but there's a case to be made for common infrastructure, and image is that, for better or worse.

[JAicewizard]

I just looked through what piet does with the image crate, and it seems it also loads images from a filepath, and it does actually need to decode the images for that. I would personally say that might be out of scope for piet/druid but that's not my decision to make.
If this is every removed I would suggest to remove the unnecessary features, because if piet doesn't use it it would be weird to require it. I do use the image crate, but if I wanted to remove support for x format to save compile speed, I can not do that right now.

[jneem]

Wait, why did you close this? It seems like a reasonable request. And as far as I can tell, the image loading in piet can still work, it will just only support the formats whose features are included...

[JAicewizard]

Oh I thought it was dismissed, sorry I misunderstood the response.

[JAicewizard]

I think the most sensible way to go around this is to default to all default image features to be included, and allow the user to explicitly disable it. Just like it is done in the image crate itself. This prevents the user from being surprised by "unsupported format" whenever they try to load a file, but still allows them to control the formats being supported. Having everything enabled by default seems reasonable, things should work in as many cases as possible by default.

This does mean update druid with new "features" every time image adds support for another format, but that is just kinda how features work, I guess :/

[raphlinus]

Hmm. I personally consider default features to be an anti-pattern (for reasons we're seeing now) and would be entirely comfortable with disabling them, and having people opt in by setting the features on their own image crate dep. But that has its own downsides, including the fact that the image version has to match the dep from druid, and I wouldn't be surprised if it caused a challenge in our examples - I'm not sure it is possible to turn on a feature of a dependent crate in an example but not the main lib.

Would these features be named "image_jpeg", in analogy to "jpeg_rayon"?

[JAicewizard]

I think I recently saw some example with a required feature (that way you can just use "druid_shell/jpeg")?
I think the features should just be analogous to whatever image uses. Just passing them on gives fine-grained control.
Having users opt-in via their own dependency would be best in preventing feature changes affect the user, but like you said it forces them to use the same version as druid.

How about druid-shell disables by default, and then druid passes on all the features to the user? This gets into messy things, its late here but I will think about it some more.

[JAicewizard]

I made a little branch here showing what I would do.
I also looked a bit more into the piet dependencies, and it seems that druid-shell imports piet-common, and piet-common doesnt enable the optional dependency image for piet. I check all the other piets, and none enable this dependency.

I could make the feature-chain go al the way to piet, but I don't really see the point in that. (but maybe it should be done to be sure? I dont see the point in having piet load images when druid doesnt depend on it either. Is piet supposed to be used by others too?)

[jneem]

druid doesn't load images directly, but it uses the piet::ImageBuf type in a few places (e.g. custom cursors and the Image widget). Some of the druid examples use piet's image feature to create ImageBufs from pngs. Is that what you meant by others "using" piet? I guess they sort of are, but the types are re-exported by druid anyway 🤷

[JAicewizard]

Maybe I am confused about optional dependencies work in rust, if druid-shell depends no image, and piet has an optional dependency on image, does piet get compiled with cfg(feature = "image")=true as well? I cannot see any crate directly enabling image for piet. I never went this deep into dependencies with rust.

But either way, the main question remains, do we do it like this example branch I made or do we make the users import Image themselves?
And if we decide to do it like the example, do we pass it all the way to piet, or do we stop at druid-shell (or druid itself?)

[jneem]

Oh, the confusion is probably because the image stuff in druid-shell is only there temporarily while I wait for a piet release that contains piet#321. It seems like that already happened, so I'll make a pr to remove that. Then druid will be actually using the ImageBuf stuff from piet instead of the copy that's (temporarily) in druid-shell.

As for the re-exporting features, I don't have a strong opinion either way.

[JAicewizard]

I personally think the passing through is the "best" way of doing it, but it requires a PR to piet as well.
I will make a PR for piet, and then once a that change is released I can make a PR to druid as well.