Code review, remove identity test

This commit is contained in:
Adam
2025-07-14 17:38:32 -07:00
parent 7ecd1d0054
commit 54097ebcb5
14 changed files with 71 additions and 113 deletions

View File

@@ -89,13 +89,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
category: "General",
node_template: NodeTemplate {
document_node: DocumentNode {
<<<<<<< HEAD
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
implementation: DocumentNodeImplementation::proto("graphene_core::ops::IdentityNode"),
=======
implementation: DocumentNodeImplementation::proto("graphene_std::any::IdentityNode"),
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
inputs: vec![NodeInput::value(TaggedValue::None, true)],
..Default::default()
},
@@ -113,7 +107,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
category: "General",
node_template: NodeTemplate {
document_node: DocumentNode {
implementation: DocumentNodeImplementation::proto("graphene_std::any::IdentityNode"),
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
manual_composition: Some(generic!(T)),
inputs: vec![NodeInput::value(TaggedValue::None, false)],
..Default::default()

View File

@@ -152,10 +152,6 @@ pub enum NodeGraphMessage {
node_id: NodeId,
alias: String,
},
SetReference {
node_id: NodeId,
reference: Option<String>,
},
SetToNodeOrLayer {
node_id: NodeId,
is_layer: bool,

View File

@@ -1582,7 +1582,6 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
let document_bbox: [DVec2; 2] = viewport_bbox.map(|p| network_metadata.persistent_metadata.navigation_metadata.node_graph_to_viewport.inverse().transform_point2(p));
let mut nodes = Vec::new();
for node_id in &self.frontend_nodes {
let Some(node_bbox) = network_interface.node_bounding_box(node_id, breadcrumb_network_path) else {
log::error!("Could not get bbox for node: {:?}", node_id);
@@ -1730,9 +1729,6 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
responses.add(NodeGraphMessage::SendWires);
}
NodeGraphMessage::SetReference { node_id, reference } => {
network_interface.set_reference(&node_id, breadcrumb_network_path, reference);
}
NodeGraphMessage::SetToNodeOrLayer { node_id, is_layer } => {
if is_layer && !network_interface.is_eligible_to_be_layer(&node_id, selection_network_path) {
return;
@@ -2484,21 +2480,16 @@ impl NodeGraphMessageHandler {
// Offset node insertion 3 grid spaces left and 1 grid space up so the center of the node is dragged
position = position - DVec2::new(GRID_SIZE as f64 * 3., GRID_SIZE as f64);
// Offset to account for division rounding error and place the selected node to the top left of the input
if position.x < 0. {
position.x = position.x - 1.;
}
if position.y < 0. {
position.y = position.y - 1.;
}
let Some(mut input) = network_interface.take_input(&disconnecting, breadcrumb_network_path) else {
return;
};
match &mut input {
NodeInput::Value { exposed, .. } => *exposed = false,
_ => return,
_ => {
network_interface.set_input(&disconnecting, input, breadcrumb_network_path);
return;
}
}
let drag_start = DragStart {

View File

@@ -89,7 +89,7 @@ pub fn start_widgets(parameter_widgets_info: ParameterWidgetsInfo) -> Vec<Widget
description,
input_type,
blank_assist,
exposable: exposeable,
exposable,
} = parameter_widgets_info;
let Some(document_node) = document_node else {
@@ -103,7 +103,7 @@ pub fn start_widgets(parameter_widgets_info: ParameterWidgetsInfo) -> Vec<Widget
};
let description = if description != "TODO" { description } else { String::new() };
let mut widgets = Vec::with_capacity(6);
if exposeable {
if exposable {
widgets.push(expose_widget(node_id, index, input_type, input.is_exposed()));
}
widgets.push(TextLabel::new(name).tooltip(description).widget_holder());
@@ -1913,7 +1913,7 @@ pub fn value_properties(node_id: NodeId, context: &mut NodePropertiesContext) ->
return vec![];
};
let Some(input) = document_node.inputs.get(0) else {
log::warn!("Secondary value input could not be found on value properties");
log::warn!("Value input could not be found in value properties");
return vec![];
};
let mut select_value_widgets = Vec::new();
@@ -1932,20 +1932,16 @@ pub fn value_properties(node_id: NodeId, context: &mut NodePropertiesContext) ->
return Vec::new();
};
// let committer = || {|_| {
// let messages = vec![
// DocumentMessage::AddTransaction.into(),
// NodeGraphMessage::RunDocumentGraph.into(),
// ];
// Message::Batched(messages.into_boxed_slice()).into()
// };
let updater = || {
move |v: &TaggedValueChoice| {
let value = v.to_tagged_value();
let messages = vec![NodeGraphMessage::SetInputValue { node_id, input_index: 0, value }.into(), NodeGraphMessage::SendGraph.into()];
Message::Batched(messages.into_boxed_slice()).into()
Message::Batched {
messages: messages.into_boxed_slice(),
}
.into()
}
};
let value_dropdown = enum_choice::<TaggedValueChoice>().dropdown_menu(choice, updater, || commit_value);

View File

@@ -503,7 +503,7 @@ impl NodeNetworkInterface {
pub fn take_input(&mut self, input_connector: &InputConnector, network_path: &[NodeId]) -> Option<NodeInput> {
let Some(network) = self.network_mut(network_path) else {
log::error!("Could not get network in input_from_connector");
log::error!("Could not get network in take_input");
return None;
};
let input = match input_connector {

View File

@@ -979,13 +979,32 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId],
document.network_interface.set_input(&InputConnector::node(*node_id, 3), old_inputs[4].clone(), network_path);
document.network_interface.set_input(&InputConnector::node(*node_id, 4), old_inputs[5].clone(), network_path);
document.network_interface.set_input(&InputConnector::node(*node_id, 5), old_inputs[3].clone(), network_path);
} else {
// Swap it back if we're not changing anything
let _ = document.network_interface.replace_inputs(node_id, network_path, &mut current_node_template);
upgraded = true;
}
}
if !upgraded {
let _ = document.network_interface.replace_inputs(node_id, network_path, &mut current_node_template);
}
}
// Add the "Depth" parameter to the "Instance Index" node
if reference == "Instance Index" && inputs_count == 0 {
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 mut node_path = network_path.to_vec();
node_path.push(*node_id);
document.network_interface.add_import(TaggedValue::None, false, 0, "Primary", "", &node_path);
document.network_interface.add_import(TaggedValue::U32(0), false, 1, "Loop Level", "TODO", &node_path);
}
// ==================================
// PUT ALL MIGRATIONS ABOVE THIS LINE
// ==================================
// Ensure layers are positioned as stacks if they are upstream siblings of another layer
document.network_interface.load_structure();
let all_layers = LayerNodeIdentifier::ROOT_PARENT.descendants(document.network_interface.document_metadata()).collect::<Vec<_>>();

View File

@@ -1,6 +1,30 @@
use crate::Node;
use crate::{
Node,
registry::{Any, DynFuture, SharedNodeContainer},
};
use std::marker::PhantomData;
pub struct IdentityNode {
value: SharedNodeContainer,
}
impl<'i> Node<'i, Any<'i>> for IdentityNode {
type Output = DynFuture<'i, Any<'i>>;
fn eval(&'i self, input: Any<'i>) -> Self::Output {
Box::pin(async move { self.value.eval(input).await })
}
}
impl IdentityNode {
pub const fn new(value: SharedNodeContainer) -> Self {
IdentityNode { value }
}
}
pub mod identity {
pub const IDENTIFIER: crate::ProtoNodeIdentifier = crate::ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode");
}
// Type
// TODO: Document this
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
@@ -135,13 +159,3 @@ impl<'input, I: 'input + Convert<_O> + Sync + Send, _O: 'input> Node<'input, I>
Box::pin(async move { input.convert() })
}
}
#[cfg(test)]
mod test {
use super::*;
#[test]
pub fn identity_node() {
assert_eq!(identity(&4), &4);
}
}

View File

@@ -7,7 +7,7 @@ use glam::IVec2;
use graphene_core::memo::MemoHashGuard;
pub use graphene_core::uuid::NodeId;
pub use graphene_core::uuid::generate_uuid;
use graphene_core::{Cow, MemoHash, ProtoNodeIdentifier, Type};
use graphene_core::{Cow, MemoHash, ProtoNodeIdentifier, Type, ops};
use log::Metadata;
use rustc_hash::FxHashMap;
use std::collections::HashMap;
@@ -460,7 +460,7 @@ pub enum DocumentNodeImplementation {
impl Default for DocumentNodeImplementation {
fn default() -> Self {
Self::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"))
Self::ProtoNode(ops::identity::IDENTIFIER)
}
}
@@ -916,7 +916,7 @@ impl NodeNetwork {
return;
};
// If the node is hidden, replace it with an identity node
let identity_node = DocumentNodeImplementation::ProtoNode("graphene_std::any::IdentityNode".into());
let identity_node = DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER);
if !node.visible && node.implementation != identity_node {
node.implementation = identity_node;
@@ -1092,7 +1092,7 @@ impl NodeNetwork {
fn remove_id_node(&mut self, id: NodeId) -> Result<(), String> {
let node = self.nodes.get(&id).ok_or_else(|| format!("Node with id {id} does not exist"))?.clone();
if let DocumentNodeImplementation::ProtoNode(ident) = &node.implementation {
if ident.name == "graphene_std::any::IdentityNode" {
if ident.name == ops::identity::IDENTIFIER.name {
assert_eq!(node.inputs.len(), 1, "Id node has more than one input");
if let NodeInput::Node { node_id, output_index, .. } = node.inputs[0] {
let node_input_output_index = output_index;
@@ -1139,13 +1139,13 @@ impl NodeNetwork {
Ok(())
}
/// Strips out any [`graphene_std::any::IdentityNode`]s that are unnecessary.
/// Strips out any [`graphene_std::ops::IdentityNode`]s that are unnecessary.
pub fn remove_redundant_id_nodes(&mut self) {
let id_nodes = self
.nodes
.iter()
.filter(|(_, node)| {
matches!(&node.implementation, DocumentNodeImplementation::ProtoNode(ident) if ident == &ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"))
matches!(&node.implementation, DocumentNodeImplementation::ProtoNode(ident) if ident == &ops::identity::IDENTIFIER)
&& node.inputs.len() == 1
&& matches!(node.inputs[0], NodeInput::Node { .. })
})
@@ -1333,7 +1333,7 @@ mod test {
fn extract_node() {
let id_node = DocumentNode {
inputs: vec![],
implementation: DocumentNodeImplementation::ProtoNode("graphene_std::any::IdentityNode".into()),
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
..Default::default()
};
// TODO: Extend test cases to test nested network
@@ -1535,13 +1535,7 @@ mod test {
NodeId(1),
DocumentNode {
inputs: vec![NodeInput::network(concrete!(u32), 0)],
<<<<<<< HEAD
implementation: DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER),
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
=======
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
..Default::default()
},
),
@@ -1549,13 +1543,7 @@ mod test {
NodeId(2),
DocumentNode {
inputs: vec![NodeInput::network(concrete!(u32), 1)],
<<<<<<< HEAD
implementation: DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER),
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
=======
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
..Default::default()
},
),
@@ -1582,13 +1570,7 @@ mod test {
NodeId(2),
DocumentNode {
inputs: vec![result_node_input],
<<<<<<< HEAD
implementation: DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER),
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
=======
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
..Default::default()
},
),

View File

@@ -143,7 +143,7 @@ pub struct ProtoNode {
impl Default for ProtoNode {
fn default() -> Self {
Self {
identifier: ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"),
identifier: ops::identity::IDENTIFIER,
construction_args: ConstructionArgs::Value(value::TaggedValue::U32(0).into()),
input: ProtoNodeInput::None,
original_location: OriginalLocation::default(),

View File

@@ -46,20 +46,3 @@ pub fn input_node<O: StaticType>(n: SharedNodeContainer) -> DowncastBothNode<(),
pub fn downcast_node<I: StaticType, O: StaticType>(n: SharedNodeContainer) -> DowncastBothNode<I, O> {
DowncastBothNode::new(n)
}
pub struct IdentityNode {
value: SharedNodeContainer,
}
impl<'i> Node<'i, Any<'i>> for IdentityNode {
type Output = DynFuture<'i, Any<'i>>;
fn eval(&'i self, input: Any<'i>) -> Self::Output {
Box::pin(async move { self.value.eval(input).await })
}
}
impl IdentityNode {
pub const fn new(value: SharedNodeContainer) -> Self {
IdentityNode { value }
}
}

View File

@@ -20,13 +20,7 @@ mod tests {
NodeId(0),
DocumentNode {
inputs: vec![NodeInput::network(concrete!(u32), 0)],
<<<<<<< HEAD
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
=======
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
..Default::default()
},
),

View File

@@ -14,8 +14,9 @@ use graphene_std::Context;
use graphene_std::GraphicElement;
#[cfg(feature = "gpu")]
use graphene_std::any::DowncastBothNode;
use graphene_std::any::{ComposeTypeErased, DynAnyNode, IdentityNode, IntoTypeErasedNode};
use graphene_std::any::{ComposeTypeErased, DynAnyNode, IntoTypeErasedNode};
use graphene_std::application_io::{ImageTexture, SurfaceFrame};
use graphene_std::ops::IdentityNode;
#[cfg(feature = "gpu")]
use graphene_std::wasm_application_io::{WasmEditorApi, WasmSurfaceHandle};
use node_registry_macros::{async_node, convert_node, into_node};
@@ -115,7 +116,7 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
),
),
(
ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"),
graphene_std::ops::identity::IDENTIFIER,
|args| {
Box::pin(async move {
let node = IdentityNode::new(args[0].clone());

View File

@@ -68,13 +68,7 @@ pub fn wrap_network_in_scope(mut network: NodeNetwork, editor_api: Arc<WasmEdito
inner_network,
render_node,
DocumentNode {
<<<<<<< HEAD
implementation: DocumentNodeImplementation::ProtoNode(graphene_std::ops::identity::IDENTIFIER),
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
implementation: DocumentNodeImplementation::proto("graphene_core::ops::IdentityNode"),
=======
implementation: DocumentNodeImplementation::proto("graphene_std::any::IdentityNode"),
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
inputs: vec![NodeInput::value(TaggedValue::EditorApi(editor_api), false)],
..Default::default()
},

View File

@@ -49,13 +49,7 @@ pub fn generate_node_substitutions() -> HashMap<ProtoNodeIdentifier, DocumentNod
let input_count = inputs.len();
let network_inputs = (0..input_count).map(|i| NodeInput::node(NodeId(i as u64), 0)).collect();
<<<<<<< HEAD
let identity_node = ops::identity::IDENTIFIER;
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
let identity_node = ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode");
=======
let identity_node = ProtoNodeIdentifier::new("graphene_std::any::IdentityNode");
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
let into_node_registry = &interpreted_executor::node_registry::NODE_REGISTRY;