From 947a131a4b8e86f026f785097e899013a1ee361c Mon Sep 17 00:00:00 2001 From: 0HyperCube <78500760+0HyperCube@users.noreply.github.com> Date: Fri, 29 Dec 2023 08:38:45 +0000 Subject: [PATCH] Add graph type error diagnostics to the UI (#1535) * Fontend input types * Fix index of errors / types * Bug fixes, styling improvements, and code review * Improvements to the error box --------- Co-authored-by: Keavon Chambers --- .../document/document_message_handler.rs | 4 +- .../document/node_graph/node_graph_message.rs | 9 +- .../node_graph/node_graph_message_handler.rs | 70 +++++-- .../node_properties.rs | 10 +- .../properties_panel/utility_types.rs | 2 +- editor/src/node_graph_executor.rs | 24 ++- frontend/src/components/Editor.svelte | 19 +- .../floating-menus/ColorPicker.svelte | 2 +- .../src/components/panels/Document.svelte | 4 +- frontend/src/components/views/Graph.svelte | 108 +++++++++-- .../buttons/ParameterExposeButton.svelte | 24 ++- frontend/src/consts.ts | 2 + frontend/src/wasm-communication/messages.ts | 8 +- node-graph/gcore/src/types.rs | 23 ++- node-graph/graph-craft/src/document.rs | 158 +++++++++++++--- node-graph/graph-craft/src/proto.rs | 179 ++++++++++++++---- .../src/dynamic_executor.rs | 85 +++++++-- node-graph/interpreted-executor/src/lib.rs | 2 +- .../other/bezier-rs-demos/public/style.css | 1 + website/sass/base.scss | 2 + 20 files changed, 566 insertions(+), 170 deletions(-) create mode 100644 frontend/src/consts.ts diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index deedc21e..4a20ebca 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -65,7 +65,7 @@ pub struct DocumentMessageHandler { #[serde(default = "default_rulers_visible")] pub rulers_visible: bool, #[serde(default = "default_collapsed")] - pub collapsed: Vec, // TODO: Is this actually used? Maybe or maybe not. Investigate and potentially remove. + pub collapsed: Vec, // ============================================= // Fields omitted from the saved document format // ============================================= @@ -265,7 +265,7 @@ impl MessageHandler> for DocumentMessageHand node_graph_message_handler: &self.node_graph_handler, executor, document_name: self.name.as_str(), - document_network: &mut self.network, + document_network: &self.network, document_metadata: &mut self.metadata, }; self.properties_panel_message_handler diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs index 52c3b083..82275923 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs @@ -1,7 +1,8 @@ use crate::messages::prelude::*; - use graph_craft::document::value::TaggedValue; use graph_craft::document::{DocumentNode, NodeId, NodeInput}; +use graph_craft::proto::GraphErrors; +use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypes; #[impl_message(Message, DocumentMessage, NodeGraph)] #[derive(PartialEq, Clone, Debug, serde::Serialize, serde::Deserialize)] @@ -111,4 +112,10 @@ pub enum NodeGraphMessage { node_id: NodeId, }, UpdateNewNodeGraph, + UpdateTypes { + #[serde(skip)] + resolved_types: ResolvedDocumentNodeTypes, + #[serde(skip)] + node_graph_errors: GraphErrors, + }, } diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index d3dffe7a..da321ed9 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -5,10 +5,11 @@ 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::document_metadata::{DocumentMetadata, LayerNodeIdentifier}; use crate::messages::prelude::*; - use graph_craft::document::value::TaggedValue; -use graph_craft::document::{DocumentNode, NodeId, NodeInput, NodeNetwork, NodeOutput}; +use graph_craft::document::{DocumentNode, NodeId, NodeInput, NodeNetwork, NodeOutput, Source}; +use graph_craft::proto::GraphErrors; use graphene_core::*; +use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypes; mod document_node_types; mod node_properties; @@ -23,22 +24,22 @@ pub enum FrontendGraphDataType { Raster, #[serde(rename = "color")] Color, - #[serde(rename = "number")] + #[serde(rename = "general")] Text, #[serde(rename = "vector")] Subpath, #[serde(rename = "number")] Number, - #[serde(rename = "number")] + #[serde(rename = "general")] Boolean, /// Refers to the mathematical vector, with direction and magnitude. - #[serde(rename = "vec2")] + #[serde(rename = "number")] Vector, - #[serde(rename = "graphic")] + #[serde(rename = "raster")] GraphicGroup, #[serde(rename = "artboard")] Artboard, - #[serde(rename = "palette")] + #[serde(rename = "color")] Palette, } impl FrontendGraphDataType { @@ -65,6 +66,8 @@ pub struct FrontendGraphInput { #[serde(rename = "dataType")] data_type: FrontendGraphDataType, name: String, + #[serde(rename = "resolvedType")] + resolved_type: Option, } #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize, specta::Type)] @@ -72,6 +75,8 @@ pub struct FrontendGraphOutput { #[serde(rename = "dataType")] data_type: FrontendGraphDataType, name: String, + #[serde(rename = "resolvedType")] + resolved_type: Option, } #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize, specta::Type)] @@ -92,6 +97,7 @@ pub struct FrontendNode { pub position: (i32, i32), pub disabled: bool, pub previewed: bool, + pub errors: Option, } // (link_start, link_end, link_end_input_index) @@ -124,6 +130,8 @@ impl FrontendNodeType { #[derive(Debug, Clone, PartialEq)] pub struct NodeGraphMessageHandler { pub network: Vec, + pub resolved_types: ResolvedDocumentNodeTypes, + pub node_graph_errors: GraphErrors, has_selection: bool, widgets: [LayoutGroup; 2], } @@ -144,6 +152,8 @@ impl Default for NodeGraphMessageHandler { Self { network: Vec::new(), + resolved_types: ResolvedDocumentNodeTypes::default(), + node_graph_errors: Vec::new(), has_selection: false, widgets: [LayoutGroup::Row { widgets: Vec::new() }, LayoutGroup::Row { widgets: right_side_widgets }], } @@ -264,7 +274,7 @@ impl NodeGraphMessageHandler { } } - fn send_graph(network: &NodeNetwork, graph_view_overlay_open: bool, responses: &mut VecDeque) { + fn send_graph(&self, network: &NodeNetwork, graph_view_overlay_open: bool, responses: &mut VecDeque) { responses.add(PropertiesPanelMessage::Refresh); if !graph_view_overlay_open { @@ -298,6 +308,7 @@ impl NodeGraphMessageHandler { let mut nodes = Vec::new(); for (id, node) in &network.nodes { + let node_path = vec![*id]; // TODO: This should be based on the graph runtime type inference system in order to change the colors of node connectors to match the data type in use let Some(node_type) = document_node_types::resolve_document_node_type(&node.name) else { warn!("Node '{}' does not exist in library", node.name); @@ -305,20 +316,26 @@ impl NodeGraphMessageHandler { }; // Inputs - let mut inputs = node.inputs.iter().zip(node_type.inputs.iter().map(|input_type| FrontendGraphInput { - data_type: input_type.data_type, - name: input_type.name.to_string(), + let mut inputs = node.inputs.iter().zip(node_type.inputs.iter().enumerate().map(|(index, input_type)| { + let index = node.inputs.iter().take(index).filter(|input| input.is_exposed()).count(); + FrontendGraphInput { + data_type: input_type.data_type, + name: input_type.name.to_string(), + resolved_type: self.resolved_types.inputs.get(&Source { node: node_path.clone(), index }).map(|input| format!("{input:?}")), + } })); let primary_input = inputs.next().filter(|(input, _)| input.is_exposed()).map(|(_, input_type)| input_type); let exposed_inputs = inputs.filter(|(input, _)| input.is_exposed()).map(|(_, input_type)| input_type).collect(); // Outputs - let mut outputs = node_type.outputs.iter().map(|output_type| FrontendGraphOutput { + let mut outputs = node_type.outputs.iter().enumerate().map(|(index, output_type)| FrontendGraphOutput { data_type: output_type.data_type, name: output_type.name.to_string(), + resolved_type: self.resolved_types.outputs.get(&Source { node: node_path.clone(), index }).map(|output| format!("{output:?}")), }); let primary_output = if node.has_primary_output { outputs.next() } else { None }; + let errors = self.node_graph_errors.iter().find(|error| error.node_path.starts_with(&node_path)).map(|error| error.error.clone()); nodes.push(FrontendNode { is_layer: node.is_layer(), id: *id, @@ -331,6 +348,7 @@ impl NodeGraphMessageHandler { position: node.metadata.position.into(), previewed: network.outputs_contain(*id), disabled: network.disabled.contains(id), + errors: errors.map(|e| format!("{e:?}")), }) } responses.add(FrontendMessage::UpdateNodeGraph { nodes, links }); @@ -607,7 +625,7 @@ impl<'a> MessageHandler> for NodeGrap } } if let Some(network) = document_network.nested_network(&self.network) { - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); } self.update_selected(document_network, metadata, responses); } @@ -635,7 +653,7 @@ impl<'a> MessageHandler> for NodeGrap responses.add(NodeGraphMessage::InsertNode { node_id, document_node }); } - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); self.update_selected(document_network, metadata, responses); responses.add(NodeGraphMessage::SendGraph { should_rerender: false }); } @@ -648,7 +666,7 @@ impl<'a> MessageHandler> for NodeGrap self.network.pop(); } if let Some(network) = document_network.nested_network(&self.network) { - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); } self.update_selected(document_network, metadata, responses); } @@ -698,7 +716,7 @@ impl<'a> MessageHandler> for NodeGrap node.metadata.position += IVec2::new(displacement_x, displacement_y) } } - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); } NodeGraphMessage::PasteNodes { serialized_nodes } => { let Some(network) = document_network.nested_network(&self.network) else { @@ -763,7 +781,7 @@ impl<'a> MessageHandler> for NodeGrap } NodeGraphMessage::SendGraph { should_rerender } => { if let Some(network) = document_network.nested_network(&self.network) { - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); if should_rerender { responses.add(NodeGraphMessage::RunDocumentGraph); } @@ -890,7 +908,7 @@ impl<'a> MessageHandler> for NodeGrap } else if !network.inputs.contains(&node_id) && !network.original_outputs().iter().any(|output| output.node_id == node_id) { network.disabled.push(node_id); } - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); // Only generate node graph if one of the selected nodes is connected to the output if network.connected_to_output(node_id) { @@ -926,7 +944,7 @@ impl<'a> MessageHandler> for NodeGrap } else { return; } - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); } self.update_selection_action_buttons(document_network, metadata, responses); responses.add(NodeGraphMessage::RunDocumentGraph); @@ -936,13 +954,25 @@ impl<'a> MessageHandler> for NodeGrap metadata.clear_selected_nodes(); responses.add(BroadcastEvent::SelectionChanged); - Self::send_graph(network, graph_view_overlay_open, responses); + self.send_graph(network, graph_view_overlay_open, responses); let node_types = document_node_types::collect_node_types(); responses.add(FrontendMessage::UpdateNodeTypes { node_types }); } self.update_selected(document_network, metadata, responses); } + NodeGraphMessage::UpdateTypes { resolved_types, node_graph_errors } => { + let changed = self.resolved_types != resolved_types || self.node_graph_errors != node_graph_errors; + + self.resolved_types = resolved_types; + self.node_graph_errors = node_graph_errors; + + if changed { + if let Some(network) = document_network.nested_network(&self.network) { + self.send_graph(network, graph_view_overlay_open, responses) + } + } + } } self.has_selection = metadata.has_selected_nodes(); } diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler/node_properties.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler/node_properties.rs index 95a0df91..567c7bb9 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler/node_properties.rs @@ -214,7 +214,7 @@ fn vec2_widget(document_node: &DocumentNode, node_id: NodeId, index: usize, name } fn vec_f32_input(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, text_props: TextInput, blank_assist: bool) -> Vec { - let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::Color, blank_assist); + let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::Vector, blank_assist); let from_string = |string: &str| { string @@ -243,7 +243,7 @@ fn vec_f32_input(document_node: &DocumentNode, node_id: NodeId, index: usize, na } fn vec_dvec2_input(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, text_props: TextInput, blank_assist: bool) -> Vec { - let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::Color, blank_assist); + let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::Vector, blank_assist); let from_string = |string: &str| { string @@ -739,7 +739,7 @@ fn gradient_positions(rows: &mut Vec, document_node: &DocumentNode, } fn color_widget(document_node: &DocumentNode, node_id: NodeId, index: usize, name: &str, color_props: ColorButton, blank_assist: bool) -> LayoutGroup { - let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::Number, blank_assist); + let mut widgets = start_widgets(document_node, node_id, index, name, FrontendGraphDataType::Color, blank_assist); if let NodeInput::Value { tagged_value, exposed: false } = &document_node.inputs[index] { if let &TaggedValue::Color(x) = tagged_value { @@ -1452,9 +1452,7 @@ pub fn transform_properties(document_node: &DocumentNode, node_id: NodeId, _cont let scale = vec2_widget(document_node, node_id, 3, "Scale", "W", "H", "x", None, add_blank_assist); - let vector_data = start_widgets(document_node, node_id, 0, "Data", FrontendGraphDataType::Vector, false); - let vector_data = LayoutGroup::Row { widgets: vector_data }; - vec![vector_data, translation, rotation, scale] + vec![translation, rotation, scale] } pub fn node_section_font(document_node: &DocumentNode, node_id: NodeId, _context: &mut NodePropertiesContext) -> Vec { diff --git a/editor/src/messages/portfolio/document/properties_panel/utility_types.rs b/editor/src/messages/portfolio/document/properties_panel/utility_types.rs index 5448053e..1e9cd51b 100644 --- a/editor/src/messages/portfolio/document/properties_panel/utility_types.rs +++ b/editor/src/messages/portfolio/document/properties_panel/utility_types.rs @@ -6,7 +6,7 @@ use graph_craft::document::NodeNetwork; pub struct PropertiesPanelMessageHandlerData<'a> { pub document_name: &'a str, - pub document_network: &'a mut NodeNetwork, + pub document_network: &'a NodeNetwork, pub document_metadata: &'a mut DocumentMetadata, pub node_graph_message_handler: &'a NodeGraphMessageHandler, pub executor: &'a mut NodeGraphExecutor, diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index fe629bea..f219639a 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -12,6 +12,7 @@ use graph_craft::document::value::TaggedValue; use graph_craft::document::{generate_uuid, DocumentNodeImplementation, NodeId, NodeNetwork}; use graph_craft::graphene_compiler::Compiler; use graph_craft::imaginate_input::ImaginatePreferences; +use graph_craft::proto::GraphErrors; use graphene_core::application_io::{NodeGraphUpdateMessage, NodeGraphUpdateSender, RenderConfig}; use graphene_core::memo::IORecord; use graphene_core::raster::{Image, ImageFrame}; @@ -22,7 +23,7 @@ use graphene_core::vector::style::ViewMode; use graphene_core::vector::VectorData; use graphene_core::{Color, GraphicElement, SurfaceFrame}; use graphene_std::wasm_application_io::{WasmApplicationIo, WasmEditorApi}; -use interpreted_executor::dynamic_executor::DynamicExecutor; +use interpreted_executor::dynamic_executor::{DynamicExecutor, ResolvedDocumentNodeTypes}; use glam::{DAffine2, DVec2, UVec2}; use std::cell::RefCell; @@ -40,6 +41,8 @@ pub struct NodeRuntime { pub(crate) thumbnails: HashMap, pub(crate) click_targets: HashMap>, pub(crate) upstream_transforms: HashMap, + pub(crate) resolved_types: ResolvedDocumentNodeTypes, + pub(crate) node_graph_errors: GraphErrors, graph_hash: Option, monitor_nodes: Vec>, } @@ -73,6 +76,8 @@ pub(crate) struct GenerationResponse { new_thumbnails: HashMap, new_click_targets: HashMap>, new_upstream_transforms: HashMap, + resolved_types: ResolvedDocumentNodeTypes, + node_graph_errors: GraphErrors, transform: DAffine2, } @@ -113,6 +118,8 @@ impl NodeRuntime { click_targets: HashMap::new(), graph_hash: None, upstream_transforms: HashMap::new(), + resolved_types: ResolvedDocumentNodeTypes::default(), + node_graph_errors: Vec::new(), monitor_nodes: Vec::new(), } } @@ -147,6 +154,8 @@ impl NodeRuntime { new_thumbnails: self.thumbnails.clone(), new_click_targets: self.click_targets.clone().into_iter().map(|(id, targets)| (LayerNodeIdentifier::new_unchecked(id), targets)).collect(), new_upstream_transforms: self.upstream_transforms.clone(), + resolved_types: self.resolved_types.clone(), + node_graph_errors: core::mem::take(&mut self.node_graph_errors), transform, }; self.sender.send_generation_response(response); @@ -188,7 +197,7 @@ impl NodeRuntime { self.monitor_nodes = scoped_network .recursive_nodes() .filter(|(_, node)| node.implementation == DocumentNodeImplementation::proto("graphene_core::memo::MonitorNode<_, _, _>")) - .map(|(_, node)| node.path.clone().unwrap_or_default()) + .map(|(_, node)| node.original_location.path.clone().unwrap_or_default()) .collect::>(); // We assume only one output @@ -201,11 +210,11 @@ impl NodeRuntime { assert_ne!(proto_network.nodes.len(), 0, "No protonodes exist?"); if let Err(e) = self.executor.update(proto_network).await { - error!("Failed to update executor:\n{e}"); - return Err(e); + self.node_graph_errors = e; + } else { + self.graph_hash = Some(hash_code); } - - self.graph_hash = Some(hash_code); + self.resolved_types = self.executor.document_node_types(); } use graph_craft::graphene_compiler::Executor; @@ -560,8 +569,11 @@ impl NodeGraphExecutor { new_thumbnails, new_click_targets, new_upstream_transforms, + resolved_types, + node_graph_errors, transform, }) => { + responses.add(NodeGraphMessage::UpdateTypes { resolved_types, node_graph_errors }); let node_graph_output = result.map_err(|e| format!("Node graph evaluation failed: {e:?}"))?; let execution_context = self.futures.remove(&generation_id).ok_or_else(|| "Invalid generation ID".to_string())?; diff --git a/frontend/src/components/Editor.svelte b/frontend/src/components/Editor.svelte index f8dbc41b..8d0dbd05 100644 --- a/frontend/src/components/Editor.svelte +++ b/frontend/src/components/Editor.svelte @@ -101,22 +101,19 @@ --color-e-nearwhite-rgb: 238, 238, 238; --color-f-white: #fff; --color-f-white-rgb: 255, 255, 255; + --color-error-red: #d6536e; + --color-error-red-rgb: 214, 83, 110; --color-data-general: #c5c5c5; --color-data-general-dim: #767676; - --color-data-vector: #65bbe5; - --color-data-vector-dim: #4b778c; + --color-data-number: #cbbab4; + --color-data-number-dim: #87736b; --color-data-raster: #e4bb72; --color-data-raster-dim: #8b7752; - --color-data-mask: #8d85c7; - --color-data-number: #d6536e; - --color-data-number-dim: #803242; - --color-data-vec2: #cc00ff; - --color-data-vec2-dim: #71008d; - --color-data-color: #70a898; - --color-data-color-dim: #43645b; - --color-data-graphic: #e4bb72; - --color-data-graphic-dim: #8b7752; + --color-data-vector: #65bbe5; + --color-data-vector-dim: #4b778c; + --color-data-color: #dce472; + --color-data-color-dim: #898d55; --color-data-artboard: #70a898; --color-data-artboard-dim: #3a6156; diff --git a/frontend/src/components/floating-menus/ColorPicker.svelte b/frontend/src/components/floating-menus/ColorPicker.svelte index b8db15cd..fc34f335 100644 --- a/frontend/src/components/floating-menus/ColorPicker.svelte +++ b/frontend/src/components/floating-menus/ColorPicker.svelte @@ -627,7 +627,6 @@ width: 24px; font-size: 0; overflow: hidden; - transition: background-color 0.5s ease; div { display: inline-block; @@ -636,6 +635,7 @@ // For the least jarring luminance conversion, these colors are derived by placing a black layer with the "desaturate" blend mode over the colors. // We don't use the CSS `filter: grayscale(1);` property because it produces overly dark tones for bright colors with a noticeable jump on hover. background: var(--pure-color-gray); + transition: background-color 0.2s ease; } &:hover div, diff --git a/frontend/src/components/panels/Document.svelte b/frontend/src/components/panels/Document.svelte index 06d970f8..8ae44ad5 100644 --- a/frontend/src/components/panels/Document.svelte +++ b/frontend/src/components/panels/Document.svelte @@ -583,7 +583,7 @@ &[title^="Coming Soon"] { opacity: 0.25; - transition: opacity 0.25s; + transition: opacity 0.2s; &:hover { opacity: 1; @@ -703,7 +703,7 @@ .graph-view { pointer-events: none; - transition: opacity 0.1s ease-in-out; + transition: opacity 0.2s ease-in-out; opacity: 0; &.open { diff --git a/frontend/src/components/views/Graph.svelte b/frontend/src/components/views/Graph.svelte index 05b197bc..a3571920 100644 --- a/frontend/src/components/views/Graph.svelte +++ b/frontend/src/components/views/Graph.svelte @@ -1,11 +1,13 @@ -