Fix panic for Frame::from_rgb* methods when there are exactly 256 colors

[okaneco]

• Change u8 zip iterator to use 0..=255 instead of 0..
• Add test

The creation of the colors_lookup hashmap panics in debug when it
bypasses the NeuQuant algorithm and has exactly 256 colors.

[okaneco]

This sort doesn't seem necessary, or it could be unstable?

[HeroicKatora]

I'm not completely sure either. I think it may help a bit with compression of the images by making similar colors have a similar index in the palette. That would actually be interesting to try out because sorting is certainly not an optimal way to achieve that. If you have some days to spend on optimizing this :D

[HeroicKatora]

Although pure LZW compression should not be affected by the indices anyways, its length is invariant to permutation of symbols afaik.

[okaneco]

That was my thought process as well, I might look into this a bit but would have to come up with a benchmark.

Nearby, there may be room for improvement at the cost of making the logic more complex. A HashSet is used, collected into a Vec, then a HashMap is built from that Vec. I imagine the hashing is expensive but I don't have any numbers to show. The HashMap and palette indexing could be built from the start but there'd be a mental overhead of handling the indexing bookkeeping.

image-gif/src/common.rs

Lines 220 to 243 in 0333539

	// Attempt to build a palette of all colors. If we go over 256 colors,
	// switch to the NeuQuant algorithm.
	let mut colors: HashSet<(u8, u8, u8, u8)> = HashSet::new();
	for pixel in pixels.chunks_exact(4) {
	if colors.insert((pixel[0], pixel[1], pixel[2], pixel[3])) && colors.len() > 256 {
	// > 256 colours, let's use NeuQuant.
	let nq = color_quant::NeuQuant::new(speed, 256, pixels);
	return Frame {
	width,
	height,
	buffer: Cow::Owned(pixels.chunks_exact(4).map(|pix| nq.index_of(pix) as u8).collect()),
	palette: Some(nq.color_map_rgb()),
	transparent: transparent.map(|t| nq.index_of(&t) as u8),
	..Frame::default()
	};
	}
	}
	// Palette size <= 256 elements, we can build an exact palette.
	let mut colors_vec: Vec<(u8, u8, u8, u8)> = colors.into_iter().collect();
	colors_vec.sort();
	let palette = colors_vec.iter().map(|&(r, g, b, _a)| vec![r, g, b]).flatten().collect();
	let colors_lookup: HashMap<(u8, u8, u8, u8), u8> = colors_vec.into_iter().zip(0..).collect();

If I don't get around to it, I'll try to file an issue.

[HeroicKatora]

Open an issue on it right away, that way we at least keep track of this.