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

Split ImageBuffer and relevant low-level structs and traits for raw images into a separate crate? #793

Open
newpavlov opened this issue Jul 18, 2018 · 8 comments

Comments

@newpavlov
Copy link

It could be useful for using those structs in other crates which work with raw images and don't want to depend on whole image crate. Alternatively we could add additional default feature which will gate functionality not related to raw image representation.

@tomaka
Copy link
Contributor

tomaka commented Jul 18, 2018

Alternatively we could add additional default feature which will gate functionality not related to raw image representation.

I would prefer this solution. Extracting crates and types into an external crate will likely cause semver hell in the long term.

@newpavlov
Copy link
Author

newpavlov commented Jul 18, 2018

But on the other hand separate crate will allow to stabilize it (lets call it image-core for now) earlier. Plus encoder/decoder crates will be able to depend on image-core, thus providing unified API, which will allow to move out shims which you currently have in image crate. As kind of prior art you can look at motivation behind rand_core split.

@fintelia
Copy link
Contributor

I think we should give this some serious consideration. Right now the crates for specific image types (image-png, image-tiff, etc.) each have to implement their own versions of ColorType, ImageDecoder, and so forth which leads to a lot of redundancy.

@HeroicKatora
Copy link
Member

HeroicKatora commented Feb 12, 2019

I found another reason to support this option: There are two separate concerns at play here. The decoder part, and the image manipulation part. For web, such as in servo, the decoder part is much more focussed and should undergo extreme scrutiny. However, the focus of this library as is is far to large to generate complete 'input' coverage, as even the input term itself is extremely hard to define.

So, in the long term I think we should aim to narrow down the scope of several libs (bikeshedding names):

  • image-core that only provides buffers and methods for pixel handling but no policy and also no interpretation of pixel values (no color spaces), no blending, etc. The best equivalent I can think of is the Linux DRM subsystem that only provides abstract byte buffers and common enumeration definitions in general afaik (ignoring the driver-supplied interface part).
  • image which would re-export those for compat and integrate them with codecs/decoders, but excluding what is currently in crate::imageops. DynamicImage may be better left here than in image-core, to make is more likely for image-core to be possibly no-std and because of its auto converting mechanisms being technically policy. And here would be the extension point for color space conversion if we choose to do so.
  • imageproc for operations on images.

That would give us the possibility of achieving a strong, and well tested foundation with image-core so that users who don't want to opt into the wider scope of auto conversion and operations can make use of decoders still.

@icefoxen
Copy link

I like this but I would suggest that the interpretation of pixel values be part of image-core. Pretty much every image format has its own enum for describing the options here and having them all be convertible into a single one would be a useful point of unification.

@fintelia
Copy link
Contributor

@icefoxen Could you clarify what you mean by interpretation of pixel values? My current thinking is to have image-core include ColorType and ExtendedColorType but otherwise just operate on byte buffers. (Those types aren't yet available on master, but will be at some point).

@icefoxen
Copy link

Ah, sorry for being unclear. I was suggesting that image::ColorType be part of image-core. I thought that was what "no interpretation of pixel values" was referring to but now that I read it again I suppose it's referring to things like linear color vs sRGB.

@fintelia
Copy link
Contributor

fintelia commented Sep 14, 2024

A couple years late, but I should probably provide an update on this...

I tried working on this issue. Created a repository for image-core, started copying over the relevant types, and was getting ready to release an initial 0.1 release. However, in the meantime we did the image 0.23.0 release which included some breaking changes to ImageDecoder, ColorType, and so forth. That made me take step back and think through what the process would be for doing breaking changes if these types had already been moved to the image-core crate. It would be considerably more disruptive: the main image crate and every single decoder that implemented traits from image-core would have to make their own breaking releases in lockstep!

Without the ability to rapidly iterate on the types/traits, I didn't see how image-core would be able to reach stability any faster than having the types in the main image crate. Thus, I stopped working on the image-core transition and redirected my focus to other aspects of the crate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants