From 107e55e91d4cacf64b9ad22b8a13915f86e3ffd5 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 21 Feb 2023 15:42:13 -0800 Subject: [PATCH] Improve the Vibrance node (#1050) * Fix vibrance * Improve results * Clean up implementation * Fix warnings from unrelated commit * Improve negative vibrance --- .../messages/tool/tool_messages/path_tool.rs | 4 +- node-graph/gcore/src/raster/adjustments.rs | 56 +++++++++++++++++-- node-graph/gcore/src/raster/color.rs | 2 +- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index 8c818033..74bc5a0d 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -6,9 +6,7 @@ use crate::messages::prelude::*; use crate::messages::tool::common_functionality::overlay_renderer::OverlayRenderer; use crate::messages::tool::common_functionality::shape_editor::{ManipulatorPointInfo, ShapeEditor}; use crate::messages::tool::common_functionality::snapping::SnapManager; -use crate::messages::tool::common_functionality::transformation_cage::axis_align_drag; -use crate::messages::tool::utility_types::{EventToMessageMap, Fsm, ToolActionHandlerData, ToolMetadata, ToolTransition, ToolType}; -use crate::messages::tool::utility_types::{HintData, HintGroup, HintInfo}; +use crate::messages::tool::utility_types::{EventToMessageMap, Fsm, HintData, HintGroup, HintInfo, ToolActionHandlerData, ToolMetadata, ToolTransition, ToolType}; use document_legacy::intersection::Quad; use graphene_std::vector::consts::ManipulatorType; diff --git a/node-graph/gcore/src/raster/adjustments.rs b/node-graph/gcore/src/raster/adjustments.rs index 6c7724e5..4450a0cf 100644 --- a/node-graph/gcore/src/raster/adjustments.rs +++ b/node-graph/gcore/src/raster/adjustments.rs @@ -216,13 +216,61 @@ pub struct VibranceNode { vibrance: Vibrance, } -// TODO: The current results are incorrect, try implementing this from https://stackoverflow.com/questions/33966121/what-is-the-algorithm-for-vibrance-filters +// From https://stackoverflow.com/questions/33966121/what-is-the-algorithm-for-vibrance-filters +// The results of this implementation are very close to correct, but not quite perfect #[node_macro::node_fn(VibranceNode)] fn vibrance_node(color: Color, vibrance: f64) -> Color { - let [hue, saturation, lightness, alpha] = color.to_hsla(); + // TODO: Remove conversion to linear when the whole node graph uses linear color + let color = color.to_linear_srgb(); + let vibrance = vibrance as f32 / 100.; - let saturation = saturation + vibrance * (1. - saturation); - Color::from_hsla(hue, saturation, lightness, alpha) + // Slow the effect down by half when it's negative, since artifacts begin appearing past -50%. + // So this scales the 0% to -50% range to 0% to -100%. + let slowed_vibrance = if vibrance >= 0. { vibrance } else { vibrance * 0.5 }; + + let channel_max = color.r().max(color.g()).max(color.b()); + let channel_min = color.r().min(color.g()).min(color.b()); + let channel_difference = channel_max - channel_min; + + let scale_multiplier = if channel_max == color.r() { + let green_blue_difference = (color.g() - color.b()).abs(); + let t = (green_blue_difference / channel_difference).min(1.); + t * 0.5 + 0.5 + } else { + 1. + }; + let scale = slowed_vibrance * scale_multiplier * (2. - channel_difference); + let channel_reduction = channel_min * scale; + let scale = 1. + scale * (1. - channel_difference); + + let luminance_initial = color.to_linear_srgb().luminance_srgb(); + let altered_color = color.map_rgb(|channel| (channel * scale - channel_reduction)).to_linear_srgb(); + let luminance = altered_color.luminance_srgb(); + let altered_color = altered_color.map_rgb(|channel| channel * luminance_initial / luminance); + + let channel_max = altered_color.r().max(altered_color.g()).max(altered_color.b()); + let altered_color = if Color::linear_to_srgb(channel_max) > 1. { + let scale = (1. - luminance) / (channel_max - luminance); + altered_color.map_rgb(|channel| (channel - luminance) * scale + luminance) + } else { + altered_color + }; + let altered_color = altered_color.to_gamma_srgb(); + + let altered_color = if vibrance >= 0. { + altered_color + } else { + // TODO: The result ends up a bit darker than it should be, further investigation is needed + let luminance = color.luminance_rec_601(); + + // Near -0% vibrance we mostly use `altered_color`. + // Near -100% vibrance, we mostly use half the desaturated luminance color and half `altered_color`. + let factor = -slowed_vibrance; + altered_color.map_rgb(|channel| channel * (1. - factor) + luminance * factor) + }; + + // TODO: Remove conversion to linear when the whole node graph uses linear color + altered_color.to_gamma_srgb() } #[derive(Debug, Clone, Copy)] diff --git a/node-graph/gcore/src/raster/color.rs b/node-graph/gcore/src/raster/color.rs index ff8f94c3..210e8ab4 100644 --- a/node-graph/gcore/src/raster/color.rs +++ b/node-graph/gcore/src/raster/color.rs @@ -235,7 +235,7 @@ impl Color { } pub fn with_luminance(&self, luminance: f32) -> Color { - let d = luminance - self.luminance_rec_601(); + let d = luminance - self.luminance_rec_601_rounded(); self.map_rgb(|c| (c + d).clamp(0., 1.)) }