Add no_std Support

[bushrat011899]

Objective

Before committing to no_std support in image, there are open questions around how to sever dependence on std::io without compromising current performance or compatibility. Since image-gif current does not have no_std support, and is comparatively small, solving that problem here could indicate how the problem could be handled more broadly.

Solution Overview

Similar to ciborium-io, I have re-defined a subset of the std::io traits for use within image-gif. However, unlike ciborium-io, I do not blanket-implement the trait for all types implementing the std::io originals. Instead, each redefined trait gets a transparent wrapper type which forwards the image-gif methods to the underlying std::io ones.

pub trait Write { ... }

#[cfg(feature = "std")]
pub struct IoWrite<W: std::io::Write>(W);

#[cfg(feature = "std")]
impl<W: std::io::Write> Write for IoWrite<W> { ... }

Since the wrapper is only available with the feature that adds the implementation, features remain additive. For image-gif, I have not provided any baked-in implementations for types like Vec<u8>, opting instead to leave it up to end users to write their own wrappers around core/alloc types if they need to. This reduces the maintenance burden on image-gif itself, since it only has 1 implementation to test, the one that simply forwards to std::io which it was already testing.

Solution

• Increased MSRV to 1.81 for core::error::Error. This simplifies feature gates considerably.
• Updated the color_quant feature to use BTreeMap and BTreeSet instead of HashMap and HashSet. This is purely an internal change and should not have any observable side effects.
• Added BufRead and Write traits to traits.rs which contain the bare-minimum functionality required for the rest of the crate. These are public so crate consumers may define their own readers/writers.
• Updated WriteBytesExt to be a super-trait of Write. I also changed the implementations to use u16::to_le_bytes (likewise for u32 and u64). This solves what I believe is a possible bug in the implementation where one of those values could be partially written before an error is returned, which is inconsistent with the otherwise total use of write_all.
• [With std feature] Added IoBufReader and IoWrite wrapper types and implemented BufRead and Write for them.
• Added EncodingFormatError::BufferImproperlySized variant. This reduces reliance on std::io::Error and is not a breaking change, as EncodingFormatError is already non_exhaustive
• Added EncodingError::MissingWriter and EncodingError::OutOfMemory for the same reasons above. This is a breaking change as it was not non_exhaustive. I have now marked it non_exhaustive to prevent this issue in the future.
• Changed EncodingError::Io to wrap a Box<dyn Error> instead of an io::Error. This just allows any error type to be used for IO errors, but is a breaking change.
• Added Encoder::new_with_writer which constructs an Encoder<W> where W: Write. This is contrast to the existing Encoder::new, which now returns an Encoder<IoWriter<W>> where W: std::io::Write.
• Added DecodingError::InsufficientBuffer, DecodingError::UnexpectedEof, DecodingError::MissingDecoder variants for the same reasons as above.
• Changed DecodingError::Io to wrap a Box<dyn Error> for the same reasons as above.
• Implemented the OutputBuffer::Vec code-path for LzwReader::decode_bytes. This wasn't required, but it appeared simple enough.
• Added DecodeOptions::read_info_with_buffered_reader which constructs a Decoder<R> where R: BufRead. This is contrast to the existing DecodeOptions::read_info, which now returns an Decoder<IoBufReader<std::io::BufReader<R>>> where R: std::io::Read.
• Added Decoder::new_with_buffered_reader which constructs a Decoder<R> where R: BufRead. This is contrast to the existing Decoder::new, which now returns an Decoder<IoBufReader<std::io::BufReader<R>>> where R: std::io::Read.
• Ran cargo fmt (this is the reason the diff is so large, the actual no_std changes are closer to +400/-200). Please refer to the individual commits to see the effect of formatting on its own.

[bushrat011899]

Not sure why the test decode::check_for_end_code_is_configurable is failing. I don't think the changes I have made should impact that particular test. I can replicate locally so it isn't spurious.

[197g]

Can we split this up over two or three PRs?

There's probably a way to change the architecture to make this work right, more sans-io, but this looks on the surface like adding some confusion in trying to support two different underlying models of streams at the same time. I find it very hard to judge the correctness of what's being implemented through the wrappers and traits that are intermingled with pulling things out of the internals.

The best way to make this reviewable would be:

• first PR the changes to trait bounds (e.g. Read to BufRead) and API surface of std, so that their justification can be done independently from no-std. This also makes it easier to judge ramifications of them in isolation.
• then PR some cleanup of the error types as necessary.
• then PR the traits for no-std which now should appear as their separate API surface so we don't have to worry about changes to std behavior at this stage.

I've indicated some of the critical parts of the code in comments, they apply to one of these three bullet points respectively.

[197g]

That's one of those little things that I meant with: "will there be a tradeoff between no-std and performance". Those are not equivalent, though of course it may not matter much. The use, building a palette, is one of the noticeably slowest paths of naively encoding.

Maybe this is actually good because performance sensitive users may already be falling back to supplying a palette and thus not executing any of the set operations. But maybe they do. It's very speculative.

[197g]

If I get an IO error I want to inspect at least the error code, i.e. if it is WouldBlock or ConnectionReset, StorageFull etc. Exactly this is being erase which the majority of std users do not benefit from. Another tradeoff between no-std and proper modelling.

[197g]

This part I actually like, io::ErrorKind is arguably incorrect. If you want to rework the interface bit-by-bit into something amenable to no-std, it would be a good start to ensure all the internal errors are no longer dependent on io::Error/io::ErrorKind in any way. This could also clean up some stringly typed errors.

[197g]

This is a good change regardless. We may have been double-buffering all the time and the interface for constructing the reader isn't actually that different. The only change change for existing downstream users would be that they have to denote the type as Decoder<BufReader<R>> instead of Decoder<R> if they actually annotated the type exactly.

[197g]

Similar to ciborium-io, I have re-defined a subset of the std::io traits for use within image-gif. However, unlike ciborium-io, I do not blanket-implement the trait for all types implementing the std::io originals. Instead, each redefined trait gets a transparent wrapper type which forwards the image-gif methods to the underlying std::io ones.

If you intend to publish this as a crate in some capacity, compare to not-io which implements this model; maybe we can determine the non-obvious parts of the design space by this parallel construction. Do note it seems to get a little complicated to write the arising trait bounds in public interface of consumers, at least I haven't found good guide-level explanations on that yet—there's tradeoffs in all cases. (And update the crate to 1.81 on the way; which was the major blocker for me pursuing this).

[bushrat011899]

Marking as draft while I split out smaller PRs based on this one. Once the splitting is complete I'll close this PR.