Skip to content

Commit bbde68d

Browse files
andrews05andrews05
authored
Refactor optimize_raw (#670)
Code always tends to get messy over time. I've found the `optimize_raw` function increasingly harder to read, particularly after the addition of fast mode, so I've taken some time to refactor and simplify it. One change of note here is the main compression trials now use the Evaluator. This means verbose output is a little different which is shown below. There is no change to performance or output size. `-vvo2`: master ``` Processing: tests/files/rgba_8_should_be_palette_4.png 500x400 pixels, PNG format 8-bit RGB + Alpha, non-interlaced IDAT size = 2757 bytes File size = 18109 bytes Eval: 4-bit Indexed (5 colors) None 1837 bytes Eval: 8-bit Indexed (5 colors) None 1988 bytes Eval: 4-bit Indexed (5 colors) Bigrams >1837 bytes Eval: 8-bit Indexed (5 colors) Bigrams >1837 bytes Transformed image to 4-bit Indexed (5 colors), non-interlaced Evaluating: 2 filters Eval: 4-bit Indexed (5 colors) Sub >1810 bytes Eval: 4-bit Indexed (5 colors) Entropy >1810 bytes Trying: None zc = 11 f = None 1583 bytes Found better combination: zc = 11 f = None 1583 bytes IDAT size = 1583 bytes (1174 bytes decrease) file size = 16962 bytes (1147 bytes = 6.33% decrease) 16962 bytes (6.33% smaller): Running in pretend mode, no output ``` `-vvo2`: PR ``` Processing: tests/files/rgba_8_should_be_palette_4.png 500x400 pixels, PNG format 8-bit RGB + Alpha, non-interlaced IDAT size = 2757 bytes File size = 18109 bytes Eval: 4-bit Indexed (5 colors) None 1837 bytes Eval: 8-bit Indexed (5 colors) None 1988 bytes Eval: 4-bit Indexed (5 colors) Bigrams >1837 bytes Eval: 8-bit Indexed (5 colors) Bigrams >1837 bytes Transformed image to 4-bit Indexed (5 colors), non-interlaced Evaluating 2 filters Eval: 4-bit Indexed (5 colors) Sub >1810 bytes Eval: 4-bit Indexed (5 colors) Entropy >1810 bytes Trying filter None with zc = 11 1610 bytes Found better result: zc = 11, f = None IDAT size = 1583 bytes (1174 bytes decrease) file size = 16962 bytes (1147 bytes = 6.33% decrease) 16962 bytes (6.33% smaller): Running in pretend mode, no output ``` `-vvZo5`: master ``` Processing: tests/files/rgba_8_should_be_palette_4.png 500x400 pixels, PNG format 8-bit RGB + Alpha, non-interlaced IDAT size = 2757 bytes File size = 18109 bytes Eval: 8-bit Indexed (battiato sort) None 1821 bytes Eval: 4-bit Indexed (5 colors) None 1657 bytes Eval: 8-bit Indexed (mzeng sort) None 1821 bytes Eval: 8-bit Indexed (5 colors) None 1821 bytes Eval: 8-bit Indexed (battiato sort) Bigrams >1821 bytes Eval: 4-bit Indexed (5 colors) Bigrams >1657 bytes Eval: 8-bit Indexed (mzeng sort) Bigrams >1657 bytes Eval: 8-bit Indexed (5 colors) Bigrams >1657 bytes Transformed image to 4-bit Indexed (5 colors), non-interlaced Trying: 8 filters zc = zopfli f = Brute 1562 bytes zc = zopfli f = Sub >1562 bytes zc = zopfli f = Bigrams >1562 bytes zc = zopfli f = None 1407 bytes zc = zopfli f = Up >1407 bytes zc = zopfli f = MinSum >1407 bytes zc = zopfli f = BigEnt >1407 bytes zc = zopfli f = Entropy >1407 bytes Found better combination: zc = zopfli f = None 1407 bytes IDAT size = 1407 bytes (1350 bytes decrease) file size = 16786 bytes (1323 bytes = 7.31% decrease) 16786 bytes (7.31% smaller): Running in pretend mode, no output ``` `-vvZo5`: PR ``` Processing: tests/files/rgba_8_should_be_palette_4.png 500x400 pixels, PNG format 8-bit RGB + Alpha, non-interlaced IDAT size = 2757 bytes File size = 18109 bytes Eval: 8-bit Indexed (battiato sort) None 1821 bytes Eval: 4-bit Indexed (5 colors) None 1657 bytes Eval: 8-bit Indexed (mzeng sort) None 1821 bytes Eval: 8-bit Indexed (5 colors) None 1821 bytes Eval: 8-bit Indexed (battiato sort) Bigrams >1657 bytes Eval: 4-bit Indexed (5 colors) Bigrams >1657 bytes Eval: 8-bit Indexed (mzeng sort) Bigrams >1657 bytes Eval: 8-bit Indexed (5 colors) Bigrams >1657 bytes Transformed image to 4-bit Indexed (5 colors), non-interlaced Trying 8 filters with zopfli, zi = 15 Eval: 4-bit Indexed (5 colors) Brute 1589 bytes Eval: 4-bit Indexed (5 colors) Bigrams 1641 bytes Eval: 4-bit Indexed (5 colors) Sub 1711 bytes Eval: 4-bit Indexed (5 colors) None 1434 bytes Eval: 4-bit Indexed (5 colors) Up 1764 bytes Eval: 4-bit Indexed (5 colors) MinSum 1760 bytes Eval: 4-bit Indexed (5 colors) BigEnt 1742 bytes Eval: 4-bit Indexed (5 colors) Entropy 1748 bytes Found better result: zopfli, zi = 15, f = None IDAT size = 1407 bytes (1350 bytes decrease) file size = 16786 bytes (1323 bytes = 7.31% decrease) 16786 bytes (7.31% smaller): Running in pretend mode, no output ```
1 parent 8a44cdb commit bbde68d

File tree

10 files changed

+190
-256
lines changed

10 files changed

+190
-256
lines changed

‎benches/deflate.rs

+5-20
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,15 @@ fn deflate_16_bits(b: &mut Bencher) {
1313
let input = test::black_box(PathBuf::from("tests/files/rgb_16_should_be_rgb_16.png"));
1414
let png = PngData::new(&input, &Options::default()).unwrap();
1515

16-
b.iter(|| {
17-
let min = AtomicMin::new(None);
18-
deflate(png.raw.data.as_ref(), 12, &min)
19-
});
16+
b.iter(|| deflate(png.raw.data.as_ref(), 12, None));
2017
}
2118

2219
#[bench]
2320
fn deflate_8_bits(b: &mut Bencher) {
2421
let input = test::black_box(PathBuf::from("tests/files/rgb_8_should_be_rgb_8.png"));
2522
let png = PngData::new(&input, &Options::default()).unwrap();
2623

27-
b.iter(|| {
28-
let min = AtomicMin::new(None);
29-
deflate(png.raw.data.as_ref(), 12, &min)
30-
});
24+
b.iter(|| deflate(png.raw.data.as_ref(), 12, None));
3125
}
3226

3327
#[bench]
@@ -37,10 +31,7 @@ fn deflate_4_bits(b: &mut Bencher) {
3731
));
3832
let png = PngData::new(&input, &Options::default()).unwrap();
3933

40-
b.iter(|| {
41-
let min = AtomicMin::new(None);
42-
deflate(png.raw.data.as_ref(), 12, &min)
43-
});
34+
b.iter(|| deflate(png.raw.data.as_ref(), 12, None));
4435
}
4536

4637
#[bench]
@@ -50,10 +41,7 @@ fn deflate_2_bits(b: &mut Bencher) {
5041
));
5142
let png = PngData::new(&input, &Options::default()).unwrap();
5243

53-
b.iter(|| {
54-
let min = AtomicMin::new(None);
55-
deflate(png.raw.data.as_ref(), 12, &min)
56-
});
44+
b.iter(|| deflate(png.raw.data.as_ref(), 12, None));
5745
}
5846

5947
#[bench]
@@ -63,10 +51,7 @@ fn deflate_1_bits(b: &mut Bencher) {
6351
));
6452
let png = PngData::new(&input, &Options::default()).unwrap();
6553

66-
b.iter(|| {
67-
let min = AtomicMin::new(None);
68-
deflate(png.raw.data.as_ref(), 12, &min)
69-
});
54+
b.iter(|| deflate(png.raw.data.as_ref(), 12, None));
7055
}
7156

7257
#[bench]

‎src/atomicmin.rs

-5
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ impl AtomicMin {
2222
}
2323
}
2424

25-
/// Unset value is `usize_max`
26-
pub const fn as_atomic_usize(&self) -> &AtomicUsize {
27-
&self.val
28-
}
29-
3025
/// Try a new value, returning true if it is the new minimum
3126
pub fn set_min(&self, new_val: usize) -> bool {
3227
new_val < self.val.fetch_min(new_val, SeqCst)

‎src/colors.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,11 @@ impl Display for ColorType {
3232
#[inline]
3333
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
3434
match self {
35-
Self::Grayscale { .. } => Display::fmt("Grayscale", f),
36-
Self::RGB { .. } => Display::fmt("RGB", f),
37-
Self::Indexed { palette } => {
38-
Display::fmt(&format!("Indexed ({} colors)", palette.len()), f)
39-
}
40-
Self::GrayscaleAlpha => Display::fmt("Grayscale + Alpha", f),
41-
Self::RGBA => Display::fmt("RGB + Alpha", f),
35+
Self::Grayscale { .. } => write!(f, "Grayscale"),
36+
Self::RGB { .. } => write!(f, "RGB"),
37+
Self::Indexed { palette } => write!(f, "Indexed ({} colors)", palette.len()),
38+
Self::GrayscaleAlpha => write!(f, "Grayscale + Alpha"),
39+
Self::RGBA => write!(f, "RGB + Alpha"),
4240
}
4341
}
4442
}

‎src/deflate/deflater.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
use libdeflater::*;
22

3-
use crate::{atomicmin::AtomicMin, PngError, PngResult};
3+
use crate::{PngError, PngResult};
44

5-
pub fn deflate(data: &[u8], level: u8, max_size: &AtomicMin) -> PngResult<Vec<u8>> {
5+
pub fn deflate(data: &[u8], level: u8, max_size: Option<usize>) -> PngResult<Vec<u8>> {
66
let mut compressor = Compressor::new(CompressionLvl::new(level.into()).unwrap());
7-
let capacity = max_size
8-
.get()
9-
.unwrap_or_else(|| compressor.zlib_compress_bound(data.len()));
7+
let capacity = max_size.unwrap_or_else(|| compressor.zlib_compress_bound(data.len()));
108
let mut dest = vec![0; capacity];
119
let len = compressor
1210
.zlib_compress(data, &mut dest)

‎src/deflate/mod.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::{fmt, fmt::Display};
55

66
pub use deflater::{crc32, deflate, inflate};
77

8-
use crate::{AtomicMin, PngError, PngResult};
8+
use crate::{PngError, PngResult};
99
#[cfg(feature = "zopfli")]
1010
mod zopfli_oxipng;
1111
#[cfg(feature = "zopfli")]
@@ -30,13 +30,13 @@ pub enum Deflaters {
3030
}
3131

3232
impl Deflaters {
33-
pub(crate) fn deflate(self, data: &[u8], max_size: &AtomicMin) -> PngResult<Vec<u8>> {
33+
pub(crate) fn deflate(self, data: &[u8], max_size: Option<usize>) -> PngResult<Vec<u8>> {
3434
let compressed = match self {
3535
Self::Libdeflater { compression } => deflate(data, compression, max_size)?,
3636
#[cfg(feature = "zopfli")]
3737
Self::Zopfli { iterations } => zopfli_deflate(data, iterations)?,
3838
};
39-
if let Some(max) = max_size.get() {
39+
if let Some(max) = max_size {
4040
if compressed.len() > max {
4141
return Err(PngError::DeflatedDataTooLong(max));
4242
}
@@ -49,9 +49,9 @@ impl Display for Deflaters {
4949
#[inline]
5050
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
5151
match self {
52-
Self::Libdeflater { compression } => Display::fmt(compression, f),
52+
Self::Libdeflater { compression } => write!(f, "zc = {compression}"),
5353
#[cfg(feature = "zopfli")]
54-
Self::Zopfli { .. } => Display::fmt("zopfli", f),
54+
Self::Zopfli { iterations } => write!(f, "zopfli, zi = {iterations}"),
5555
}
5656
}
5757
}

‎src/evaluate.rs

+21-14
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use std::sync::{
1010

1111
#[cfg(feature = "parallel")]
1212
use crossbeam_channel::{unbounded, Receiver, Sender};
13+
use deflate::Deflaters;
1314
use indexmap::IndexSet;
1415
use log::trace;
1516
use rayon::prelude::*;
@@ -28,9 +29,15 @@ pub(crate) struct Candidate {
2829
}
2930

3031
impl Candidate {
32+
/// Return an estimate of the output size which can help with evaluation of very small data
33+
#[must_use]
34+
pub fn estimated_output_size(&self) -> usize {
35+
self.idat_data.len() + self.image.key_chunks_size()
36+
}
37+
3138
fn cmp_key(&self) -> impl Ord {
3239
(
33-
self.idat_data.len() + self.image.key_chunks_size(),
40+
self.estimated_output_size(),
3441
self.image.data.len(),
3542
self.filter,
3643
// Prefer the later image added (e.g. baseline, which is always added last)
@@ -43,7 +50,7 @@ impl Candidate {
4350
pub(crate) struct Evaluator {
4451
deadline: Arc<Deadline>,
4552
filters: IndexSet<RowFilter>,
46-
compression: u8,
53+
deflater: Deflaters,
4754
optimize_alpha: bool,
4855
nth: AtomicUsize,
4956
executed: Arc<AtomicUsize>,
@@ -60,15 +67,15 @@ impl Evaluator {
6067
pub fn new(
6168
deadline: Arc<Deadline>,
6269
filters: IndexSet<RowFilter>,
63-
compression: u8,
70+
deflater: Deflaters,
6471
optimize_alpha: bool,
6572
) -> Self {
6673
#[cfg(feature = "parallel")]
6774
let eval_channel = unbounded();
6875
Self {
6976
deadline,
7077
filters,
71-
compression,
78+
deflater,
7279
optimize_alpha,
7380
nth: AtomicUsize::new(0),
7481
executed: Arc::new(AtomicUsize::new(0)),
@@ -118,7 +125,7 @@ impl Evaluator {
118125
// These clones are only cheap refcounts
119126
let deadline = self.deadline.clone();
120127
let filters = self.filters.clone();
121-
let compression = self.compression;
128+
let deflater = self.deflater;
122129
let optimize_alpha = self.optimize_alpha;
123130
let executed = self.executed.clone();
124131
let best_candidate_size = self.best_candidate_size.clone();
@@ -140,9 +147,16 @@ impl Evaluator {
140147
return;
141148
}
142149
let filtered = image.filter_image(filter, optimize_alpha);
143-
let idat_data = deflate::deflate(&filtered, compression, &best_candidate_size);
150+
let idat_data = deflater.deflate(&filtered, best_candidate_size.get());
144151
if let Ok(idat_data) = idat_data {
145-
let size = idat_data.len() + image.key_chunks_size();
152+
let new = Candidate {
153+
image: image.clone(),
154+
idat_data,
155+
filtered,
156+
filter,
157+
nth,
158+
};
159+
let size = new.estimated_output_size();
146160
best_candidate_size.set_min(size);
147161
trace!(
148162
"Eval: {}-bit {:23} {:8} {} bytes",
@@ -151,13 +165,6 @@ impl Evaluator {
151165
filter,
152166
size
153167
);
154-
let new = Candidate {
155-
image: image.clone(),
156-
idat_data,
157-
filtered,
158-
filter,
159-
nth,
160-
};
161168

162169
#[cfg(feature = "parallel")]
163170
{

‎src/headers.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
display_chunks::DISPLAY_CHUNKS,
99
error::PngError,
1010
interlace::Interlacing,
11-
AtomicMin, Deflaters, PngResult,
11+
Deflaters, PngResult,
1212
};
1313

1414
#[derive(Debug, Clone)]
@@ -275,9 +275,9 @@ pub fn extract_icc(iccp: &Chunk) -> Option<Vec<u8>> {
275275
}
276276
}
277277

278-
/// Construct an iCCP chunk by compressing the ICC profile
279-
pub fn construct_iccp(icc: &[u8], deflater: Deflaters) -> PngResult<Chunk> {
280-
let mut compressed = deflater.deflate(icc, &AtomicMin::new(None))?;
278+
/// Make an iCCP chunk by compressing the ICC profile
279+
pub fn make_iccp(icc: &[u8], deflater: Deflaters, max_size: Option<usize>) -> PngResult<Chunk> {
280+
let mut compressed = deflater.deflate(icc, max_size)?;
281281
let mut data = Vec::with_capacity(compressed.len() + 5);
282282
data.extend(b"icc"); // Profile name - generally unused, can be anything
283283
data.extend([0, 0]); // Null separator, zlib compression method

0 commit comments

Comments
 (0)