From 9b7912265102012ce9cc89c17d7f964bf1ea1d6b Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight <saschanaz@outlook.com> Date: Fri, 1 Mar 2024 00:35:41 +0100 Subject: [PATCH 1/2] Emit u16 pixels instead of f32 This magically fixes WIC too-bright bug. --- src/lib.rs | 36 +++++++++++++++++++----------------- tests/wic.rs | 2 +- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index dc88fc0..e92e507 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -242,18 +242,18 @@ impl IWICBitmapSource_Impl for JXLWICBitmapFrameDecode { fn GetPixelFormat(&self) -> windows::core::Result<GUID> { log::trace!("JXLWICBitmapFrameDecode::GetPixelFormat"); - // TODO: Support all formats + match self.pixel_format { - PixelFormat::Gray => Ok(GUID_WICPixelFormat32bppGrayFloat), + PixelFormat::Gray => Ok(GUID_WICPixelFormat16bppGray), + // WIC doesn't support Graya, but maybe can be emulated with RGBA PixelFormat::Graya => Err(windows::core::Error::new( WINCODEC_ERR_UNSUPPORTEDPIXELFORMAT, "Gray alpha image is currently not supported", )), - PixelFormat::Rgb => Ok(GUID_WICPixelFormat96bppRGBFloat), - PixelFormat::Rgba => Ok(GUID_WICPixelFormat128bppRGBAFloat), - jxl_oxide::PixelFormat::Cmyk | jxl_oxide::PixelFormat::Cmyka => Err( - windows::core::Error::new(WINCODEC_ERR_BADIMAGE, "Cmyk is currently not supported"), - ), + PixelFormat::Rgb => Ok(GUID_WICPixelFormat48bppRGB), + PixelFormat::Rgba => Ok(GUID_WICPixelFormat64bppRGBA), + PixelFormat::Cmyk => Ok(GUID_WICPixelFormat64bppCMYK), + PixelFormat::Cmyka => Ok(GUID_WICPixelFormat80bppCMYKAlpha), } } @@ -285,23 +285,25 @@ impl IWICBitmapSource_Impl for JXLWICBitmapFrameDecode { return Err(E_INVALIDARG.ok().unwrap_err()); } + let pbbuffer = pbbuffer as *mut u16; + let prc = unsafe { prc.as_ref().unwrap() }; log::trace!("JXLWICBitmapFrameDecode::CopyPixels::WICRect {:?}", prc); let channels = self.frame.channels(); + let buf = self.frame.buf(); for y in prc.Y..(prc.Y + prc.Height) { - let src_offset = self.width as i32 * channels as i32 * y; + let src_offset = self.width as i32 * channels as i32 * y + prc.X; let dst_offset = prc.Width * 4 * (y - prc.Y); - unsafe { - std::ptr::copy_nonoverlapping( - self.frame - .buf() - .as_ptr() - .offset((src_offset + prc.X) as isize), - (pbbuffer as *mut f32).offset(dst_offset as isize), - (prc.Width as usize) * channels, - ); + + for x in prc.X..(prc.X + prc.Width) * channels as i32 { + // XXX: jxl-oxide emits f32 pixels, but it can't be used as-is because of WIC limitation. + // Thus here we convert f32 to u16 instead. https://github.com/saschanaz/jxl-winthumb/issues/29 + unsafe { + *pbbuffer.offset((dst_offset + x) as isize) = + (buf[(src_offset + x) as usize] * u16::MAX as f32) as u16; + } } } diff --git a/tests/wic.rs b/tests/wic.rs index 78707e6..c8b5c3f 100644 --- a/tests/wic.rs +++ b/tests/wic.rs @@ -42,7 +42,7 @@ fn basic() { } .expect("Copy pixels"); assert_eq!(pixels[0], 0, "red"); - assert_eq!(pixels[1], 42, "green"); // XXX: But this should be 6... + assert_eq!(pixels[1], 6, "green"); assert_eq!(pixels[2], 0, "blue"); assert_eq!(pixels[3], 255, "alpha"); } From bfd9b50905fff38f2b95ce585d39cc176c6c71e7 Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight <saschanaz@outlook.com> Date: Fri, 1 Mar 2024 01:15:09 +0100 Subject: [PATCH 2/2] logic errors --- src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e92e507..dfdf8c1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -290,14 +290,14 @@ impl IWICBitmapSource_Impl for JXLWICBitmapFrameDecode { let prc = unsafe { prc.as_ref().unwrap() }; log::trace!("JXLWICBitmapFrameDecode::CopyPixels::WICRect {:?}", prc); - let channels = self.frame.channels(); + let channels = self.frame.channels() as i32; let buf = self.frame.buf(); for y in prc.Y..(prc.Y + prc.Height) { - let src_offset = self.width as i32 * channels as i32 * y + prc.X; - let dst_offset = prc.Width * 4 * (y - prc.Y); + let src_offset = (self.width as i32 * y + prc.X) * channels; + let dst_offset = prc.Width * (y - prc.Y) * channels; - for x in prc.X..(prc.X + prc.Width) * channels as i32 { + for x in prc.X..(prc.X + prc.Width) * channels { // XXX: jxl-oxide emits f32 pixels, but it can't be used as-is because of WIC limitation. // Thus here we convert f32 to u16 instead. https://github.com/saschanaz/jxl-winthumb/issues/29 unsafe {