image icon indicating copy to clipboard operation
image copied to clipboard

DynamicImage::ImageRgb16 copy_from doesn't preserve all the color data

Open AugLuk opened this issue 11 months ago • 1 comments

"image" version: 0.25.5 rustc version: 1.84.0 target: stable-aarch64-apple-darwin profile: release

Expected

When copying sub-images between 2 DynamicImage::ImageRgb16 images, the all color data should be preserved

Actual behaviour

Only u8 worth of data (256 values) per pixel's color channel is transferred This is because get_pixel that is used inside copy_from always returns Rgba<u8>

Reproduction steps

Hopefully it's self explanatory

Possible fix implementation direction

For my project I'm using this crude image_copy_from function I wrote myself It doesn't work with all DynamicImage variants, but I only needed ImageRgb16 It could be expanded to all variants using a macro Consider that:

  • Flat samples might hold u8, u16 or f32, depending on the variant
  • Different variants would have different pixel_channels values

Also, it might be more performant than the original copy_from, but I haven't tested it

Code:

use image::DynamicImage::ImageRgb16;

pub fn image_copy_from(
  target: &mut image::DynamicImage,
  source: &image::DynamicImage,
  x: usize,
  y: usize,
) {
  let target_width = target.width() as usize;
  let target_height = target.height() as usize;
  let source_width = source.width() as usize;
  let source_height = source.height() as usize;

  let source_x_max = if x < target_width {
    source_width.min(target_width - x)
  } else {
    0
  };
  let source_y_max = if y < target_height {
    source_height.min(target_height - y)
  } else {
    0
  };

  match (target, source) {
    (ImageRgb16(target), ImageRgb16(source)) => {
      let pixel_channels = 3;

      let source_fs = source.as_flat_samples();
      let mut target_fs = target.as_flat_samples_mut();
      
      for source_y in 0..source_y_max {
        let first_source_index = source_y * source_width * pixel_channels;
        let first_target_index = ((y + source_y) * target_width + x) * pixel_channels;

        let last_source_index = first_source_index + source_x_max * pixel_channels;
        let last_target_index = first_target_index + source_x_max * pixel_channels;

        let source_slice = &source_fs.as_slice()[first_source_index..last_source_index];
        let target_slice = &mut target_fs.as_mut_slice()[first_target_index..last_target_index];

        target_slice.copy_from_slice(source_slice);
      }
    }
    _ => unimplemented!(),
  }
}

AugLuk avatar Jan 23 '25 16:01 AugLuk

Unfortunately, I think this is another consequence of DynamicImage implementing the GenericImage trait. See also: #2237, #2274, and #2344

fintelia avatar Jan 24 '25 01:01 fintelia