diff --git a/CHANGELOG.md b/CHANGELOG.md index 4be13fe8..781ea0b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for s3m and mod format files to `agb-tracker`. +### Changed + +- Changed how 16 colour palettes are optimised to give better average case results. You should find that + either your palettes will always import, or never import correctly. + ## [0.21.0] - 2024/09/24 ### Added diff --git a/agb-image-converter/Cargo.toml b/agb-image-converter/Cargo.toml index 15dd5d66..7e78a931 100644 --- a/agb-image-converter/Cargo.toml +++ b/agb-image-converter/Cargo.toml @@ -20,3 +20,7 @@ proc-macro2 = "1" quote = "1" asefile = "0.3.8" fontdue = "0.9" +pagination-packing = "2.1.0" + +[dev-dependencies] +quickcheck = "1" diff --git a/agb-image-converter/src/colour.rs b/agb-image-converter/src/colour.rs index 26c64f6b..c5e47a37 100644 --- a/agb-image-converter/src/colour.rs +++ b/agb-image-converter/src/colour.rs @@ -1,6 +1,6 @@ -use std::str::FromStr; +use std::{fmt, str::FromStr}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct Colour { pub r: u8, pub g: u8, @@ -8,6 +8,18 @@ pub struct Colour { pub a: u8, } +impl fmt::Debug for Colour { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "#{:02x}{:02x}{:02x}", self.r, self.g, self.b)?; + + if self.a != 0xff { + write!(f, "{:02x}", self.a)?; + } + + Ok(()) + } +} + impl Colour { pub fn from_rgb(r: u8, g: u8, b: u8, a: u8) -> Self { Colour { r, g, b, a } @@ -38,3 +50,26 @@ impl FromStr for Colour { Ok(Colour::from_rgb(r, g, b, 255)) } } + +#[cfg(test)] +impl quickcheck::Arbitrary for Colour { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + Self::from_rgb( + quickcheck::Arbitrary::arbitrary(g), + quickcheck::Arbitrary::arbitrary(g), + quickcheck::Arbitrary::arbitrary(g), + quickcheck::Arbitrary::arbitrary(g), + ) + } + + fn shrink(&self) -> Box> { + Box::new( + vec![ + Colour::from_rgb(0, 0, 0, 0), + Colour::from_rgb(self.r, self.g, self.b, 0), + *self, + ] + .into_iter(), + ) + } +} diff --git a/agb-image-converter/src/deduplicator.rs b/agb-image-converter/src/deduplicator.rs index 92498b6d..f96dec7f 100644 --- a/agb-image-converter/src/deduplicator.rs +++ b/agb-image-converter/src/deduplicator.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, hash::BuildHasher}; +use std::collections::BTreeMap; use crate::{colour::Colour, image_loader::Image}; @@ -42,7 +42,7 @@ pub struct DeduplicatedData { pub transformation: Transformation, } -#[derive(Clone, PartialEq, Eq, Hash)] +#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] struct Tile { data: [Colour; 64], } @@ -99,9 +99,7 @@ pub(crate) fn deduplicate_image(input: &Image, can_flip: bool) -> (Image, Vec (Image, Vec TokenStream { } } - let optimised_results = optimiser.optimise_palettes(); + let optimised_results = optimiser + .optimise_palettes() + .expect("Failed to optimise palettes"); let (palette_data, tile_data, assignments) = palette_tile_data(&optimised_results, &images); diff --git a/agb-image-converter/src/palette16.rs b/agb-image-converter/src/palette16.rs index 03ae3a57..e290e9d3 100644 --- a/agb-image-converter/src/palette16.rs +++ b/agb-image-converter/src/palette16.rs @@ -1,10 +1,13 @@ use crate::colour::Colour; -use std::collections::HashSet; +use std::{ + collections::{BTreeSet, HashSet}, + fmt, +}; const MAX_COLOURS: usize = 256; const MAX_COLOURS_PER_PALETTE: usize = 16; -#[derive(Debug, Clone, Eq, PartialEq, Hash)] +#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)] pub(crate) struct Palette16 { colours: Vec, } @@ -62,12 +65,15 @@ impl Palette16 { self.colours.iter() } - fn union_length(&self, other: &Palette16) -> usize { - self.colours + fn with_transparent(&self, transparent_colour: Colour) -> Self { + let mut new_colours = self.colours.clone(); + let transparent_colour_index = new_colours .iter() - .chain(&other.colours) - .collect::>() - .len() + .position(|&c| c == transparent_colour) + .expect("Could not find tranparent colour in palette"); + new_colours.swap(0, transparent_colour_index); + + Self::from(&new_colours) } fn is_satisfied_by(&self, other: &Palette16) -> bool { @@ -87,6 +93,20 @@ impl IntoIterator for Palette16 { } } +impl<'a, T> From for Palette16 +where + T: IntoIterator, +{ + fn from(value: T) -> Self { + let mut palette = Palette16::new(); + for colour in value.into_iter() { + palette.add_colour(*colour); + } + + palette + } +} + pub(crate) struct Palette16Optimiser { palettes: Vec, colours: Vec, @@ -125,28 +145,42 @@ impl Palette16Optimiser { } } - pub fn optimise_palettes(&self) -> Palette16OptimisationResults { - let mut assignments = vec![0; self.palettes.len()]; - let mut optimised_palettes = vec![]; + pub fn optimise_palettes(&self) -> Result { + let transparent_colour = self + .transparent_colour + .unwrap_or_else(|| Colour::from_rgb(255, 0, 255, 0)); - let mut unsatisfied_palettes = self + let palettes_to_optimise = self .palettes .iter() .cloned() - .collect::>(); + .map(|mut palette| { + // ensure each palette we're creating the covering for has the transparent colour in it + palette.add_colour(transparent_colour); + palette + }) + .collect::>() + .into_iter() + .map(|palette| palette.colours) + .collect::>(); - while !unsatisfied_palettes.is_empty() { - let palette = self.find_maximal_palette_for(&unsatisfied_palettes); + let packed_palettes = + pagination_packing::overload_and_remove::<_, _, Vec<_>>(&palettes_to_optimise, 16); - unsatisfied_palettes.retain(|test_palette| !test_palette.is_satisfied_by(&palette)); + let optimised_palettes = packed_palettes + .iter() + .map(|packed_palette| { + let colours = packed_palette.unique_symbols(&palettes_to_optimise); + Palette16::from(colours).with_transparent(transparent_colour) + }) + .collect::>(); - optimised_palettes.push(palette); - - if optimised_palettes.len() == MAX_COLOURS / MAX_COLOURS_PER_PALETTE { - panic!("Failed to find covering palettes"); - } + if optimised_palettes.len() > 16 { + return Err(DoesNotFitError(packed_palettes.len())); } + let mut assignments = vec![0; self.palettes.len()]; + for (i, overall_palette) in self.palettes.iter().enumerate() { assignments[i] = optimised_palettes .iter() @@ -154,59 +188,86 @@ impl Palette16Optimiser { .unwrap(); } - Palette16OptimisationResults { + Ok(Palette16OptimisationResults { optimised_palettes, assignments, transparent_colour: self.transparent_colour, + }) + } +} + +pub struct DoesNotFitError(pub usize); + +impl fmt::Display for DoesNotFitError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "Could not fit colours into palette, needed {} bins but can have at most 16", + self.0 + ) + } +} + +impl fmt::Debug for DoesNotFitError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self}") + } +} + +#[cfg(test)] +mod test { + use quickcheck::{quickcheck, Arbitrary}; + + use super::*; + + quickcheck! { + fn less_than_256_colours_always_fits(palettes: Vec, transparent_colour: Colour) -> bool { + let mut optimiser = Palette16Optimiser::new(Some(transparent_colour)); + for palette in palettes.clone().into_iter().take(16) { + optimiser.add_palette(palette); + } + + let Ok(optimisation_results) = optimiser.optimise_palettes() else { + return false + }; + + check_palette_invariants(palettes.iter().take(16), optimisation_results, transparent_colour) } } - fn find_maximal_palette_for(&self, unsatisfied_palettes: &HashSet) -> Palette16 { - let mut palette = Palette16::new(); - - palette.add_colour( - self.transparent_colour - .unwrap_or_else(|| Colour::from_rgb(255, 0, 255, 0)), - ); - - loop { - let mut colour_usage = vec![0; MAX_COLOURS]; - let mut a_colour_is_used = false; - - for current_palette in unsatisfied_palettes { - if palette.union_length(current_palette) > MAX_COLOURS_PER_PALETTE { - continue; - } - - for colour in ¤t_palette.colours { - if palette.colours.contains(colour) { - continue; - } - - if let Some(colour_index) = self.colours.iter().position(|c| c == colour) { - colour_usage[colour_index] += 1; - a_colour_is_used = true; - } - } + fn check_palette_invariants<'a>( + palettes: impl Iterator, + optimisation_results: Palette16OptimisationResults, + transparent_colour: Colour, + ) -> bool { + for (i, palette) in palettes.enumerate() { + let optimised_palette = + &optimisation_results.optimised_palettes[optimisation_results.assignments[i]]; + if !palette.is_satisfied_by(optimised_palette) { + return false; } - if !a_colour_is_used { - return palette; + if optimised_palette.colour_index(transparent_colour) != 0 { + return false; + } + } + + true + } + + impl Arbitrary for Palette16 { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let mut palette = Palette16::new(); + + let size: usize = Arbitrary::arbitrary(g); + // never entirely fill the palette, will give at most 15 colours + let size = size.rem_euclid(16); + + for _ in 0..size { + palette.add_colour(Arbitrary::arbitrary(g)); } - let best_index = colour_usage - .iter() - .enumerate() - .max_by(|(_, usage1), (_, usage2)| usage1.cmp(usage2)) - .unwrap() - .0; - - let best_colour = self.colours[best_index]; - - palette.add_colour(best_colour); - if palette.colours.len() == MAX_COLOURS_PER_PALETTE { - return palette; - } + palette } } } diff --git a/agb-image-converter/src/palette256.rs b/agb-image-converter/src/palette256.rs index 8283081b..33d77b09 100644 --- a/agb-image-converter/src/palette256.rs +++ b/agb-image-converter/src/palette256.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::BTreeSet; use crate::{ colour::Colour, @@ -7,13 +7,13 @@ use crate::{ }; pub struct Palette256 { - colours: HashSet, + colours: BTreeSet, } impl Palette256 { pub fn new() -> Self { Self { - colours: HashSet::new(), + colours: BTreeSet::new(), } } @@ -41,8 +41,8 @@ impl Palette256 { .cloned() .collect(); - let current_colours_set = HashSet::from_iter(optimised_palette_colours.iter().cloned()); - let new_colours: HashSet<_> = self + let current_colours_set = BTreeSet::from_iter(optimised_palette_colours.iter().cloned()); + let new_colours: BTreeSet<_> = self .colours .symmetric_difference(¤t_colours_set) .collect();