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

Support RLE encoding for TGA #2053

Closed
marvin-j97 opened this issue Nov 12, 2023 · 6 comments
Closed

Support RLE encoding for TGA #2053

marvin-j97 opened this issue Nov 12, 2023 · 6 comments

Comments

@marvin-j97
Copy link
Contributor

marvin-j97 commented Nov 12, 2023

I would like to be able to save images as run-length encoded TGA to save space.
The TGA decoder seems to support RLE, but the encoder doesn't.

Sources

Considerations

I'm wondering, from an outside perspective, if I have a DynamicImage, how would one set the encoded = yes/no flag when using img.save("img.tga"). Would something like (pseudo API) img.save_with(path, tga::SaveOptions::encoded()) be required for control over RLE encoding...

@fintelia
Copy link
Contributor

The API could use DynamicImage::write_with_encoder. Analogous to how you can control compression parameters for JPEG:

let encoder = JpegEncoder::new_with_quality(&mut writer, 95);
img.write_with_encoder(encoder)?;

That said, TGA compression is not great by modern standards. If you can use a different format, you'll likely get better compression

@marvin-j97
Copy link
Contributor Author

marvin-j97 commented Nov 12, 2023

That said, TGA compression is not great by modern standards. If you can use a different format, you'll likely get better compression

It's not my choice, unfortunately 😐

The API could use DynamicImage::write_with_encoder

So a possible API could be:

pub struct TgaEncoder<W: Write> {
    writer: W,
    use_rle: bool,
}

impl<W: Write> for TgaEncoder<W> {
  // ... new

  pub fn with_rle(w: W) -> TgaEncoder<W> {
    TgaEncoder {
      writer: w,
      use_rle: true,
    }
  }

  pub fn use_rle(mut self) -> TgaEncoder<W> {
    self.use_rle = true;
    self
  }
}

let encoder = TgaEncoder::with_rle(&mut writer);
// or
let encoder = TgaEncoder::new(&mut writer).use_rle();
img.write_with_encoder(encoder);

@fintelia
Copy link
Contributor

Yeah. Though, I'd probably suggest calling it "rle" or "compression" or something like that. Even the uncompressed version still counts as "encoding".

Another question is whether it makes more sense to add the new option as a constructor ("with_rle") or as a setter ("set_rle"). I think the existing encoders all use constructors, but that doesn't seem as sustainable if we add more options...

@marvin-j97
Copy link
Contributor Author

Yeah. Though, I'd probably suggest calling it "rle" or "compression" or something like that. Even the uncompressed version still counts as "encoding".

Another question is whether it makes more sense to add the new option as a constructor ("with_rle") or as a setter ("set_rle"). I think the existing encoders all use constructors, but that doesn't seem as sustainable if we add more options...

Updated 👍

@marvin-j97
Copy link
Contributor Author

marvin-j97 commented Nov 12, 2023

I'm trying to read a bit into the code, and just documenting my thoughts.

Looks like tga::Header::from_pixel_data would need another parameter use_rle, and extend the match from:

match color_type {
  ColorType::Rgba8 => (8, 24, ImageType::RawTrueColor),
  // ...
}

to

match (color_type, use_rle) {
  (ColorType::Rgba8, false) => (8, 24, ImageType::RawTrueColor),
  (ColorType::Rgba8, true) => (8, 24, ImageType::RunTrueColor),
  // ... etc
}

Need to check the spec to see if this is the only change required for header.rs.

@marvin-j97
Copy link
Contributor Author

Here's my first pass for TGA RLE support:

#2056

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

2 participants