Skip to content

Fix style controls corrupting stack nodes like Transform that can't host blending or paint nodes #4428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Keavon merged 5 commits into from
Aug 11, 2026
Merged

Fix style controls corrupting stack nodes like Transform that can't host blending or paint nodes #4428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
097ce92
Stop chain node insertion from stranding a node on layers without a c…
Keavon Aug 11, 2026
38290e2
Gate the Layers panel blending controls on whether the layer can host…
Keavon Aug 11, 2026
07f5a7c
Gate the fill and stroke setters on whether the layer can host paint …
Keavon Aug 11, 2026
57f07c7
Make the tool options fill and stroke controls treat layers that cann…
Keavon Aug 11, 2026
c31d031
Remove the unused first_selected_stroke_weight helper
Keavon Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions editor/src/messages/portfolio/document/document_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3409,6 +3409,11 @@ impl DocumentMessageHandler {
let selected_nodes = self.network_interface.selected_nodes();
let selected_layers_except_artboards = selected_nodes.selected_layers_except_artboards(&self.network_interface);

// A layer whose chain cannot carry blending nodes has nowhere to put the value, so it disqualifies the whole selection
let all_layers_support_blending = selected_nodes
.selected_layers_except_artboards(&self.network_interface)
.all(|layer| self.network_interface.layer_hosts_blending_nodes(&layer.to_node(), &[]));

// Look up the current opacity and blend mode of the selected layers (if any), and split the iterator into the first tuple and the rest.
let mut blending_options = selected_layers_except_artboards.map(|layer| {
(
Expand All @@ -3420,8 +3425,8 @@ impl DocumentMessageHandler {
let first_blending_options = blending_options.next();
let result_blending_options = blending_options;

// If there are no selected layers, disable the opacity and blend mode widgets.
let disabled = first_blending_options.is_none();
// If there are no selected layers, or any of them cannot host the nodes, disable the opacity and blend mode widgets.
let disabled = first_blending_options.is_none() || !all_layers_support_blending;

// Amongst the selected layers, check if the opacities and blend modes are identical across all layers.
// The result is setting `option` and `blend_mode` to Some value if all their values are identical, or None if they are not.
Expand Down
29 changes: 21 additions & 8 deletions editor/src/messages/portfolio/document/graph_operation/utility_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,16 @@ impl<'a> ModifyInputsContext<'a> {
self.existing_node_id(&DefinitionIdentifier::ProtoNode(reference), create_if_nonexistent)
}

/// The same as [`Self::existing_proto_node_id`], but yielding `None` on layers whose chain cannot host the node.
fn existing_chain_hosted_node_id(&mut self, reference: ProtoNodeIdentifier, create_if_nonexistent: bool) -> Option<NodeId> {
let output_layer = self.get_output_layer()?;
if !self.network_interface.layer_chain_hosts_node(&output_layer.to_node(), &[], &reference) {
return None;
}

self.existing_proto_node_id(reference, create_if_nonexistent)
}

/// Gets the node id of a document node with a specific reference that is upstream from the layer node, and optionally creates it if it does not exist.
fn existing_node_id(&mut self, reference: &DefinitionIdentifier, create_if_nonexistent: bool) -> Option<NodeId> {
// Start from the layer node or export
Expand Down Expand Up @@ -397,6 +407,9 @@ impl<'a> ModifyInputsContext<'a> {
return None;
};

// Without a secondary input there is no chain to hold the node, so inserting it would strand it at the graph origin
self.network_interface.input_from_connector(&InputConnector::layer_secondary_input(output_layer.to_node()), &[])?;

// If inserting a 'Path' node, insert a 'Combine Paths' node if the type is `Graphic`.
// TODO: Allow the 'Path' node to operate on `List` data by utilizing the reference (index or ID?) for each item.
if node_definition.identifier == "Path" {
Expand All @@ -418,7 +431,7 @@ impl<'a> ModifyInputsContext<'a> {
}

pub fn fill_color_set(&mut self, color: Option<Color>) {
let Some(fill_node_id) = self.existing_proto_node_id(graphene_std::vector_nodes::fill::IDENTIFIER, true) else {
let Some(fill_node_id) = self.existing_chain_hosted_node_id(graphene_std::vector_nodes::fill::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(fill_node_id, graphene_std::vector::fill::FillInput);
Expand All @@ -433,7 +446,7 @@ impl<'a> ModifyInputsContext<'a> {
}

pub fn fill_gradient_set(&mut self, gradient: Gradient, gradient_form: GradientForm, settings: GradientSettings, transform: DAffine2) {
let Some(fill_node_id) = self.existing_proto_node_id(graphene_std::vector_nodes::fill::IDENTIFIER, true) else {
let Some(fill_node_id) = self.existing_chain_hosted_node_id(graphene_std::vector_nodes::fill::IDENTIFIER, true) else {
return;
};
let backup_input_connector = InputConnector::node(fill_node_id, graphene_std::vector::fill::BackupGradientInput);
Expand Down Expand Up @@ -477,15 +490,15 @@ impl<'a> ModifyInputsContext<'a> {
}

pub fn blend_mode_set(&mut self, blend_mode: BlendMode) {
let Some(blend_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::blend_mode::IDENTIFIER, true) else {
let Some(blend_node_id) = self.existing_chain_hosted_node_id(graphene_std::blending_nodes::blend_mode::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(blend_node_id, graphene_std::blending_nodes::blend_mode::BlendModeInput);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::BlendMode(blend_mode), false), false);
}

pub fn opacity_set(&mut self, opacity: f64) {
let Some(opacity_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::opacity::IDENTIFIER, true) else {
let Some(opacity_node_id) = self.existing_chain_hosted_node_id(graphene_std::blending_nodes::opacity::IDENTIFIER, true) else {
return;
};
// Enable the `has_opacity` checkbox so the value is applied
Expand All @@ -504,9 +517,9 @@ impl<'a> ModifyInputsContext<'a> {
pub fn opacity_fill_set(&mut self, fill: f64) {
// Reuse an existing Opacity node to avoid a redundant chain walk on slider drags
let identifier = graphene_std::blending_nodes::opacity::IDENTIFIER;
let existing = self.existing_proto_node_id(identifier.clone(), false);
let existing = self.existing_chain_hosted_node_id(identifier.clone(), false);
let existed = existing.is_some();
let Some(opacity_node_id) = existing.or_else(|| self.existing_proto_node_id(identifier, true)) else {
let Some(opacity_node_id) = existing.or_else(|| self.existing_chain_hosted_node_id(identifier, true)) else {
return;
};
// Freshly-created node defaults to opacity enabled; disable it so the fill slider works independently
Expand Down Expand Up @@ -820,15 +833,15 @@ impl<'a> ModifyInputsContext<'a> {

pub fn clip_mode_toggle(&mut self, clip_mode: Option<bool>) {
let clip = !clip_mode.unwrap_or(false);
let Some(clip_node_id) = self.existing_proto_node_id(graphene_std::blending_nodes::clipping_mask::IDENTIFIER, true) else {
let Some(clip_node_id) = self.existing_chain_hosted_node_id(graphene_std::blending_nodes::clipping_mask::IDENTIFIER, true) else {
return;
};
let input_connector = InputConnector::node(clip_node_id, graphene_std::blending_nodes::clipping_mask::ClipInput);
self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::Bool(clip), false), false);
}

pub fn stroke_set(&mut self, color: Option<Color>, stroke: Stroke) {
let Some(stroke_node_id) = self.existing_proto_node_id(graphene_std::vector::stroke::IDENTIFIER, true) else {
let Some(stroke_node_id) = self.existing_chain_hosted_node_id(graphene_std::vector::stroke::IDENTIFIER, true) else {
return;
};

Expand Down
2 changes: 1 addition & 1 deletion editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2852,7 +2852,7 @@ impl NodeGraphMessageHandler {
}))
);

let clippable = layer.can_be_clipped(network_interface.document_metadata());
let clippable = layer.can_be_clipped(network_interface.document_metadata()) && network_interface.layer_hosts_blending_nodes(&node_id, &[]);

let data = LayerPanelEntry {
id: node_id,
Expand Down
58 changes: 49 additions & 9 deletions editor/src/messages/portfolio/document/utility_types/network_interface/resolved_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet};
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{DocumentNodeImplementation, InlineRust, NodeInput};
use graph_craft::proto::{GraphErrorType, GraphErrors};
use graph_craft::{Type, concrete};
use graph_craft::{ProtoNodeIdentifier, Type, concrete};
use graphene_std::uuid::NodeId;
use interpreted_executor::dynamic_executor::{NodeTypes, ResolvedDocumentNodeTypesDelta};
use interpreted_executor::node_registry::NODE_REGISTRY;
Expand Down Expand Up @@ -63,15 +63,14 @@ impl TypeSource {
self.compiled_nested_type().is_some_and(|ty| matches!(ty, Type::List(_)) || ty.bundle_element_name().is_some())
}

/// The element type's identifier name with any rank-0 `Item` or rank-1 `List` wrapper peeled, so semantic type checks can be rank-agnostic.
/// The element type with any rank-0 `Item` or rank-1 `List` wrapper peeled, so semantic type checks can be rank-agnostic.
pub fn compiled_element_type(&self) -> Option<&Type> {
Some(element_of(self.compiled_nested_type()?))
}

/// The identifier name of [`Self::compiled_element_type`].
pub fn compiled_element_name(&self) -> Option<String> {
let nested_type = self.compiled_nested_type()?;
// A rank-0 `Item` or rank-1 `List` peels to its element; a bare value reports itself
let element = match nested_type {
Type::Item(element) | Type::List(element) => element.as_ref(),
other => other,
};
Some(element.identifier_name())
Some(self.compiled_element_type()?.identifier_name())
}

pub fn compiled_nested_type(&self) -> Option<&Type> {
Expand Down Expand Up @@ -110,6 +109,14 @@ impl TypeSource {
}
}

/// Peels any `Fn`/`Future` wrapper, then any rank-0 `Item` or rank-1 `List` wrapper, down to the element type.
fn element_of(ty: &Type) -> &Type {
match ty.nested_type() {
Type::Item(element) | Type::List(element) => element.as_ref(),
other => other,
}
}

impl NodeNetworkInterface {
fn input_has_error(&self, input_connector: &InputConnector, network_path: &[NodeId]) -> bool {
match input_connector {
Expand Down Expand Up @@ -169,6 +176,39 @@ impl NodeNetworkInterface {
}
}

/// Whether the given node has a registered implementation accepting the layer chain's element type as its content input.
/// A chain awaiting compilation has no resolved type yet, so only a known-wrong type or a type error locks the layer out.
pub fn layer_chain_hosts_node(&self, node_id: &NodeId, network_path: &[NodeId], node: &ProtoNodeIdentifier) -> bool {
let secondary_input = InputConnector::layer_secondary_input(*node_id);
if !self.input_from_connector(&secondary_input, network_path).is_some_and(|input| input.is_exposed()) {
return false;
}

let chain_type = self.input_type(&secondary_input, network_path);
match chain_type.compiled_element_type() {
Some(element) => {
let Some(implementations) = NODE_REGISTRY.get(node) else {
log::error!("Proto node {node:?} not found in the node registry, in layer_chain_hosts_node");
return false;
};
implementations.keys().any(|node_io| node_io.inputs.first().is_some_and(|content| element_of(content) == element))
Comment thread
Keavon marked this conversation as resolved.
}
None => !matches!(chain_type, TypeSource::Invalid),
}
}

/// Whether the blending nodes (blend mode, opacity, clipping mask) can be spliced into this layer's chain.
pub fn layer_hosts_blending_nodes(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool {
// Blend Mode stands in for the trio since they share one implementations list
self.layer_chain_hosts_node(node_id, network_path, &graphene_std::blending_nodes::blend_mode::IDENTIFIER)
}

/// Whether the Fill and Stroke nodes can be spliced into this layer's chain.
pub fn layer_hosts_paint_nodes(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool {
// Fill stands in for both since they share one implementations list
self.layer_chain_hosts_node(node_id, network_path, &graphene_std::vector_nodes::fill::IDENTIFIER)
}

/// Get the [`TypeSource`] for any InputConnector.
/// If the input is not compiled, then an Unknown or default from the definition is returned.
pub fn input_type(&self, input_connector: &InputConnector, network_path: &[NodeId]) -> TypeSource {
Expand Down
35 changes: 14 additions & 21 deletions editor/src/messages/tool/common_functionality/color_selector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,8 @@ pub fn sync_drawing_state(drawing: &mut DrawingToolState, natural_fill_enabled:
/// Reads the stroke proto-node inputs (align, cap, join, miter limit, paint order, dash lengths, dash offset) across the selection and updates
/// the matching fields on `drawing`. Each field becomes `None` (mixed) when selected strokes disagree. With no selection, fields are left as-is.
fn sync_stroke_options(drawing: &mut DrawingToolState, document: &DocumentMessageHandler) -> bool {
let strokes: Vec<_> = document
.network_interface
.selected_nodes()
.selected_layers_except_artboards(&document.network_interface)
let strokes: Vec<_> = graph_modification_utils::paintable_selected_layers(document)
.into_iter()
.filter_map(|layer| graph_modification_utils::get_stroke_options(layer, &document.network_interface))
.collect();
if strokes.is_empty() {
Expand Down Expand Up @@ -391,14 +389,9 @@ pub fn sync_fill_only(fill: &mut ToolColorOptions, natural_fill_enabled: bool, f
}
}

/// True if at least one (non-artboard) layer is currently selected.
pub fn has_selection(document: &DocumentMessageHandler) -> bool {
document
.network_interface
.selected_nodes()
.selected_layers_except_artboards(&document.network_interface)
.next()
.is_some()
/// True if at least one selected layer can take paint, making the swatches edit the selection instead of the tool's own colors.
pub fn has_paintable_selection(document: &DocumentMessageHandler) -> bool {
!graph_modification_utils::paintable_selected_layers(document).is_empty()
}

/// Applies a user-picked fill (gradient or solid). With a selection, writes to the layers; with none, pushes a solid to the swap-routed working color slot.
Expand All @@ -411,7 +404,7 @@ pub fn apply_fill_only_color_pick(fill: &mut ToolColorOptions, fill_choice: Fill
fill.fill_choice = Some(fill_choice.clone());
fill.enabled = Some(true);
fill.tracks_working_color = false;
if has_selection(document) {
if has_paintable_selection(document) {
if document.network_interface.transaction_status() == TransactionStatus::Finished {
responses.add(DocumentMessage::StartTransaction);
}
Expand All @@ -426,7 +419,7 @@ pub fn apply_stroke_color_pick(drawing: &mut DrawingToolState, color: Option<Col
drawing.stroke.fill_choice = Some(color.map_or(FillChoice::None, FillChoice::Solid));
drawing.stroke.enabled = Some(true);
drawing.stroke.tracks_working_color = false;
if has_selection(document) {
if has_paintable_selection(document) {
if document.network_interface.transaction_status() == TransactionStatus::Finished {
responses.add(DocumentMessage::StartTransaction);
}
Expand All @@ -447,7 +440,7 @@ pub fn apply_fill_enabled(drawing: &mut DrawingToolState, enabled: bool, global:
/// Single-slot variant of [`apply_fill_enabled`]. `working_color` is the fallback used when re-ticking or unticking from a mixed state.
pub fn apply_fill_only_enabled(fill: &mut ToolColorOptions, enabled: bool, working_color: Color, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
fill.enabled = Some(enabled);
if has_selection(document) {
if has_paintable_selection(document) {
responses.add(DocumentMessage::AddTransaction);
}
if enabled {
Expand All @@ -471,7 +464,7 @@ pub fn apply_fill_only_enabled(fill: &mut ToolColorOptions, enabled: bool, worki
/// Toggles the stroke checkbox: mirrors [`apply_fill_enabled`].
pub fn apply_stroke_enabled(drawing: &mut DrawingToolState, enabled: bool, global: &DocumentToolData, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
drawing.stroke.enabled = Some(enabled);
if has_selection(document) {
if has_paintable_selection(document) {
responses.add(DocumentMessage::AddTransaction);
}
if enabled {
Expand All @@ -493,7 +486,7 @@ pub fn apply_stroke_enabled(drawing: &mut DrawingToolState, enabled: bool, globa
/// Applies a user-edited stroke weight to the selection, also persisting it as the no-selection default.
pub fn apply_line_weight(drawing: &mut DrawingToolState, line_weight: f64, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
drawing.line_weight = Some(line_weight);
if !has_selection(document) {
if !has_paintable_selection(document) {
drawing.default_line_weight = line_weight;
}
graph_modification_utils::set_stroke_weight_for_selected_layers(line_weight, document, responses);
Expand All @@ -507,7 +500,7 @@ pub fn apply_working_colors(drawing: &mut DrawingToolState, global: &DocumentToo

/// Refreshes a single swatch from the given working color, subject to the rules in [`apply_working_colors`].
pub fn refresh_slot_working_color(slot: &mut ToolColorOptions, working_color: Color, document: &DocumentMessageHandler) {
if slot.fill_choice.is_some() && (!has_selection(document) || slot.tracks_working_color) {
if slot.fill_choice.is_some() && (!has_paintable_selection(document) || slot.tracks_working_color) {
slot.fill_choice = Some(solid(working_color));
}
}
Expand All @@ -524,7 +517,7 @@ pub fn reset_colors_on_deactivation(drawing: &mut DrawingToolState, global: &Doc
pub fn swap_fill_and_stroke(drawing: &mut DrawingToolState, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
drawing.colors_swapped = !drawing.colors_swapped;

if has_selection(document) {
if has_paintable_selection(document) {
responses.add(DocumentMessage::AddTransaction);
}

Expand All @@ -539,7 +532,7 @@ pub fn swap_fill_and_stroke(drawing: &mut DrawingToolState, document: &DocumentM
drawing.fill.tracks_working_color = new_fill_tracks;
drawing.stroke.tracks_working_color = new_stroke_tracks;

if has_selection(document) {
if has_paintable_selection(document) {
// Apply to layers only when we have a concrete value (`None` means mixed, no single value to broadcast).
if drawing.fill.is_active()
&& let Some(choice) = new_fill
Expand Down Expand Up @@ -579,7 +572,7 @@ pub enum WeightSyncOutcome {

/// Inspects the selection and returns how the weight widget should update.
pub fn compute_weight_sync(document: &DocumentMessageHandler) -> WeightSyncOutcome {
let layers: Vec<_> = document.network_interface.selected_nodes().selected_layers_except_artboards(&document.network_interface).collect();
let layers = graph_modification_utils::paintable_selected_layers(document);

if layers.is_empty() {
return WeightSyncOutcome::NoSelection;
Expand Down
Loading
Loading