Skip to content
This repository has been archived by the owner on Jul 23, 2024. It is now read-only.

Should image-core be no_std #4

Open
3 tasks
HeroicKatora opened this issue Apr 8, 2020 · 4 comments
Open
3 tasks

Should image-core be no_std #4

HeroicKatora opened this issue Apr 8, 2020 · 4 comments
Labels
question Further information is requested

Comments

@HeroicKatora
Copy link
Member

The discussion in image#1184 made me realized that the naming image-core might imply that the crate actually only depends on core. Since this is not already the case, it's liky a very good idea to follow regardless. As a very first step it would help if it were at least clear what features require which standard dependency. This also surfaces some other questions:

  • The error type has a variant for io::Error. We, ourselves, had misused that variant a couple of times until 0.23 which had made the switch more painful than necessary. It's also preventing us from developing our own error type independently and not particularly accurate. Why should it be possible that copying an image buffer results in an IO error?. As also mentioned in this blog post about sled it is also not very ergonomic when one considers that io errors are possibly more fatal to a program than wrong data.

  • ImageDecoder locks all implementors into returning ImageError. This crates a close dependency between the error type and decoding functionality, possibly a lot closer than necessary.

  • The ImageFormat::from_path functionality relies on Path which is only available in std as it is (somewhat) OS-dependent.

@HeroicKatora HeroicKatora added the question Further information is requested label Apr 8, 2020
@fintelia
Copy link
Contributor

fintelia commented Apr 9, 2020

I'd also really like for image-core to be no_std. I'd also add that io::Read plays a role in this puzzle:

  • The ImageDecoder design is tightly coupled to io::Read via the into_reader method and the implicit assumption that decoding operates on an underlying reader (which is why decoding functions need to be able to pass along any io::Error that gets generated).

With some work it might be possible to punt the standard library dependency into the crates that implement ImageDecoder, but I'd view that as only a partial solution. There isn't really a good reason that a BMP or PNG decoding crate shouldn't be able to work just with alloc.

@luojia65
Copy link

luojia65 commented Jun 22, 2020

I think image-core crate should be no_std too, by this way embedded device developers may benefit from rich image-rs ecosystem. There could be many ways to achieve it.
I have a random thought on implementation: provide a special trait for image I/O opertaions. Underlying environment may only need to implement these traits, then they would use the whole image-core features. Or when an operating system is present, image project provide a default implementation (like Rust's std to core). We could discuss on how should we define this trait; it might be better than using a Write or Read provided by std or other crates. We could put cfg onto specific image format type to reduce space cost for embedded devices, thus they may choose their supported image format only, and can easily extend into another image format.

@luojia65
Copy link

We can take embedded-hal's code style: https://docs.rs/embedded-hal/1.0.0-alpha.1/embedded_hal/index.html

@fintelia
Copy link
Contributor

fintelia commented Jun 22, 2020

Could you point to which part/types in that that crate use the pattern you are describing? Since most of our users are still likely going to be running in libstd settings, we want to make sure that there are still ergonomic ways to use the interface with std::fs::File objects, Vecs, and so forth.

For specifically providing features only when an OS is present, we can rely on a "std" feature on the image-core crate. This would work better than trying to provide trait implementations in a second crate due to coherence issues. (This is actually a problem for the standard library, and why they are actually thinking of trying to merge the core and std crates)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants