From a62ee0db15ddcbefaf8a1d6d6a5126284bd11b37 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 10 Feb 2025 16:14:53 +1300 Subject: [PATCH] Disallow grayscale if not sRGB --- src/headers.rs | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-- src/lib.rs | 95 +++++++++--------------------------------------- 2 files changed, 112 insertions(+), 81 deletions(-) diff --git a/src/headers.rs b/src/headers.rs index cf447ed0..2bfbda11 100644 --- a/src/headers.rs +++ b/src/headers.rs @@ -1,5 +1,5 @@ use indexmap::IndexSet; -use log::warn; +use log::{debug, trace, warn}; use rgb::{RGB16, RGBA8}; use crate::{ @@ -8,7 +8,7 @@ use crate::{ display_chunks::DISPLAY_CHUNKS, error::PngError, interlace::Interlacing, - Deflaters, PngResult, + Deflaters, Options, PngResult, }; #[derive(Debug, Clone)] @@ -314,3 +314,97 @@ pub fn srgb_rendering_intent(icc_data: &[u8]) -> Option { _ => None, } } + +/// Process aux chunks and potentially adjust options before optimizing +pub fn preprocess_chunks(aux_chunks: &mut Vec, opts: &mut Options) { + let has_srgb = aux_chunks.iter().any(|c| &c.name == b"sRGB"); + // Grayscale conversion should not be performed if the image is not in the sRGB colorspace + // An sRGB profile would need to be stripped on conversion, so disallow if stripping is disabled + let mut allow_grayscale = !has_srgb || opts.strip != StripChunks::None; + + if let Some(iccp_idx) = aux_chunks.iter().position(|c| &c.name == b"iCCP") { + allow_grayscale = false; + // See if we can replace an iCCP chunk with an sRGB chunk + let may_replace_iccp = opts.strip != StripChunks::None && opts.strip.keep(b"sRGB"); + if may_replace_iccp && has_srgb { + // Files aren't supposed to have both chunks, so we chose to honor sRGB + trace!("Removing iCCP chunk due to conflict with sRGB chunk"); + aux_chunks.remove(iccp_idx); + allow_grayscale = true; + } else if let Some(icc) = extract_icc(&aux_chunks[iccp_idx]) { + let intent = if may_replace_iccp { + srgb_rendering_intent(&icc) + } else { + None + }; + // sRGB-like profile can be replaced with an sRGB chunk with the same rendering intent + if let Some(intent) = intent { + trace!("Replacing iCCP chunk with equivalent sRGB chunk"); + aux_chunks[iccp_idx] = Chunk { + name: *b"sRGB", + data: vec![intent], + }; + allow_grayscale = true; + } else if opts.idat_recoding { + // Try recompressing the profile + let cur_len = aux_chunks[iccp_idx].data.len(); + if let Ok(iccp) = make_iccp(&icc, opts.deflate, Some(cur_len - 1)) { + debug!( + "Recompressed iCCP chunk: {} ({} bytes decrease)", + iccp.data.len(), + cur_len - iccp.data.len() + ); + aux_chunks[iccp_idx] = iccp; + } + } + } + } + + if !allow_grayscale && opts.grayscale_reduction { + debug!("Disabling grayscale reduction due to presence of sRGB or iCCP chunk"); + opts.grayscale_reduction = false; + } + + // Check for APNG by presence of acTL chunk + if aux_chunks.iter().any(|c| &c.name == b"acTL") { + warn!("APNG detected, disabling all reductions"); + opts.interlace = None; + opts.bit_depth_reduction = false; + opts.color_type_reduction = false; + opts.palette_reduction = false; + opts.grayscale_reduction = false; + } +} + +/// Perform cleanup of certain aux chunks after optimization has been completed +pub fn postprocess_chunks(aux_chunks: &mut Vec, ihdr: &IhdrData, orig_ihdr: &IhdrData) { + // If the depth/color type has changed, some chunks may be invalid and should be dropped + // While these could potentially be converted, they have no known use case today and are + // generally more trouble than they're worth + if orig_ihdr.bit_depth != ihdr.bit_depth || orig_ihdr.color_type != ihdr.color_type { + aux_chunks.retain(|c| { + let invalid = &c.name == b"bKGD" || &c.name == b"sBIT" || &c.name == b"hIST"; + if invalid { + warn!( + "Removing {} chunk as it no longer matches the image data", + std::str::from_utf8(&c.name).unwrap() + ); + } + !invalid + }); + } + + // Remove any sRGB or iCCP chunks if the image was converted to or from grayscale + if orig_ihdr.color_type.is_gray() != ihdr.color_type.is_gray() { + aux_chunks.retain(|c| { + let invalid = &c.name == b"sRGB" || &c.name == b"iCCP"; + if invalid { + trace!( + "Removing {} chunk as it no longer matches the color type", + std::str::from_utf8(&c.name).unwrap() + ); + } + !invalid + }); + } +} diff --git a/src/lib.rs b/src/lib.rs index 3546cc05..10c85d09 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,7 +26,6 @@ extern crate rayon; mod rayon; use std::{ - borrow::Cow, fs::{File, Metadata}, io::{stdin, stdout, BufWriter, Read, Write}, path::Path, @@ -155,23 +154,27 @@ impl RawImage { /// Create an optimized png from the raw image data using the options provided pub fn create_optimized_png(&self, opts: &Options) -> PngResult> { + let mut opts = opts.to_owned(); + let mut aux_chunks: Vec<_> = self + .aux_chunks + .iter() + .filter(|c| opts.strip.keep(&c.name)) + .cloned() + .collect(); + preprocess_chunks(&mut aux_chunks, &mut opts); + let deadline = Arc::new(Deadline::new(opts.timeout)); - let Some(result) = optimize_raw(self.png.clone(), opts, deadline, None) else { + let Some(result) = optimize_raw(self.png.clone(), &opts, deadline, None) else { return Err(PngError::new("Failed to optimize input data")); }; let mut png = PngData { raw: result.image, idat_data: result.idat_data, - aux_chunks: self - .aux_chunks - .iter() - .filter(|c| opts.strip.keep(&c.name)) - .cloned() - .collect(), + aux_chunks, frames: Vec::new(), }; - postprocess_chunks(&mut png, opts, &self.png.ihdr); + postprocess_chunks(&mut png.aux_chunks, &png.raw.ihdr, &self.png.ihdr); Ok(png.output()) } @@ -346,19 +349,9 @@ fn optimize_png( debug!(" IDAT size = {} bytes", idat_original_size); debug!(" File size = {} bytes", file_original_size); - // Check for APNG by presence of acTL chunk - let opts = if png.aux_chunks.iter().any(|c| &c.name == b"acTL") { - warn!("APNG detected, disabling all reductions"); - let mut opts = opts.to_owned(); - opts.interlace = None; - opts.bit_depth_reduction = false; - opts.color_type_reduction = false; - opts.palette_reduction = false; - opts.grayscale_reduction = false; - Cow::Owned(opts) - } else { - Cow::Borrowed(opts) - }; + let mut opts = opts.to_owned(); + preprocess_chunks(&mut png.aux_chunks, &mut opts); + let max_size = if opts.force { None } else { @@ -368,10 +361,9 @@ fn optimize_png( png.raw = result.image; png.idat_data = result.idat_data; recompress_frames(png, &opts, deadline, result.filter)?; + postprocess_chunks(&mut png.aux_chunks, &png.raw.ihdr, &raw.ihdr); } - postprocess_chunks(png, &opts, &raw.ihdr); - let output = png.output(); if idat_original_size >= png.idat_data.len() { @@ -619,61 +611,6 @@ fn report_format(prefix: &str, png: &PngImage) { ); } -/// Perform cleanup of certain chunks from the `PngData` object, after optimization has been completed -fn postprocess_chunks(png: &mut PngData, opts: &Options, orig_ihdr: &IhdrData) { - if let Some(iccp_idx) = png.aux_chunks.iter().position(|c| &c.name == b"iCCP") { - // See if we can replace an iCCP chunk with an sRGB chunk - let may_replace_iccp = opts.strip != StripChunks::None && opts.strip.keep(b"sRGB"); - if may_replace_iccp && png.aux_chunks.iter().any(|c| &c.name == b"sRGB") { - // Files aren't supposed to have both chunks, so we chose to honor sRGB - trace!("Removing iCCP chunk due to conflict with sRGB chunk"); - png.aux_chunks.remove(iccp_idx); - } else if let Some(icc) = extract_icc(&png.aux_chunks[iccp_idx]) { - let intent = if may_replace_iccp { - srgb_rendering_intent(&icc) - } else { - None - }; - // sRGB-like profile can be replaced with an sRGB chunk with the same rendering intent - if let Some(intent) = intent { - trace!("Replacing iCCP chunk with equivalent sRGB chunk"); - png.aux_chunks[iccp_idx] = Chunk { - name: *b"sRGB", - data: vec![intent], - }; - } else if opts.idat_recoding { - // Try recompressing the profile - let cur_len = png.aux_chunks[iccp_idx].data.len(); - if let Ok(iccp) = make_iccp(&icc, opts.deflate, Some(cur_len - 1)) { - debug!( - "Recompressed iCCP chunk: {} ({} bytes decrease)", - iccp.data.len(), - cur_len - iccp.data.len() - ); - png.aux_chunks[iccp_idx] = iccp; - } - } - } - } - - // If the depth/color type has changed, some chunks may be invalid and should be dropped - // While these could potentially be converted, they have no known use case today and are - // generally more trouble than they're worth - let ihdr = &png.raw.ihdr; - if orig_ihdr.bit_depth != ihdr.bit_depth || orig_ihdr.color_type != ihdr.color_type { - png.aux_chunks.retain(|c| { - let invalid = &c.name == b"bKGD" || &c.name == b"sBIT" || &c.name == b"hIST"; - if invalid { - warn!( - "Removing {} chunk as it no longer matches the image data", - std::str::from_utf8(&c.name).unwrap() - ); - } - !invalid - }); - } -} - /// Recompress the additional frames of an APNG fn recompress_frames( png: &mut PngData,