Add a "Use Tint" toggle to the 'Black & White' node and give its tint the Luminosity blend's clipping (#4540)

* Add a 'Use Tint' toggle to the 'Black & White' node and give its tint the Luminosity blend's clipping

* Fix the Black & White migration for a wired tint and gray out the tint when unused

editor/src/messages/portfolio/document/node_graph/node_properties.rs

@@ -1075,6 +1075,40 @@ pub fn optional_f64_widget(parameter_widgets_info: ParameterWidgetsInfo, bool_in
 	widgets
 }
 
+/// `parameter_widgets_info` is for the color parameter. `bool_input_index` is the input index of the bool parameter, drawn as a checkbox in front of the color.
+/// A color row gated by the bool input at `bool_input_index`, whose checkbox takes the assist slot after the label like the
+/// Opacity node's toggles, so the caller passes `blank_assist = false`. An exposed color shows neither, as in that node.
+pub fn optional_color_widget(parameter_widgets_info: ParameterWidgetsInfo, bool_input_index: usize, color_button: ColorInput) -> LayoutGroup {
+	let node_id = parameter_widgets_info.node_id;
+	let enabled = parameter_widgets_info
+		.document_node
+		.and_then(|document_node| document_node.inputs.get(bool_input_index))
+		.and_then(|input| input.as_non_exposed_value())
+		.and_then(|value| if let TaggedValue::Bool(enabled) = value { Some(*enabled) } else { None });
+	let label_count = start_widgets(&parameter_widgets_info).len();
+	let exposed = parameter_widgets_info.is_exposed();
+
+	let LayoutGroup::Row(mut row) = color_widget(parameter_widgets_info, color_button.disabled(enabled == Some(false))) else {
+		return LayoutGroup::row(Vec::new());
+	};
+	if let Some(enabled) = enabled
+		&& !exposed
+	{
+		let checkbox = [
+			Separator::new(SeparatorStyle::Unrelated).widget_instance(),
+			Separator::new(SeparatorStyle::Related).widget_instance(),
+			CheckboxInput::new(enabled)
+				.on_update(update_value_at_index(|x: &CheckboxInput| TaggedValue::Bool(x.checked), node_id, bool_input_index))
+				.on_commit(commit_value)
+				.widget_instance(),
+			Separator::new(SeparatorStyle::Related).widget_instance(),
+		];
+		row.widgets.splice(label_count..label_count, checkbox);
+	}
+
+	LayoutGroup::Row(row)
+}
+
 pub fn number_widget(parameter_widgets_info: ParameterWidgetsInfo, number_props: NumberInput) -> Vec<WidgetInstance> {
 	let mut widgets = start_widgets(&parameter_widgets_info);
 
@@ -2157,7 +2191,8 @@ pub(crate) fn black_and_white_properties(node_id: NodeId, context: &mut NodeProp
 
 	let number_input = NumberInput::default().mode_increment().unit("%").min(-200.).max(300.);
 
-	let tint = color_widget(ParameterWidgetsInfo::new(node_id, TintInput, true, context), ColorInput::default());
+	let use_tint: ParameterRef = UseTintInput.into();
+	let tint = optional_color_widget(ParameterWidgetsInfo::new(node_id, TintInput, false, context), use_tint.input_index, ColorInput::default());
 
 	let mut layout = vec![tint];
 	let params: &[(ParameterRef, Color, f64)] = &[

editor/src/messages/portfolio/document_migration.rs

@@ -2266,6 +2266,22 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId],
 		inputs_count = 51;
 	}
 
+	// Black & White gained a Use Tint toggle ahead of its tint color; a non-black tint used to be the only way to tint
+	if reference == DefinitionIdentifier::ProtoNode(graphene_std::raster::black_and_white::IDENTIFIER) && inputs_count == 8 {
+		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)?;
+		document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 0), old_inputs[0].clone(), network_path);
+		for (index, input) in old_inputs.iter().enumerate().skip(1).take(7) {
+			document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index + 1), input.clone(), network_path);
+		}
+		let use_tint = !matches!(old_inputs[1].as_value(), Some(TaggedValue::Color(color)) if *color == Color::BLACK);
+		document
+			.network_interface
+			.set_input(&InputConnector::node_at_index(*node_id, 1), NodeInput::value(TaggedValue::Bool(use_tint), false), network_path);
+		inputs_count = 9;
+	}
+
 	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);

node-graph/graph-craft/src/document/value.rs

@@ -644,9 +644,8 @@ impl TaggedValue {
 				});
 			}
 
-			// Hex syntax (e.g. "000000ff"), which a string literal default reaches here without its quotes
-			let hex = input.trim().trim_matches('"').trim().trim_start_matches('#');
-			let color = SRGBA8::from_hex_str(hex).map(Color::from);
+			// Hex syntax (e.g. "#1cd1ad", or "#1cd1ad70" with alpha), which a string literal default reaches here without its quotes
+			let color = input.trim().trim_matches('"').trim().strip_prefix('#').and_then(SRGBA8::from_hex_str).map(Color::from);
 			if color.is_none() {
 				log::error!("Invalid default value color string: {input}");
 			}
@@ -654,7 +653,7 @@ impl TaggedValue {
 		}
 
 		fn to_gradient(input: &str) -> Option<Gradient> {
-			// String syntax: (e.g. "000000ff, ff0000ff")
+			// String syntax: (e.g. "#000000ff, #ff0000ff")
 			let stops = input.split(',').filter_map(|s| to_color(s.trim())).collect::<Vec<_>>();
 			match stops.len() {
 				0 => {
@@ -1039,16 +1038,17 @@ mod paint_default_parsing {
 		);
 	}
 
-	/// A hex string default reaches the parser without the quotes its literal had in the node signature, and must still parse.
+	/// A hex string default reaches the parser without the quotes its literal had in the node signature, and must carry its hash prefix.
 	#[test]
-	fn hex_string_color_default_parses_without_quotes() {
+	fn hex_string_color_default_requires_its_hash_prefix() {
 		let tint = Some(TaggedValue::Color(Color::from(SRGBA8::new(225, 211, 179, 255))));
-		assert_eq!(TaggedValue::from_primitive_string("e1d3b3", &item!(Color)), tint, "a bare hex default should resolve");
 		assert_eq!(
 			TaggedValue::from_primitive_string("\"#e1d3b3\"", &item!(Color)),
 			tint,
 			"a quoted, hash-prefixed hex default should resolve"
 		);
+		assert_eq!(TaggedValue::from_primitive_string("#e1d3b3ff", &item!(Color)), tint, "an alpha-suffixed hex default should resolve");
+		assert_eq!(TaggedValue::from_primitive_string("e1d3b3", &item!(Color)), None, "a bare hex default should be rejected");
 	}
 
 	/// Table-era documents stored the red-slash "no paint" fill as an empty color table, which must keep

node-graph/nodes/raster/src/adjustments.rs

@@ -622,7 +622,8 @@ fn black_and_white<T: Adjust<Color>>(
 	#[implementations(Raster<CPU>, Color, Gradient)]
 	#[gpu_image]
 	image: Item<T>,
-	#[default(Color::BLACK)] tint: Item<Color>,
+	use_tint: Item<bool>,
+	#[default("#e1d3b3")] tint: Item<Color>,
 	#[default(40.)]
 	#[range]
 	#[soft(-200..300)]
@@ -650,6 +651,7 @@ fn black_and_white<T: Adjust<Color>>(
 ) -> Item<T> {
 	let mut image = image;
 	let tint = tint.into_element();
+	let use_tint = use_tint.into_element();
 	let reds = reds.into_element();
 	let yellows = yellows.into_element();
 	let greens = greens.into_element();
@@ -685,18 +687,17 @@ fn black_and_white<T: Adjust<Color>>(
 			yellow_part * yellows + (red_part - yellow_part) * reds + (green_part - yellow_part) * greens
 		};
 
-		let luminance = gray_base + additional;
+		let luminance = (gray_base + additional).clamp(0., 1.);
+		if !use_tint {
+			return Color::from_gamma_srgb_channels(luminance, luminance, luminance, alpha_part);
+		}
 
-		// TODO: Fix "Color" blend mode implementation so it matches the expected behavior perfectly (it's currently close)
-		// Apply luminance substitution in gamma space
-		let [tr, tg, tb, _] = tint.to_gamma_srgb_channels();
-		let tint_luma_rec_601 = 0.3 * tr + 0.59 * tg + 0.11 * tb;
-		let delta = luminance - tint_luma_rec_601;
-		let result_r = (tr + delta).clamp(0., 1.);
-		let result_g = (tg + delta).clamp(0., 1.);
-		let result_b = (tb + delta).clamp(0., 1.);
+		// The tint takes on the gray's luminosity the way the Luminosity blend mode would
+		let [tint_r, tint_g, tint_b, _] = tint.to_gamma_srgb_channels();
+		let tint_luma = luma_rec_601_fixed_point(tint_r, tint_g, tint_b);
+		let [tinted_r, tinted_g, tinted_b] = set_luminosity(tint_r, tint_g, tint_b, tint_luma, luminance);
 
-		Color::from_gamma_srgb_channels(result_r, result_g, result_b, alpha_part)
+		Color::from_gamma_srgb_channels(tinted_r, tinted_g, tinted_b, alpha_part)
 	});
 	image
 }
@@ -1911,7 +1912,7 @@ fn photo_filter<T: Adjust<Color>>(
 	#[implementations(Raster<CPU>, Color, Gradient)]
 	#[gpu_image]
 	image: Item<T>,
-	#[default("ec8a00")] color: Item<Color>,
+	#[default("#ec8a00")] color: Item<Color>,
 	#[default(25.)] density: Item<PercentageF32>,
 	#[default(true)] preserve_luminosity: Item<bool>,
 ) -> Item<T> {
@@ -1937,7 +1938,7 @@ fn photo_filter<T: Adjust<Color>>(
 		let mut b = linear_to_srgb(filtered[2].clamp(0., 1.));
 
 		if preserve_luminosity {
-			[r, g, b] = set_luminosity(r, g, b, luma_rec_601_fixed(r, g, b), luma_rec_601_fixed(r_in, g_in, b_in));
+			[r, g, b] = set_luminosity(r, g, b, luma_rec_601_fixed_point(r, g, b), luma_rec_601_fixed_point(r_in, g_in, b_in));
 		}
 
 		Color::from_gamma_srgb_channels(r, g, b, alpha)
@@ -1959,7 +1960,7 @@ fn multiply_matrix(matrix: &[[f32; 3]; 3], vector: [f32; 3]) -> [f32; 3] {
 }
 
 /// The Rec. 601 luma in the 14-bit fixed point that PSD interop depends on.
-fn luma_rec_601_fixed(r: f32, g: f32, b: f32) -> f32 {
+fn luma_rec_601_fixed_point(r: f32, g: f32, b: f32) -> f32 {
 	(4915. * r + 9667. * g + 1802. * b) / 16384.
 }
 
@@ -2160,6 +2161,61 @@ mod tests {
 		}
 	}
 
+	/// Runs Black & White with the default sliders on one gamma-space RGB value (0..255) and returns the gamma-space result on the same scale.
+	fn run_black_and_white(input: [f32; 3], tint: [f32; 3]) -> [f32; 3] {
+		let pixel = Color::from_gamma_srgb_channels(input[0] / 255., input[1] / 255., input[2] / 255., 1.);
+		let tint = Color::from_gamma_srgb_channels(tint[0] / 255., tint[1] / 255., tint[2] / 255., 1.);
+		let result = black_and_white(
+			(),
+			Item::new_from_element(pixel),
+			true.into(),
+			tint.into(),
+			40_f32.into(),
+			60_f32.into(),
+			40_f32.into(),
+			60_f32.into(),
+			20_f32.into(),
+			80_f32.into(),
+		);
+		let [r, g, b, _] = result.into_element().to_gamma_srgb_channels();
+		[r * 255., g * 255., b * 255.]
+	}
+
+	#[test]
+	fn black_and_white_tint_takes_the_grays_luminosity() {
+		for (input, tint, expected) in [
+			([200., 200., 200.], [225., 211., 179.], [213., 199., 167.]),
+			([50., 50., 50.], [225., 211., 179.], [63., 49., 17.]),
+			([200., 100., 50.], [225., 211., 179.], [133., 119., 87.]),
+			([200., 200., 200.], [30., 60., 120.], [176., 202., 255.]),
+			([50., 50., 50.], [30., 60., 120.], [22., 52., 112.]),
+			([200., 100., 50.], [30., 60., 120.], [92., 122., 182.]),
+		] {
+			let actual = run_black_and_white(input, tint);
+			for (actual, expected) in actual.iter().zip(expected) {
+				assert!((actual - expected).abs() <= 1., "{input:?} tinted {tint:?}: expected {expected}, got {actual}");
+			}
+		}
+	}
+
+	#[test]
+	fn black_and_white_clipped_channels_are_pulled_toward_the_luminosity() {
+		// A pure red tint over grays, where the shifted channels run out of range
+		for (gray, expected) in [
+			(1., [3.33, 0., 0.]),
+			(38., [126.67, 0., 0.]),
+			(75., [250.01, 0., 0.]),
+			(78., [255., 2.15, 2.15]),
+			(129., [255., 75., 75.]),
+			(200., [255., 176.43, 176.43]),
+		] {
+			let actual = run_black_and_white([gray, gray, gray], [255., 0., 0.]);
+			for (actual, expected) in actual.iter().zip(expected) {
+				assert!((actual - expected).abs() <= 0.51, "gray {gray} tinted red: expected {expected}, got {actual}");
+			}
+		}
+	}
+
 	/// Runs the node on one gamma-space RGB value (0..255) with the master sliders, colorize, and one range's sliders at
 	/// its default range values, returning the gamma-space result on the same scale.
 	fn run_hue_saturation(input: [f32; 3], master: [f32; 3], colorize: Option<[f32; 3]>, range: Option<(HueSaturationRange, [f32; 3])>) -> [f32; 3] {