From ecd2007e6328cd8cc3d5a16d912c51490deb7460 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 13 Sep 2026 03:02:27 -0700 Subject: [PATCH] Remove the 'Threshold' node's "Luminance Calculation" parameter (#4528) Remove the 'Threshold' node's luminance calculation dropdown so it always compares the Rec. 601 luma --- .../document/node_graph/node_properties.rs | 9 +-- .../messages/portfolio/document_migration.rs | 13 ++++ node-graph/nodes/raster/src/adjustments.rs | 62 +++++++++++++------ 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/editor/src/messages/portfolio/document/node_graph/node_properties.rs b/editor/src/messages/portfolio/document/node_graph/node_properties.rs index 4d8a7c451d..d1f120b163 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_properties.rs @@ -1647,16 +1647,9 @@ pub(crate) fn threshold_properties(node_id: NodeId, context: &mut NodeProperties let params: &[(ParameterRef, Color, f64)] = &[(MinLuminanceInput.into(), Color::BLACK, 50.), (MaxLuminanceInput.into(), Color::WHITE, 100.)]; - let mut layout = Vec::with_capacity(3); + let mut layout = Vec::with_capacity(2); build_shared_spectrum_section(node_id, context, params, &mut layout); - let luminance_calc = { - let mut info = ParameterWidgetsInfo::new(node_id, LuminanceCalcInput, true, context); - info.exposable = false; - enum_choice::().for_socket(info).property_row() - }; - layout.push(luminance_calc); - layout } diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index dfb2ed6125..d3b1ebf01d 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -2182,6 +2182,19 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], } } + // The Threshold node's luminance calculation input was retired + if reference == DefinitionIdentifier::ProtoNode(graphene_std::raster_nodes::adjustments::threshold::IDENTIFIER) && inputs_count == 4 { + let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); + document.network_interface.replace_implementation(node_id, network_path, &mut node_template); + + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + + for (i, input) in old_inputs.iter().enumerate().take(3) { + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, i), input.clone(), network_path); + } + inputs_count = 3; + } + if reference == DefinitionIdentifier::ProtoNode(graphene_std::repeat::repeat_on_points::IDENTIFIER) && inputs_count == 2 { let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); document.network_interface.replace_implementation(node_id, network_path, &mut node_template); diff --git a/node-graph/nodes/raster/src/adjustments.rs b/node-graph/nodes/raster/src/adjustments.rs index bec5d01eb1..0460cc3062 100644 --- a/node-graph/nodes/raster/src/adjustments.rs +++ b/node-graph/nodes/raster/src/adjustments.rs @@ -576,39 +576,61 @@ fn invert>( #[node_macro::node(category("Raster: Adjustment"), properties("threshold_properties"), shader_node(PerPixelAdjust))] fn threshold>( _: impl Ctx, - #[implementations( - Raster, - Color, - Gradient, - )] + #[implementations(Raster, Color, Gradient)] #[gpu_image] image: Item, #[default(50.)] min_luminance: Item, #[default(100.)] max_luminance: Item, - luminance_calc: Item, ) -> Item { let mut image = image; - let min_luminance = min_luminance.into_element(); - let max_luminance = max_luminance.into_element(); - let luminance_calc = luminance_calc.into_element(); + let min_luminance = min_luminance.into_element() / 100.; + let max_luminance = max_luminance.into_element() / 100.; image.element_mut().adjust(|color| { - let min_luminance = srgb_to_linear(min_luminance / 100.); - let max_luminance = srgb_to_linear(max_luminance / 100.); + // For PSD interop, we compare this 14-bit fixed-point Rec. 601 luma against the level unrounded + let [r, g, b, _] = color.to_gamma_srgb_channels(); + let luminance = (4915. * r + 9667. * g + 1802. * b) / 16384.; - let luminance = match luminance_calc { - LuminanceCalculation::SRGB => color.luminance_rec_709(), - LuminanceCalculation::Perceptual => color.luminance_perceptual(), - LuminanceCalculation::AverageChannels => color.average_rgb_channels(), - LuminanceCalculation::MinimumChannels => color.minimum_rgb_channels(), - LuminanceCalculation::MaximumChannels => color.maximum_rgb_channels(), - }; - - if luminance >= min_luminance && luminance <= max_luminance { Color::WHITE } else { Color::BLACK } + let output = if luminance >= min_luminance && luminance <= max_luminance { Color::WHITE } else { Color::BLACK }; + output.with_alpha(color.a()) }); image } +#[cfg(all(feature = "std", test))] +mod threshold_tests { + use super::*; + + /// Whether one gamma-space RGB value (0..255) ends up white at the given threshold level (0..255). + fn is_white(input: [f32; 3], level: f32) -> bool { + let pixel = Color::from_gamma_srgb_channels(input[0] / 255., input[1] / 255., input[2] / 255., 1.); + let result = threshold((), Item::new_from_element(pixel), (level / 255. * 100.).into(), 100_f32.into()); + result.into_element().r() == 1. + } + + #[test] + fn rec_601_luma_is_compared_as_an_8_bit_level() { + assert!(!is_white([200., 100., 40.], 128.)); + assert!(!is_white([125., 130., 120.], 128.)); + assert!(is_white([0., 255., 0.], 128.)); + assert!(!is_white([255., 0., 0.], 128.)); + assert!(is_white([128., 128., 128.], 128.)); + assert!(!is_white([127., 127., 127.], 128.)); + assert!(is_white([200., 100., 40.], 123.)); + assert!(!is_white([200., 100., 40.], 124.)); + } + + #[test] + fn ties_follow_the_unrounded_fixed_point_luma() { + // Half-level lumas in 0.3/0.59/0.11 stay below the level either way, and the 14-bit weights pull a whole-level red or blue luma just under it + assert!(!is_white([189., 120., 0.], 128.)); + assert!(!is_white([248., 90., 0.], 128.)); + assert!(!is_white([135., 100., 0.], 100.)); + assert!(!is_white([255., 0., 50.], 82.)); + assert!(is_white([0., 200., 0.], 118.)); + } +} + // Aims for interoperable compatibility with: // https://www.adobe.com/devnet-apps/photoshop/fileformatashtml/#:~:text=%27grdm%27%20%3D%20Gradient%20Map // https://www.adobe.com/devnet-apps/photoshop/fileformatashtml/#:~:text=Gradient%20settings%20(Photoshop%206.0)