From 77b84699ac4901fdc3d281d4a2fe888bd6433c1f Mon Sep 17 00:00:00 2001 From: zxq82lm <231263183+zxq82lm@users.noreply.github.com> Date: Thu, 2 Oct 2025 20:18:39 -0700 Subject: [PATCH 1/4] fix: wrap hue in HSLColor + add from_degrees --- plotters/src/style/color.rs | 76 ++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/plotters/src/style/color.rs b/plotters/src/style/color.rs index 5721fe74..d6d69f9a 100644 --- a/plotters/src/style/color.rs +++ b/plotters/src/style/color.rs @@ -8,38 +8,32 @@ use serde::{Deserialize, Serialize}; use std::marker::PhantomData; -/// Any color representation +/// Common trait for all color representations. pub trait Color { - /// Normalize this color representation to the backend color fn to_backend_color(&self) -> BackendColor; - /// Convert the RGB representation to the standard RGB tuple #[inline(always)] fn rgb(&self) -> (u8, u8, u8) { self.to_backend_color().rgb } - /// Get the alpha channel of the color #[inline(always)] fn alpha(&self) -> f64 { self.to_backend_color().alpha } - /// Mix the color with given opacity fn mix(&self, value: f64) -> RGBAColor { let (r, g, b) = self.rgb(); let a = self.alpha() * value; RGBAColor(r, g, b, a) } - /// Convert the color into the RGBA color which is internally used by Plotters fn to_rgba(&self) -> RGBAColor { let (r, g, b) = self.rgb(); let a = self.alpha(); RGBAColor(r, g, b, a) } - /// Make a filled style form the color fn filled(&self) -> ShapeStyle where Self: Sized, @@ -47,7 +41,6 @@ pub trait Color { Into::::into(self).filled() } - /// Make a shape style with stroke width from a color fn stroke_width(&self, width: u32) -> ShapeStyle where Self: Sized, @@ -62,10 +55,6 @@ impl Color for &'_ T { } } -/// The RGBA representation of the color, Plotters use RGBA as the internal representation -/// of color -/// -/// If you want to directly create a RGB color with transparency use [RGBColor::mix] #[derive(Copy, Clone, PartialEq, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct RGBAColor(pub u8, pub u8, pub u8, pub f64); @@ -86,13 +75,11 @@ impl From for RGBAColor { } } -/// A color in the given palette #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct PaletteColor(usize, PhantomData

); impl PaletteColor

{ - /// Pick a color from the palette pub fn pick(idx: usize) -> PaletteColor

{ PaletteColor(idx % P::COLORS.len(), PhantomData) } @@ -108,7 +95,6 @@ impl Color for PaletteColor

{ } } -/// The color described by its RGB value #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct RGBColor(pub u8, pub u8, pub u8); @@ -128,31 +114,40 @@ impl Color for RGBColor { } } } + impl BackendStyle for RGBColor { fn color(&self) -> BackendColor { self.to_backend_color() } } -/// The color described by HSL color space #[derive(Copy, Clone, PartialEq, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct HSLColor(pub f64, pub f64, pub f64); +impl HSLColor { + #[inline] + pub fn from_degrees(h_deg: f64, s: f64, l: f64) -> Self { + Self(h_deg / 360.0, s, l) + } +} + impl Color for HSLColor { #[inline(always)] #[allow(clippy::many_single_char_names)] fn to_backend_color(&self) -> BackendColor { - let (h, s, l) = ( - self.0.clamp(0.0, 1.0), - self.1.clamp(0.0, 1.0), - self.2.clamp(0.0, 1.0), - ); + let h = if self.0 > 1.0 { + (self.0 / 360.0).rem_euclid(1.0) + } else { + self.0.rem_euclid(1.0) + }; + let s = self.1.clamp(0.0, 1.0); + let l = self.2.clamp(0.0, 1.0); if s == 0.0 { - let value = (l * 255.0).round() as u8; + let v = (l * 255.0).round() as u8; return BackendColor { - rgb: (value, value, value), + rgb: (v, v, v), alpha: 1.0, }; } @@ -164,13 +159,8 @@ impl Color for HSLColor { }; let p = 2.0 * l - q; - let cvt = |mut t| { - if t < 0.0 { - t += 1.0; - } - if t > 1.0 { - t -= 1.0; - } + let cvt = |t: f64| { + let t = t.rem_euclid(1.0); let value = if t < 1.0 / 6.0 { p + (q - p) * 6.0 * t } else if t < 1.0 / 2.0 { @@ -189,3 +179,29 @@ impl Color for HSLColor { } } } + +#[cfg(test)] +mod hue_robustness_tests { + use super::*; + + #[test] + fn degrees_passed_directly_should_work_for_common_cases() { + let red = HSLColor(0.0, 1.0, 0.5).to_backend_color().rgb; + assert_eq!(red, (255, 0, 0)); + + let green = HSLColor(120.0, 1.0, 0.5).to_backend_color().rgb; + assert_eq!(green, (0, 255, 0)); + + let blue = HSLColor(240.0, 1.0, 0.5).to_backend_color().rgb; + assert_eq!(blue, (0, 0, 255)); + } + + #[test] + fn from_degrees_and_direct_degrees_are_equivalent() { + for ° in &[0.0, 30.0, 60.0, 120.0, 180.0, 240.0, 300.0, 360.0] { + let a = HSLColor(deg, 1.0, 0.5).to_backend_color().rgb; + let b = HSLColor::from_degrees(deg, 1.0, 0.5).to_backend_color().rgb; + assert_eq!(a, b); + } + } +} From 2369300534b7fb8a10949e5ed8aefa9ac42d8b2c Mon Sep 17 00:00:00 2001 From: zxq82lm <231263183+zxq82lm@users.noreply.github.com> Date: Thu, 2 Oct 2025 20:45:21 -0700 Subject: [PATCH 2/4] fix: robust HSL hue handling + add from_degrees and tests --- plotters/src/style/color.rs | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/plotters/src/style/color.rs b/plotters/src/style/color.rs index d6d69f9a..b5a4a63d 100644 --- a/plotters/src/style/color.rs +++ b/plotters/src/style/color.rs @@ -8,32 +8,38 @@ use serde::{Deserialize, Serialize}; use std::marker::PhantomData; -/// Common trait for all color representations. +/// Any color representation pub trait Color { + /// Normalize this color representation to the backend color fn to_backend_color(&self) -> BackendColor; + /// Convert the RGB representation to the standard RGB tuple #[inline(always)] fn rgb(&self) -> (u8, u8, u8) { self.to_backend_color().rgb } + /// Get the alpha channel of the color #[inline(always)] fn alpha(&self) -> f64 { self.to_backend_color().alpha } + /// Mix the color with given opacity fn mix(&self, value: f64) -> RGBAColor { let (r, g, b) = self.rgb(); let a = self.alpha() * value; RGBAColor(r, g, b, a) } + /// Convert the color into the RGBA color which is internally used by Plotters fn to_rgba(&self) -> RGBAColor { let (r, g, b) = self.rgb(); let a = self.alpha(); RGBAColor(r, g, b, a) } + /// Make a filled style form the color fn filled(&self) -> ShapeStyle where Self: Sized, @@ -41,6 +47,7 @@ pub trait Color { Into::::into(self).filled() } + /// Make a shape style with stroke width from a color fn stroke_width(&self, width: u32) -> ShapeStyle where Self: Sized, @@ -55,6 +62,10 @@ impl Color for &'_ T { } } +/// The RGBA representation of the color, Plotters use RGBA as the internal representation +/// of color +/// +/// If you want to directly create a RGB color with transparency use [RGBColor::mix] #[derive(Copy, Clone, PartialEq, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct RGBAColor(pub u8, pub u8, pub u8, pub f64); @@ -75,11 +86,13 @@ impl From for RGBAColor { } } +/// A color in the given palette #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct PaletteColor(usize, PhantomData

); impl PaletteColor

{ + /// Pick a color from the palette pub fn pick(idx: usize) -> PaletteColor

{ PaletteColor(idx % P::COLORS.len(), PhantomData) } @@ -95,6 +108,7 @@ impl Color for PaletteColor

{ } } +/// The color described by its RGB value #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct RGBColor(pub u8, pub u8, pub u8); @@ -114,13 +128,13 @@ impl Color for RGBColor { } } } - impl BackendStyle for RGBColor { fn color(&self) -> BackendColor { self.to_backend_color() } } +/// The color described by HSL color space #[derive(Copy, Clone, PartialEq, Debug, Default)] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct HSLColor(pub f64, pub f64, pub f64); @@ -136,18 +150,23 @@ impl Color for HSLColor { #[inline(always)] #[allow(clippy::many_single_char_names)] fn to_backend_color(&self) -> BackendColor { + // Hue: do not clamp; normalize + // - If >1.0, treat as degrees (divide by 360), then wrap to [0,1) + // - Else, wrap value to [0,1) to accept negatives let h = if self.0 > 1.0 { (self.0 / 360.0).rem_euclid(1.0) } else { self.0.rem_euclid(1.0) }; + + // Saturation & lightness remain clamped to valid ranges let s = self.1.clamp(0.0, 1.0); let l = self.2.clamp(0.0, 1.0); if s == 0.0 { - let v = (l * 255.0).round() as u8; + let value = (l * 255.0).round() as u8; return BackendColor { - rgb: (v, v, v), + rgb: (value, value, value), alpha: 1.0, }; } @@ -159,8 +178,13 @@ impl Color for HSLColor { }; let p = 2.0 * l - q; - let cvt = |t: f64| { - let t = t.rem_euclid(1.0); + let cvt = |mut t| { + if t < 0.0 { + t += 1.0; + } + if t > 1.0 { + t -= 1.0; + } let value = if t < 1.0 / 6.0 { p + (q - p) * 6.0 * t } else if t < 1.0 / 2.0 { From ea990af34358d5abbd768330b402435cf5f19080 Mon Sep 17 00:00:00 2001 From: zxq82lm <231263183+zxq82lm@users.noreply.github.com> Date: Sun, 21 Dec 2025 17:15:41 +0100 Subject: [PATCH 3/4] fix: clarify HSL hue normalization --- plotters/src/style/color.rs | 43 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/plotters/src/style/color.rs b/plotters/src/style/color.rs index b5a4a63d..a70e7133 100644 --- a/plotters/src/style/color.rs +++ b/plotters/src/style/color.rs @@ -140,9 +140,11 @@ impl BackendStyle for RGBColor { pub struct HSLColor(pub f64, pub f64, pub f64); impl HSLColor { + /// Creates an `HSLColor` from degrees, wrapping into `[0, 360)` before normalizing. + /// Prefer this helper when specifying hue in degrees. #[inline] pub fn from_degrees(h_deg: f64, s: f64, l: f64) -> Self { - Self(h_deg / 360.0, s, l) + Self(h_deg.rem_euclid(360.0) / 360.0, s, l) } } @@ -150,14 +152,9 @@ impl Color for HSLColor { #[inline(always)] #[allow(clippy::many_single_char_names)] fn to_backend_color(&self) -> BackendColor { - // Hue: do not clamp; normalize - // - If >1.0, treat as degrees (divide by 360), then wrap to [0,1) - // - Else, wrap value to [0,1) to accept negatives - let h = if self.0 > 1.0 { - (self.0 / 360.0).rem_euclid(1.0) - } else { - self.0.rem_euclid(1.0) - }; + // Hue is expected normalized in [0,1); wrap to keep negative or slightly + // out-of-range inputs usable, but do not reinterpret raw degrees. + let h = self.0.rem_euclid(1.0); // Saturation & lightness remain clamped to valid ranges let s = self.1.clamp(0.0, 1.0); @@ -209,23 +206,31 @@ mod hue_robustness_tests { use super::*; #[test] - fn degrees_passed_directly_should_work_for_common_cases() { - let red = HSLColor(0.0, 1.0, 0.5).to_backend_color().rgb; + fn degrees_passed_via_helper_should_work_for_common_cases() { + let red = HSLColor::from_degrees(0.0, 1.0, 0.5).to_backend_color().rgb; assert_eq!(red, (255, 0, 0)); - let green = HSLColor(120.0, 1.0, 0.5).to_backend_color().rgb; + let green = HSLColor::from_degrees(120.0, 1.0, 0.5).to_backend_color().rgb; assert_eq!(green, (0, 255, 0)); - let blue = HSLColor(240.0, 1.0, 0.5).to_backend_color().rgb; + let blue = HSLColor::from_degrees(240.0, 1.0, 0.5).to_backend_color().rgb; assert_eq!(blue, (0, 0, 255)); } #[test] - fn from_degrees_and_direct_degrees_are_equivalent() { - for ° in &[0.0, 30.0, 60.0, 120.0, 180.0, 240.0, 300.0, 360.0] { - let a = HSLColor(deg, 1.0, 0.5).to_backend_color().rgb; - let b = HSLColor::from_degrees(deg, 1.0, 0.5).to_backend_color().rgb; - assert_eq!(a, b); - } + fn from_degrees_wraps_and_matches_normalized() { + let normalized = HSLColor(120.0 / 360.0, 1.0, 0.5).to_backend_color().rgb; + let via_helper = HSLColor::from_degrees(120.0, 1.0, 0.5).to_backend_color().rgb; + assert_eq!(normalized, via_helper); + + let wrap_positive = + HSLColor::from_degrees(720.0, 1.0, 0.5).to_backend_color().rgb; + let wrap_negative = + HSLColor::from_degrees(-120.0, 1.0, 0.5).to_backend_color().rgb; + let canonical = + HSLColor::from_degrees(0.0, 1.0, 0.5).to_backend_color().rgb; + + assert_eq!(wrap_positive, canonical); + assert_eq!(wrap_negative, HSLColor::from_degrees(240.0, 1.0, 0.5).to_backend_color().rgb); } } From 398328e2147ecb15a67846279c2b226d7523872d Mon Sep 17 00:00:00 2001 From: zxq82lm <231263183+zxq82lm@users.noreply.github.com> Date: Sun, 21 Dec 2025 17:25:31 +0100 Subject: [PATCH 4/4] feat: validate HSLColor construction --- plotters/src/style/color.rs | 98 ++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 11 deletions(-) diff --git a/plotters/src/style/color.rs b/plotters/src/style/color.rs index a70e7133..056755c9 100644 --- a/plotters/src/style/color.rs +++ b/plotters/src/style/color.rs @@ -6,6 +6,7 @@ use plotters_backend::{BackendColor, BackendStyle}; #[cfg(feature = "serialization")] use serde::{Deserialize, Serialize}; +use std::fmt; use std::marker::PhantomData; /// Any color representation @@ -139,12 +140,53 @@ impl BackendStyle for RGBColor { #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] pub struct HSLColor(pub f64, pub f64, pub f64); +/// Errors that can occur when constructing an `HSLColor`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum HSLColorError { + /// Hue (or degrees input) must be finite. + NonFiniteHue, + /// Saturation must be in the closed interval `[0, 1]`. + SaturationOutOfRange, + /// Lightness must be in the closed interval `[0, 1]`. + LightnessOutOfRange, +} + +impl fmt::Display for HSLColorError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + HSLColorError::NonFiniteHue => f.write_str("hue must be finite"), + HSLColorError::SaturationOutOfRange => f.write_str("saturation must be in [0, 1]"), + HSLColorError::LightnessOutOfRange => f.write_str("lightness must be in [0, 1]"), + } + } +} + +impl std::error::Error for HSLColorError {} + impl HSLColor { + /// Creates an `HSLColor` from normalized components, returning an error if any are out of range. + pub fn try_new(h: f64, s: f64, l: f64) -> Result { + if !h.is_finite() { + return Err(HSLColorError::NonFiniteHue); + } + if !s.is_finite() || s < 0.0 || s > 1.0 { + return Err(HSLColorError::SaturationOutOfRange); + } + if !l.is_finite() || l < 0.0 || l > 1.0 { + return Err(HSLColorError::LightnessOutOfRange); + } + Ok(Self(h, s, l)) + } + /// Creates an `HSLColor` from degrees, wrapping into `[0, 360)` before normalizing. - /// Prefer this helper when specifying hue in degrees. + /// Prefer this helper when specifying hue in degrees. Returns an error if saturation + /// or lightness fall outside `[0, 1]` or if the input hue is non-finite. #[inline] - pub fn from_degrees(h_deg: f64, s: f64, l: f64) -> Self { - Self(h_deg.rem_euclid(360.0) / 360.0, s, l) + pub fn from_degrees(h_deg: f64, s: f64, l: f64) -> Result { + if !h_deg.is_finite() { + return Err(HSLColorError::NonFiniteHue); + } + Self::try_new(h_deg.rem_euclid(360.0) / 360.0, s, l) } } @@ -207,30 +249,64 @@ mod hue_robustness_tests { #[test] fn degrees_passed_via_helper_should_work_for_common_cases() { - let red = HSLColor::from_degrees(0.0, 1.0, 0.5).to_backend_color().rgb; + let red = HSLColor::from_degrees(0.0, 1.0, 0.5) + .unwrap() + .to_backend_color() + .rgb; assert_eq!(red, (255, 0, 0)); - let green = HSLColor::from_degrees(120.0, 1.0, 0.5).to_backend_color().rgb; + let green = HSLColor::from_degrees(120.0, 1.0, 0.5) + .unwrap() + .to_backend_color() + .rgb; assert_eq!(green, (0, 255, 0)); - let blue = HSLColor::from_degrees(240.0, 1.0, 0.5).to_backend_color().rgb; + let blue = HSLColor::from_degrees(240.0, 1.0, 0.5) + .unwrap() + .to_backend_color() + .rgb; assert_eq!(blue, (0, 0, 255)); } #[test] fn from_degrees_wraps_and_matches_normalized() { let normalized = HSLColor(120.0 / 360.0, 1.0, 0.5).to_backend_color().rgb; - let via_helper = HSLColor::from_degrees(120.0, 1.0, 0.5).to_backend_color().rgb; + let via_helper = HSLColor::from_degrees(120.0, 1.0, 0.5) + .unwrap() + .to_backend_color() + .rgb; assert_eq!(normalized, via_helper); let wrap_positive = - HSLColor::from_degrees(720.0, 1.0, 0.5).to_backend_color().rgb; + HSLColor::from_degrees(720.0, 1.0, 0.5).unwrap().to_backend_color().rgb; let wrap_negative = - HSLColor::from_degrees(-120.0, 1.0, 0.5).to_backend_color().rgb; + HSLColor::from_degrees(-120.0, 1.0, 0.5).unwrap().to_backend_color().rgb; let canonical = - HSLColor::from_degrees(0.0, 1.0, 0.5).to_backend_color().rgb; + HSLColor::from_degrees(0.0, 1.0, 0.5).unwrap().to_backend_color().rgb; assert_eq!(wrap_positive, canonical); - assert_eq!(wrap_negative, HSLColor::from_degrees(240.0, 1.0, 0.5).to_backend_color().rgb); + assert_eq!( + wrap_negative, + HSLColor::from_degrees(240.0, 1.0, 0.5) + .unwrap() + .to_backend_color() + .rgb + ); + } + + #[test] + fn from_degrees_rejects_out_of_range_components() { + assert!(matches!( + HSLColor::from_degrees(0.0, -0.1, 0.5), + Err(HSLColorError::SaturationOutOfRange) + )); + assert!(matches!( + HSLColor::from_degrees(0.0, 0.5, 1.1), + Err(HSLColorError::LightnessOutOfRange) + )); + assert!(matches!( + HSLColor::from_degrees(f64::INFINITY, 0.5, 0.5), + Err(HSLColorError::NonFiniteHue) + )); } }