From 9b034a5194c17b538fa89e8aa8f226769c65ebac Mon Sep 17 00:00:00 2001 From: 0HyperCube <78500760+0HyperCube@users.noreply.github.com> Date: Wed, 25 Oct 2023 23:20:55 +0100 Subject: [PATCH] Fix deleting layers (#1441) * Fix deleting layers * Code review comments --- document-legacy/src/document_metadata.rs | 2 +- .../document/document_message_handler.rs | 40 ++++++++++++++----- editor/src/messages/portfolio/document/mod.rs | 2 +- .../node_graph/node_graph_message_handler.rs | 31 ++++++++++---- .../document/utility_types/transformation.rs | 2 +- .../portfolio/portfolio_message_handler.rs | 21 ++++++++-- .../tool/tool_messages/select_tool.rs | 8 ++-- 7 files changed, 77 insertions(+), 29 deletions(-) diff --git a/document-legacy/src/document_metadata.rs b/document-legacy/src/document_metadata.rs index e1d70f99..46c826bd 100644 --- a/document-legacy/src/document_metadata.rs +++ b/document-legacy/src/document_metadata.rs @@ -77,7 +77,7 @@ impl DocumentMetadata { self.structure.entry(node_identifier).or_default() } - pub fn shallowest_unique_layers<'a>(&self, layers: impl Iterator) -> Vec> { + pub fn shallowest_unique_layers(&self, layers: impl Iterator) -> Vec> { let mut sorted_layers = layers .map(|layer| { let mut layer_path = layer.ancestors(self).collect::>(); diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index 27df3f0b..56652e03 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -100,14 +100,24 @@ impl Default for DocumentMessageHandler { } } -impl MessageHandler for DocumentMessageHandler { +pub struct DocumentInputs<'a> { + pub document_id: u64, + pub ipp: &'a InputPreprocessorMessageHandler, + pub persistent_data: &'a PersistentData, + pub preferences: &'a PreferencesMessageHandler, + pub executor: &'a mut NodeGraphExecutor, +} + +impl MessageHandler> for DocumentMessageHandler { #[remain::check] - fn process_message( - &mut self, - message: DocumentMessage, - responses: &mut VecDeque, - (document_id, ipp, persistent_data, preferences, executor): (u64, &InputPreprocessorMessageHandler, &PersistentData, &PreferencesMessageHandler, &mut NodeGraphExecutor), - ) { + fn process_message(&mut self, message: DocumentMessage, responses: &mut VecDeque, document_inputs: DocumentInputs) { + let DocumentInputs { + document_id, + ipp, + persistent_data, + preferences, + executor, + } = document_inputs; use DocumentMessage::*; let render_data = RenderData::new(&persistent_data.font_cache, self.view_mode, Some(ipp.document_bounds())); @@ -300,8 +310,10 @@ impl MessageHandler ActionList { + unimplemented!("Must use `actions_with_graph_open` instead (unless we change every implementation of the MessageHandler trait).") + } +} + +impl DocumentMessageHandler { + pub fn actions_with_graph_open(&self, graph_open: bool) -> ActionList { let mut common = actions!(DocumentMessageDiscriminant; Undo, Redo, @@ -922,7 +940,7 @@ impl MessageHandler = None; + let mut reconnect_to_input: Option = None; if reconnect { // Check whether the being-deleted node's first (primary) input is a node if let Some(node) = network.nodes.get(&deleting_node_id) { - if matches!(node.inputs.first(), Some(NodeInput::Node { .. })) { - first_input_node = Some(node.inputs[0].clone()); + // Reconnect to the node below when deleting a layer node. + let reconnect_from_input_index = if node.name == "Layer" { 7 } else { 0 }; + if matches!(&node.inputs.get(reconnect_from_input_index), Some(NodeInput::Node { .. })) { + reconnect_to_input = Some(node.inputs[reconnect_from_input_index].clone()); } } } @@ -385,10 +387,10 @@ impl NodeGraphMessageHandler { // Use the first input node as the new input if deleting node's first input is a node, // and the current node uses its primary output too - if let Some(input_node) = &first_input_node { + if let Some(reconnect_to_input) = &reconnect_to_input { if *output_index == 0 { refers_to_output_node = true; - *input = input_node.clone() + *input = reconnect_to_input.clone() } } @@ -481,7 +483,6 @@ impl<'a> MessageHandler> for NodeGrap error!("Failed to find actual index of connector index {input_node_connector_index} on node {input_node:#?}"); return; }; - document.metadata.load_structure(&document.document_network); responses.add(DocumentMessage::DocumentStructureChanged); responses.add(DocumentMessage::StartTransaction); @@ -779,7 +780,15 @@ impl<'a> MessageHandler> for NodeGrap NodeGraphMessage::SetNodeInput { node_id, input_index, input } => { if let Some(network) = document.document_network.nested_network_mut(&self.network) { if let Some(node) = network.nodes.get_mut(&node_id) { - node.inputs[input_index] = input + let Some(node_input) = node.inputs.get_mut(input_index) else { + error!("Tried to set input {input_index} to {input:?}, but the index was invalid. Node {node_id}:\n{node:#?}"); + return; + }; + let structure_changed = node_input.as_node().is_some() || input.as_node().is_some(); + *node_input = input; + if structure_changed { + document.metadata.load_structure(&document.document_network); + } } } } @@ -928,7 +937,13 @@ impl<'a> MessageHandler> for NodeGrap } fn actions(&self) -> ActionList { - if self.has_selection { + unimplemented!("Must use `actions_with_graph_open` instead (unless we change every implementation of the MessageHandler trait).") + } +} + +impl NodeGraphMessageHandler { + pub fn actions_with_node_graph_open(&self, graph_open: bool) -> ActionList { + if self.has_selection && graph_open { actions!(NodeGraphMessageDiscriminant; DeleteSelectedNodes, Cut, Copy, DuplicateSelectedNodes, ToggleHidden) } else { actions!(NodeGraphMessageDiscriminant;) diff --git a/editor/src/messages/portfolio/document/utility_types/transformation.rs b/editor/src/messages/portfolio/document/utility_types/transformation.rs index 3b9e2fd2..9fb355e5 100644 --- a/editor/src/messages/portfolio/document/utility_types/transformation.rs +++ b/editor/src/messages/portfolio/document/utility_types/transformation.rs @@ -375,7 +375,7 @@ impl<'a> Selected<'a> { let transformation = pivot * delta * pivot.inverse(); // TODO: Cache the result of `shallowest_unique_layers` to avoid this heavy computation every frame of movement, see https://github.com/GraphiteEditor/Graphite/pull/481 - for layer_ancestors in self.document.metadata.shallowest_unique_layers(self.selected.iter()) { + for layer_ancestors in self.document.metadata.shallowest_unique_layers(self.selected.iter().copied()) { let layer = *layer_ancestors.last().unwrap(); let parent = layer.parent(&self.document.metadata); if *self.tool_type == ToolType::Select { diff --git a/editor/src/messages/portfolio/portfolio_message_handler.rs b/editor/src/messages/portfolio/portfolio_message_handler.rs index e68ff386..b0fdeaf6 100644 --- a/editor/src/messages/portfolio/portfolio_message_handler.rs +++ b/editor/src/messages/portfolio/portfolio_message_handler.rs @@ -6,6 +6,7 @@ use crate::messages::frontend::utility_types::FrontendDocumentDetails; use crate::messages::input_mapper::utility_types::macros::action_keys; use crate::messages::layout::utility_types::widget_prelude::*; use crate::messages::portfolio::document::utility_types::clipboards::{Clipboard, CopyBufferEntry, INTERNAL_CLIPBOARD_COUNT}; +use crate::messages::portfolio::document::DocumentInputs; use crate::messages::prelude::*; use crate::messages::tool::utility_types::{HintData, HintGroup}; use crate::node_graph_executor::NodeGraphExecutor; @@ -45,7 +46,14 @@ impl MessageHandler { if let Some(document_id) = self.active_document_id { if let Some(document) = self.documents.get_mut(&document_id) { - document.process_message(message, responses, (document_id, ipp, &self.persistent_data, preferences, &mut self.executor)) + let document_inputs = DocumentInputs { + document_id, + ipp, + persistent_data: &self.persistent_data, + preferences, + executor: &mut self.executor, + }; + document.process_message(message, responses, document_inputs) } } } @@ -54,7 +62,14 @@ impl MessageHandler { if let Some(document) = self.documents.get_mut(&document_id) { - document.process_message(message, responses, (document_id, ipp, &self.persistent_data, preferences, &mut self.executor)) + let document_inputs = DocumentInputs { + document_id, + ipp, + persistent_data: &self.persistent_data, + preferences, + executor: &mut self.executor, + }; + document.process_message(message, responses, document_inputs) } } PortfolioMessage::AutoSaveActiveDocument => { @@ -562,7 +577,7 @@ impl MessageHandler