Deprecate all usages of the Color struct representing gamma space values, fixing round-trip precision bugs (#4149)

* Deprecate all usages of the Color struct representing gamma space values, fixing round-trip precision bugs

* Code review fixes

Author: Keavon

Cargo.lock

@@ -21,7 +21,7 @@ pub enum HsvChannel {
 #[impl_message(Message, ColorPicker)]
 #[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize)]
 pub enum ColorPickerMessage {
-	/// Initialize the picker state from an external color/gradient and announce its options. Called by the frontend when a `<ColorPicker>` opens.
+	/// Initialize the picker state from an external color/gradient and announce its options. Called by the frontend when a `<ColorPicker />` opens.
 	Open { initial_value: FillChoice, allow_none: bool, disabled: bool },
 	/// Clear the picker state. Called by the frontend when the popover closes.
 	Close,

editor/src/messages/color_picker/color_picker_message.rs

@@ -2,9 +2,10 @@ use crate::messages::color_picker::color_picker_message::{HsvChannel, RgbChannel
 use crate::messages::layout::utility_types::widget_prelude::*;
 use crate::messages::layout::utility_types::widgets::input_widgets::{ColorPresetsInputUpdate, SpectrumInputUpdate, SpectrumMarker, VisualColorPickersInputUpdate};
 use crate::messages::prelude::*;
-use color::{AlphaColor, Srgb};
 use graphene_std::Color;
-use graphene_std::vector::style::{FillChoice, GradientStops};
+use graphene_std::color::SRGBA8;
+use graphene_std::core_types::misc::parse_css_color;
+use graphene_std::vector::style::{FillChoice, FillChoiceUI, GradientStops, GradientStopsUI};
 
 /// Bounds for a midpoint position (relative to the interval between two adjacent gradient stops).
 const MIN_MIDPOINT: f64 = 0.01;
@@ -105,10 +106,13 @@ impl MessageHandler<ColorPickerMessage, ()> for ColorPickerMessageHandler {
 			ColorPickerMessage::SetChannelRgb { channel, value } => {
 				let Some(strength) = value else { return };
 				let Some(current) = self.current_color() else { return };
+				// The RGB inputs are 0..255 sRGB display values; substitute the new channel into the gamma triple and lift back to linear for storage.
+				let new_gamma_channel = (strength / 255.) as f32;
+				let [cur_r, cur_g, cur_b, cur_a] = current.to_gamma_srgb_channels();
 				let updated = match channel {
-					RgbChannel::Red => Color::from_rgbaf32_unchecked((strength / 255.) as f32, current.g(), current.b(), current.a()),
-					RgbChannel::Green => Color::from_rgbaf32_unchecked(current.r(), (strength / 255.) as f32, current.b(), current.a()),
-					RgbChannel::Blue => Color::from_rgbaf32_unchecked(current.r(), current.g(), (strength / 255.) as f32, current.a()),
+					RgbChannel::Red => Color::from_gamma_srgb_channels(new_gamma_channel, cur_g, cur_b, cur_a),
+					RgbChannel::Green => Color::from_gamma_srgb_channels(cur_r, new_gamma_channel, cur_b, cur_a),
+					RgbChannel::Blue => Color::from_gamma_srgb_channels(cur_r, cur_g, new_gamma_channel, cur_a),
 				};
 				self.adopt_color(updated);
 				self.emit_color(responses);
@@ -150,7 +154,7 @@ impl MessageHandler<ColorPickerMessage, ()> for ColorPickerMessageHandler {
 				match preset {
 					FillChoice::None => {
 						self.set_new_hsva(0., 0., 0., 1., true);
-						responses.add(FrontendMessage::ColorPickerColorChanged { value: FillChoice::None });
+						responses.add(FrontendMessage::ColorPickerColorChanged { value: FillChoiceUI::None });
 					}
 					FillChoice::Solid(color) => {
 						self.adopt_color(color);
@@ -175,7 +179,7 @@ impl MessageHandler<ColorPickerMessage, ()> for ColorPickerMessageHandler {
 				self.set_old_hsva(temp.0, temp.1, temp.2, temp.3, temp.4);
 
 				if self.is_none {
-					responses.add(FrontendMessage::ColorPickerColorChanged { value: FillChoice::None });
+					responses.add(FrontendMessage::ColorPickerColorChanged { value: FillChoiceUI::None });
 				} else {
 					self.emit_color(responses);
 				}
@@ -197,6 +201,7 @@ impl MessageHandler<ColorPickerMessage, ()> for ColorPickerMessageHandler {
 }
 
 impl ColorPickerMessageHandler {
+	// The picker's internal HSV state is HSV of sRGB display values
 	fn current_color(&self) -> Option<Color> {
 		if self.is_none {
 			None
@@ -239,7 +244,8 @@ impl ColorPickerMessageHandler {
 
 	/// Set HSV state from a Color, preserving hue and saturation in degenerate cases.
 	fn adopt_color(&mut self, color: Color) {
-		let [target_h, target_s, target_v] = rgb_to_hsv(color.r() as f64, color.g() as f64, color.b() as f64);
+		let [target_h, target_s, target_v, target_a] = color.to_hsva();
+		let (target_h, target_s, target_v, target_a) = (target_h as f64, target_s as f64, target_v as f64, target_a as f64);
 
 		// Preserve hue: avoid jumping from 360° (top) to 0° (bottom) and don't reset hue when the color is desaturated or fully dark.
 		if !(target_h == 0. && self.hue == 1.) && target_s > 0. && target_v > 0. {
@@ -250,7 +256,7 @@ impl ColorPickerMessageHandler {
 			self.saturation = target_s;
 		}
 		self.value = target_v;
-		self.alpha = color.a() as f64;
+		self.alpha = target_a;
 		self.is_none = false;
 	}
 
@@ -264,9 +270,15 @@ impl ColorPickerMessageHandler {
 		{
 			*stop_color = color;
 			let stops = gradient.clone();
-			responses.add(FrontendMessage::ColorPickerColorChanged { value: FillChoice::Gradient(stops) });
+			let fill_choice = FillChoice::Gradient(stops);
+			responses.add(FrontendMessage::ColorPickerColorChanged {
+				value: FillChoiceUI::from(&fill_choice),
+			});
 		} else {
-			responses.add(FrontendMessage::ColorPickerColorChanged { value: FillChoice::Solid(color) });
+			let fill_choice = FillChoice::Solid(color);
+			responses.add(FrontendMessage::ColorPickerColorChanged {
+				value: FillChoiceUI::from(&fill_choice),
+			});
 		}
 	}
 
@@ -364,8 +376,9 @@ impl ColorPickerMessageHandler {
 		}
 
 		self.gradient = Some(gradient.clone());
+		let fill_choice = FillChoice::Gradient(gradient);
 		responses.add(FrontendMessage::ColorPickerColorChanged {
-			value: FillChoice::Gradient(gradient),
+			value: FillChoiceUI::from(&fill_choice),
 		});
 		self.send_layouts(responses);
 	}
@@ -392,7 +405,7 @@ impl ColorPickerMessageHandler {
 			// For gradient editing, the markers' handle colors mirror their gradient stop colors
 			let markers = gradient.iter().map(|stop| SpectrumMarker::new(stop.position, stop.midpoint, stop.color)).collect();
 			let mut row_widgets = vec![
-				SpectrumInput::new(gradient.clone())
+				SpectrumInput::new(GradientStopsUI::from(gradient))
 					.markers(markers)
 					.active_marker_index(self.active_marker_index)
 					.active_marker_is_midpoint(self.active_marker_is_midpoint)
@@ -453,7 +466,11 @@ impl ColorPickerMessageHandler {
 		let old_color = self.old_color();
 
 		let hex_value = new_color.map(|c| color_to_hex_optional_alpha(&c)).unwrap_or_else(|| "-".to_string());
-		let rgb_255 = new_color.map(|c| (c.r() as f64 * 255., c.g() as f64 * 255., c.b() as f64 * 255.));
+		// RGB readouts display sRGB byte values to the user, so we convert from linear-light to gamma here before quantizing.
+		let rgb_255 = new_color.map(|c| {
+			let [r, g, b, _] = c.to_gamma_srgb_channels();
+			(r as f64 * 255., g as f64 * 255., b as f64 * 255.)
+		});
 
 		// Epsilon comparison since the picker round-trips through HSV
 		let differs = match (new_color, old_color) {
@@ -470,7 +487,7 @@ impl ColorPickerMessageHandler {
 
 		// New/old comparison swatch with swap button
 		groups.push(LayoutGroup::row(vec![
-			ColorComparisonInput::new(new_color, old_color)
+			ColorComparisonInput::new(new_color.map(SRGBA8::from), old_color.map(SRGBA8::from))
 				.is_none(self.is_none)
 				.old_is_none(self.old_is_none)
 				.disabled(self.disabled)
@@ -566,7 +583,10 @@ impl ColorPickerMessageHandler {
 				.disabled(self.disabled)
 				.show_none_option(self.allow_none && self.gradient.is_none())
 				.on_update(|update: &ColorPresetsInputUpdate| match update {
-					ColorPresetsInputUpdate::Preset(fill_choice) => ColorPickerMessage::PickPreset { preset: fill_choice.clone() }.into(),
+					ColorPresetsInputUpdate::Preset(fill_choice) => ColorPickerMessage::PickPreset {
+						preset: FillChoice::from(fill_choice),
+					}
+					.into(),
 					ColorPresetsInputUpdate::EyedropperColorCode(code) => ColorPickerMessage::EyedropperColorCode { code: code.clone() }.into(),
 				})
 				.widget_instance(),
@@ -611,33 +631,10 @@ const SATURATION_DESCRIPTION: &str = "The vividness from grayscale to full color
 const VALUE_DESCRIPTION: &str = "The brightness from black to full color.";
 const ALPHA_DESCRIPTION: &str = "The level of translucency, from transparent (0%) to opaque (100%).";
 
-/// Convert an `rgb(0..1)` triple to `hsv(0..1)`. Mirrors the legacy frontend `colorToHSV`.
-fn rgb_to_hsv(red: f64, green: f64, blue: f64) -> [f64; 3] {
-	let max = red.max(green).max(blue);
-	let min = red.min(green).min(blue);
-	let delta = max - min;
-
-	let mut hue = if delta == 0. {
-		0.
-	} else if max == red {
-		((green - blue) / delta).rem_euclid(6.)
-	} else if max == green {
-		(blue - red) / delta + 2.
-	} else {
-		(red - green) / delta + 4.
-	};
-	hue = (hue * 60. + 360.).rem_euclid(360.) / 360.;
-
-	let saturation = if max == 0. { 0. } else { delta / max };
-	let value = max;
-
-	[hue, saturation, value]
-}
-
-/// The popover's background color (the `--color-2-mildblack` design token, `#222`). Used by the comparison swatch's
-/// outline computation to brighten the inset border for colors close to this background.
-const POPOVER_BACKGROUND: Color = Color::from_rgbaf32_unchecked(0x22 as f32 / 255., 0x22 as f32 / 255., 0x22 as f32 / 255., 1.);
-/// The luminance window (in linear-light) within which a color is considered close enough to the popover background
+/// The popover's background color as sRGB gamma-encoded channels (the `--color-2-mildblack` design token, `#222`).
+/// Used by the comparison swatch's outline computation to brighten the inset border for colors close to this background.
+const POPOVER_BACKGROUND_GAMMA_CHANNELS: [f32; 4] = [0x22 as f32 / 255., 0x22 as f32 / 255., 0x22 as f32 / 255., 1.];
+/// The luminance window within which a color is considered close enough to the popover background
 /// to warrant an outline. Mirrors the `proximityRange` argument the legacy frontend passed to `contrastingOutlineFactor`.
 const OUTLINE_PROXIMITY_RANGE: f64 = 0.01;
 
@@ -646,61 +643,22 @@ const OUTLINE_PROXIMITY_RANGE: f64 = 0.01;
 fn contrasting_outline_factor(color: Option<Color>) -> f64 {
 	let Some(color) = color else { return 0. };
 
-	// WCAG-style relative luminance, with alpha composited over white in gamma space
-	let luminance = |color: Color| {
-		// TODO: Remove the `.to_linear_srgb()` once we move to correctly treating `Color` as linear.
-		Color::WHITE
-			.alpha_blend(Color::from_unassociated_alpha(color.r(), color.g(), color.b(), color.a()))
-			.to_linear_srgb()
-			.luminance_srgb() as f64
+	// WCAG-style relative luminance, with alpha composited over white in sRGB gamma space (matching the perceptual intent of `SRGBA8::contrasting_text_color`).
+	let luminance_from_gamma_channels = |[r, g, b, a]: [f32; 4]| -> f64 {
+		let inv_a = 1. - a;
+		Color::from_gamma_srgb_channels(inv_a + r * a, inv_a + g * a, inv_a + b * a, 1.).luminance_rec_709() as f64
 	};
 
-	let distance = (luminance(POPOVER_BACKGROUND) - luminance(color)).abs().max(0.);
+	let color_gamma_channels = color.to_gamma_srgb_channels();
+	let distance = (luminance_from_gamma_channels(POPOVER_BACKGROUND_GAMMA_CHANNELS) - luminance_from_gamma_channels(color_gamma_channels))
+		.abs()
+		.max(0.);
 	let proximity = 1. - (distance / OUTLINE_PROXIMITY_RANGE).min(1.);
-	let [_, saturation, _] = rgb_to_hsv(color.r() as f64, color.g() as f64, color.b() as f64);
-	proximity * (1. - saturation)
+	let [_, saturation, _, _] = color.to_hsva();
+	proximity * (1. - saturation as f64)
 }
 
-/// Format a Color as a `#`-prefixed hex string, including the alpha component only if it's not fully opaque.
+/// Format a linear `Color` as a `#`-prefixed hex string, including the alpha component only if it's not fully opaque.
 fn color_to_hex_optional_alpha(color: &Color) -> String {
-	format!(
-		"#{}",
-		if color.a() >= 1. {
-			color.to_rgb_hex_srgb_from_gamma()
-		} else {
-			color.to_rgba_hex_srgb_from_gamma()
-		}
-	)
-}
-
-/// Parse a CSS color string (named color, hex, `rgb(...)`, etc.) into a `Color` using the `color` crate's CSS Color 4 parser.
-/// Tries the input as-is first (catches CSS named colors like `red`, `rgb(...)`, and well-formed hex like `#abcdef`), then falls back to treating the input as bare hex with length-based expansion to a CSS-parseable form:
-/// - 1 char `f` → `#fff` (CSS 3-char shorthand)
-/// - 2 char `ab` → `#ababab` (repeated to 6 chars)
-/// - 4 char `abcd` → `#00abcd` (left-padded with `00`)
-/// - 5 char `abcde` → `#0abcde` (left-padded with `0`)
-/// - 3, 6, 8 char inputs are passed through with a `#` prefix.
-fn parse_css_color(input: &str) -> Option<Color> {
-	let trimmed = input.trim();
-
-	let parsed = color::parse_color(trimmed).ok().or_else(|| {
-		let bare = trimmed.strip_prefix('#').unwrap_or(trimmed);
-		if bare.is_empty() || !bare.chars().all(|c| c.is_ascii_hexdigit()) {
-			return None;
-		}
-		let expanded = match bare.len() {
-			1 => bare.repeat(3),
-			2 => bare.repeat(3),
-			4 => format!("00{bare}"),
-			5 => format!("0{bare}"),
-			_ => bare.to_string(),
-		};
-		let candidate = format!("#{expanded}");
-		// Avoid retrying the exact same string we just failed to parse.
-		(candidate != trimmed).then(|| color::parse_color(&candidate).ok()).flatten()
-	})?;
-
-	let srgb: AlphaColor<Srgb> = parsed.to_alpha_color();
-	let [red, green, blue, alpha] = srgb.components;
-	Color::from_rgbaf32(red, green, blue, alpha)
+	SRGBA8::from(*color).to_css_hex()
 }

editor/src/messages/color_picker/color_picker_message_handler.rs

@@ -13,10 +13,10 @@ use crate::messages::portfolio::utility_types::WorkspacePanelLayout;
 use crate::messages::prelude::*;
 use crate::messages::tool::tool_messages::eyedropper_tool::PrimarySecondary;
 use graph_craft::document::NodeId;
+use graphene_std::color::SRGBA8;
 use graphene_std::raster::Image;
-use graphene_std::raster::color::Color;
 use graphene_std::text::Font;
-use graphene_std::vector::style::FillChoice;
+use graphene_std::vector::style::FillChoiceUI;
 use std::path::PathBuf;
 
 #[cfg(not(target_family = "wasm"))]
@@ -167,16 +167,16 @@ pub enum FrontendMessage {
 		document_id: DocumentId,
 	},
 	UpdateGradientStopColorPickerPosition {
-		color: Color, // TODO: Color (without `none`) -> Color (with `none`)
+		color: SRGBA8, // TODO: Color (without `none`) -> Color (with `none`)
 		position: (f64, f64),
 	},
-	/// The Rust color picker handler picked a new color/gradient. The frontend `<ColorPicker>` forwards this as its `colorOrGradient` event.
+	/// The Rust color picker handler picked a new color/gradient. The frontend `<ColorPicker />` forwards this as its `colorOrGradient` event.
 	ColorPickerColorChanged {
-		value: FillChoice,
+		value: FillChoiceUI,
 	},
-	/// The Rust color picker handler is starting an undo transaction. The frontend `<ColorPicker>` forwards this as its `startHistoryTransaction` event.
+	/// The Rust color picker handler is starting an undo transaction. The frontend `<ColorPicker />` forwards this as its `startHistoryTransaction` event.
 	ColorPickerStartHistoryTransaction,
-	/// The Rust color picker handler is committing the in-flight undo transaction. The frontend `<ColorPicker>` forwards this as its `commitHistoryTransaction` event.
+	/// The Rust color picker handler is committing the in-flight undo transaction. The frontend `<ColorPicker />` forwards this as its `commitHistoryTransaction` event.
 	ColorPickerCommitHistoryTransaction,
 	UpdateImportsExports {
 		/// If the primary import is not visible, then it is None.
@@ -241,7 +241,7 @@ pub enum FrontendMessage {
 		svg: String,
 	},
 	UpdateImageData {
-		image_data: Vec<(u64, Image<Color>)>,
+		image_data: Vec<(u64, Image<SRGBA8>)>,
 	},
 	UpdateDocumentLayerDetails {
 		data: LayerPanelEntry,

editor/src/messages/frontend/frontend_message.rs

@@ -14,7 +14,7 @@ pub struct DocumentInfo {
 	pub is_saved: bool,
 }
 
-#[cfg_attr(feature = "wasm", derive(tsify::Tsify), tsify(large_number_types_as_bigints))]
+#[cfg_attr(feature = "wasm", derive(tsify::Tsify), tsify(large_number_types_as_bigints, from_wasm_abi))]
 #[derive(Clone, Debug, Default, PartialEq, serde::Serialize, serde::Deserialize)]
 pub struct PersistedState {
 	pub documents: Vec<DocumentInfo>,

editor/src/messages/frontend/utility_types.rs

@@ -1,7 +1,8 @@
 use crate::messages::input_mapper::utility_types::input_keyboard::KeysGroup;
 use crate::messages::layout::utility_types::widget_prelude::*;
 use crate::messages::prelude::*;
-use graphene_std::vector::style::FillChoice;
+use graphene_std::color::SRGBA8;
+use graphene_std::vector::style::FillChoiceUI;
 use serde_json::Value;
 use std::collections::HashMap;
 
@@ -179,11 +180,11 @@ impl LayoutMessageHandler {
 				let callback_message = match action {
 					WidgetValueAction::Commit => (color_button.on_commit.callback)(&()),
 					WidgetValueAction::Update => {
-						let Ok(fill_choice) = serde_json::from_value::<FillChoice>(value) else {
-							warn!("ColorInput update was not able to be parsed as FillChoice: {color_button:?}");
+						let Ok(fill_choice_ui) = serde_json::from_value::<FillChoiceUI>(value) else {
+							warn!("ColorInput update was not able to be parsed as FillChoiceUI: {color_button:?}");
 							return;
 						};
-						color_button.value = fill_choice;
+						color_button.value = fill_choice_ui;
 						(color_button.on_update.callback)(color_button)
 					}
 				};
@@ -496,25 +497,14 @@ fn populate_computed_display_fields(layout: &mut Layout) {
 			}
 			Widget::SpectrumInput(spectrum_input) => {
 				spectrum_input.track_css = spectrum_input.track.to_css_linear_gradient();
-				spectrum_input.track_start_css = spectrum_input
-					.track
-					.color
-					.first()
-					.map(|color| format!("#{}", color.to_rgba_hex_srgb_from_gamma()))
-					.unwrap_or_else(|| "black".to_string());
-				spectrum_input.track_end_css = spectrum_input
-					.track
-					.color
-					.last()
-					.map(|color| format!("#{}", color.to_rgba_hex_srgb_from_gamma()))
-					.unwrap_or_else(|| "black".to_string());
+				spectrum_input.track_start_css = spectrum_input.track.color.first().map(|color| color.to_css_hex()).unwrap_or_else(|| "black".to_string());
+				spectrum_input.track_end_css = spectrum_input.track.color.last().map(|color| color.to_css_hex()).unwrap_or_else(|| "black".to_string());
 			}
 			Widget::ColorComparisonInput(comparison) => {
-				use graphene_std::Color;
-				let contrasting = |color: Option<Color>| format!("#{}", color.map_or(Color::BLACK, |color| color.contrasting_text_color_from_gamma()).to_rgba_hex_srgb_from_gamma());
-				comparison.new_color_css = comparison.new_color.map(|color| format!("#{}", color.to_rgba_hex_srgb_from_gamma())).unwrap_or_default();
+				let contrasting = |color: Option<SRGBA8>| color.map_or(SRGBA8::BLACK, |color| color.contrasting_text_color()).to_css_hex();
+				comparison.new_color_css = comparison.new_color.map(|color| color.to_css_hex()).unwrap_or_default();
 				comparison.new_color_contrasting = contrasting(comparison.new_color);
-				comparison.old_color_css = comparison.old_color.map(|color| format!("#{}", color.to_rgba_hex_srgb_from_gamma())).unwrap_or_default();
+				comparison.old_color_css = comparison.old_color.map(|color| color.to_css_hex()).unwrap_or_default();
 				comparison.old_color_contrasting = contrasting(comparison.old_color);
 			}
 			_ => {}