image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.98k stars 618 forks source link

Tracking issue: Converting error representations #1134

Closed HeroicKatora closed 4 years ago

HeroicKatora commented 4 years ago

The 0.23 brought a more detailed and strictly typed error representation. We kept an internal compatibility interface here to reduce the rework effort required. The plan is to incrementally port these to use more of the new, shiny features. For a basic example, see the png module. To help, claim one of the image formats by a dedicated tracking PR or issue and rework its decoder and encoder implementation or claim a legacy constructors and deprecate and remove it.

Formats:

Constructors:

nabijaczleweli commented 4 years ago

Does this solution make sense? (diff based on 3a94fc97b148992c55ae20eafcf52d7c376df8ac)

diff --git a/src/dynimage.rs b/src/dynimage.rs
index 4415fe6e..4163e6e6 100644
--- a/src/dynimage.rs
+++ b/src/dynimage.rs
@@ -758,7 +758,9 @@ impl DynamicImage {
             }

             image::ImageOutputFormat::Unsupported(msg) => {
-                Err(ImageError::UnsupportedError(msg))
+                Err(ImageError::UnsupportedError(UnsupportedError::from_format_and_kind(
+                    ImageFormatHint::Unknown,
+                    UnsupportedErrorKind::Format(ImageFormatHint::Name(msg)))))
             },

             image::ImageOutputFormat::__NonExhaustive(marker) => match marker._private {},
diff --git a/src/image.rs b/src/image.rs
index d931ed1d..d45b77a6 100644
--- a/src/image.rs
+++ b/src/image.rs
@@ -128,10 +128,7 @@ impl From<ImageFormat> for ImageOutputFormat {
             #[cfg(feature = "farbfeld")]
             ImageFormat::Farbfeld => ImageOutputFormat::Farbfeld,

-            f => ImageOutputFormat::Unsupported(format!(
-                "Image format {:?} not supported for encoding.",
-                f
-            )),
+            f => ImageOutputFormat::Unsupported(format!("{:?}", f)),
         }
     }
 }
HeroicKatora commented 4 years ago

That should work. It's also more in line with how the variant was supposed to work.

nabijaczleweli commented 4 years ago

Status with #1203:

P:\Rust\image\src>git grep :UnsupportedError
tga/decoder.rs:            return Err(ImageError::UnsupportedError(
tga/decoder.rs:            return Err(ImageError::UnsupportedError(
tga/decoder.rs:                return Err(ImageError::UnsupportedError(
tga/decoder.rs:                return Err(ImageError::UnsupportedError(format!(
tiff.rs:            tiff::TiffError::UnsupportedError(desc) => {
tiff.rs:            tiff::TiffError::UnsupportedError(desc) => {

P:\Rust\image\src>git grep :FormatError
flat.rs:            Error::NormalFormRequired(form) => ImageError::FormatError(
tiff.rs:            err @ tiff::TiffError::FormatError(_) => {
tiff.rs:            err @ tiff::TiffError::FormatError(_) => {

P:\Rust\image\src>git grep :UnsupportedColor
dynimage.rs:        _ => return Err(ImageError::UnsupportedColor(color_type.into())),
farbfeld.rs:            return Err(ImageError::UnsupportedColor(color_type.into()));
flat.rs:            Error::WrongColor(color) => ImageError::UnsupportedColor(color.into()),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::L1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::La1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgb1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgba1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::L2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::La2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgb2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgba2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::L4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::La4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgb4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgba4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Unknown(bits as u8))),
png.rs:            _ => return Err(ImageError::UnsupportedColor(color.into())),

Untagging PNG, TIFF, and PNM. Adding FFI and farbfeld.