okaneco / kmeans-colors

k-means clustering library and binary to find dominant colors in images
Apache License 2.0
132 stars 9 forks source link

Upgrade to palette 0.7 #51

Closed djc closed 1 year ago

djc commented 1 year ago

Would be nice to update to the latest version.

okaneco commented 1 year ago

I'll try to work on that soon. Quite a bit changed from palette 0.6 to palette 0.7 so it's not quite as simple as bumping the version number.

djc commented 1 year ago

Yup. I started on a PR but can't quite figure out how to replace Component.

diff --git a/src/colors/kmeans.rs b/src/colors/kmeans.rs
index e857d4f..a34b425 100644
--- a/src/colors/kmeans.rs
+++ b/src/colors/kmeans.rs
@@ -1,12 +1,14 @@
 #[cfg(feature = "palette_color")]
 use palette::{Lab, Srgb};
+#[cfg(feature = "palette_color")]
+use palette::num::Real;

 use rand::Rng;

 use crate::kmeans::{Calculate, Hamerly, HamerlyCentroids, HamerlyPoint};

 #[cfg(feature = "palette_color")]
-impl<Wp: palette::white_point::WhitePoint> Calculate for Lab<Wp> {
+impl<T: Real, Wp: palette::white_point::WhitePoint<T>> Calculate for Lab<Wp> {
     fn get_closest_centroid(lab: &[Lab<Wp>], centroids: &[Lab<Wp>], indices: &mut Vec<u8>) {
         for color in lab.iter() {
             let mut index = 0;
@@ -162,7 +164,7 @@ impl Calculate for Srgb {
 }

 #[cfg(feature = "palette_color")]
-impl<Wp: palette::white_point::WhitePoint> Hamerly for Lab<Wp> {
+impl<T: Real, Wp: palette::white_point::WhitePoint<T>> Hamerly for Lab<Wp> {
     fn compute_half_distances(centers: &mut HamerlyCentroids<Self>) {
         // Find each center's closest center
         for ((i, ci), half_dist) in centers
@@ -441,9 +443,10 @@ pub trait MapColor: Sized {
 }

 #[cfg(feature = "palette_color")]
-impl<Wp> MapColor for Lab<Wp>
+impl<T, Wp> MapColor for Lab<Wp>
 where
-    Wp: palette::white_point::WhitePoint,
+    T: Real,
+    Wp: palette::white_point::WhitePoint<T>,
 {
     #[inline]
     fn map_indices_to_centroids(centroids: &[Self], indices: &[u8]) -> Vec<Self> {
@@ -459,9 +462,10 @@ where
 }

 #[cfg(feature = "palette_color")]
-impl<Wp> MapColor for palette::Laba<Wp>
+impl<T, Wp> MapColor for palette::Laba<Wp>
 where
-    Wp: palette::white_point::WhitePoint,
+    T: Real,
+    Wp: palette::white_point::WhitePoint<T>,
 {
     #[inline]
     fn map_indices_to_centroids(centroids: &[Self], indices: &[u8]) -> Vec<Self> {
@@ -477,7 +481,7 @@ where
 }

 #[cfg(feature = "palette_color")]
-impl<C> MapColor for Srgb<C>
+impl<T, C> MapColor for Srgb<C>
 where
     C: palette::Component,
 {

(For reference, there's a kind of changelog.)

okaneco commented 1 year ago

Thanks for looking into this. I started working on this before I last commented but kept running into the cascading errors.

Real seems to be the replacement for Component and FloatComponent in some ways with their shared behavior extracted to the traits in palette::num. I'll have to review the PRs and related issues for the reasoning behind the changes. I was probably going to open discussion topics on the palette repo for some aspects of the migration. There might be gaps in the current traits that can be augmented for future palette versions (such as no PartialOrd to sort Real by, or a ToPrimitive like in num-traits to convert from Real to f32/f64).

I believe the T bounds will look something this in order to do arithmetic with the values and comparisons.

impl<Wp, T> Calculate for Lab<Wp, T>
where
    Wp: palette::white_point::WhitePoint<T>,
    T: palette::num::Real + palette::num::Arithmetics + palette::num::PartialCmp,

Anywhere with raw floats needs to use T::from_f64.

-            let mut l = 0.0;
+            let mut l = T::from_f64(0.0);

But we still run into roadblocks because of how kmeans_colors::Calculate is declared. The traits in this crate might need to be parameterized for the output of functions that currently return f32 or more thoroughly redesigned.

pub trait Calculate: Sized {
    fn get_closest_centroid(buffer: &[Self], centroids: &[Self], indices: &mut Vec<u8>);
    fn recalculate_centroids(rng: &mut impl Rng, buf: &[Self], centroids: &mut [Self], indices: &[u8]);
    fn check_loop(centroids: &[Self], old_centroids: &[Self]) -> f32;
    fn create_random(rng: &mut impl Rng) -> Self;
    fn difference(c1: &Self, c2: &Self) -> f32;
}
okaneco commented 1 year ago

I updated palette in #52 and published 0.6 of this crate to crates.io.

djc commented 1 year ago

Thanks!