Skip to content

Commit 940c714

Browse files
committed
Code review, remove identity test
1 parent ce05901 commit 940c714

File tree

14 files changed

+71
-113
lines changed

14 files changed

+71
-113
lines changed

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
8989
category: "General",
9090
node_template: NodeTemplate {
9191
document_node: DocumentNode {
92-
<<<<<<< HEAD
9392
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
94-
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
95-
implementation: DocumentNodeImplementation::proto("graphene_core::ops::IdentityNode"),
96-
=======
97-
implementation: DocumentNodeImplementation::proto("graphene_std::any::IdentityNode"),
98-
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
9993
inputs: vec![NodeInput::value(TaggedValue::None, true)],
10094
..Default::default()
10195
},
@@ -113,7 +107,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
113107
category: "General",
114108
node_template: NodeTemplate {
115109
document_node: DocumentNode {
116-
implementation: DocumentNodeImplementation::proto("graphene_std::any::IdentityNode"),
110+
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
117111
manual_composition: Some(generic!(T)),
118112
inputs: vec![NodeInput::value(TaggedValue::None, false)],
119113
..Default::default()

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,6 @@ pub enum NodeGraphMessage {
147147
node_id: NodeId,
148148
alias: String,
149149
},
150-
SetReference {
151-
node_id: NodeId,
152-
reference: Option<String>,
153-
},
154150
SetToNodeOrLayer {
155151
node_id: NodeId,
156152
is_layer: bool,

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,6 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
13691369
let document_bbox: [DVec2; 2] = viewport_bbox.map(|p| network_metadata.persistent_metadata.navigation_metadata.node_graph_to_viewport.inverse().transform_point2(p));
13701370

13711371
let mut nodes = Vec::new();
1372-
13731372
for node_id in &self.frontend_nodes {
13741373
let Some(node_bbox) = network_interface.node_bounding_box(node_id, breadcrumb_network_path) else {
13751374
log::error!("Could not get bbox for node: {:?}", node_id);
@@ -1512,9 +1511,6 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
15121511

15131512
responses.add(NodeGraphMessage::SendWires);
15141513
}
1515-
NodeGraphMessage::SetReference { node_id, reference } => {
1516-
network_interface.set_reference(&node_id, breadcrumb_network_path, reference);
1517-
}
15181514
NodeGraphMessage::SetToNodeOrLayer { node_id, is_layer } => {
15191515
if is_layer && !network_interface.is_eligible_to_be_layer(&node_id, selection_network_path) {
15201516
return;
@@ -2208,21 +2204,16 @@ impl NodeGraphMessageHandler {
22082204
// Offset node insertion 3 grid spaces left and 1 grid space up so the center of the node is dragged
22092205
position = position - DVec2::new(GRID_SIZE as f64 * 3., GRID_SIZE as f64);
22102206

2211-
// Offset to account for division rounding error and place the selected node to the top left of the input
2212-
if position.x < 0. {
2213-
position.x = position.x - 1.;
2214-
}
2215-
if position.y < 0. {
2216-
position.y = position.y - 1.;
2217-
}
2218-
22192207
let Some(mut input) = network_interface.take_input(&disconnecting, breadcrumb_network_path) else {
22202208
return;
22212209
};
22222210

22232211
match &mut input {
22242212
NodeInput::Value { exposed, .. } => *exposed = false,
2225-
_ => return,
2213+
_ => {
2214+
network_interface.set_input(&disconnecting, input, breadcrumb_network_path);
2215+
return;
2216+
}
22262217
}
22272218

22282219
let drag_start = DragStart {

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ pub fn start_widgets(parameter_widgets_info: ParameterWidgetsInfo) -> Vec<Widget
8989
description,
9090
input_type,
9191
blank_assist,
92-
exposable: exposeable,
92+
exposable,
9393
} = parameter_widgets_info;
9494

9595
let Some(document_node) = document_node else {
@@ -103,7 +103,7 @@ pub fn start_widgets(parameter_widgets_info: ParameterWidgetsInfo) -> Vec<Widget
103103
};
104104
let description = if description != "TODO" { description } else { String::new() };
105105
let mut widgets = Vec::with_capacity(6);
106-
if exposeable {
106+
if exposable {
107107
widgets.push(expose_widget(node_id, index, input_type, input.is_exposed()));
108108
}
109109
widgets.push(TextLabel::new(name).tooltip(description).widget_holder());
@@ -1842,7 +1842,7 @@ pub fn value_properties(node_id: NodeId, context: &mut NodePropertiesContext) ->
18421842
return vec![];
18431843
};
18441844
let Some(input) = document_node.inputs.get(0) else {
1845-
log::warn!("Secondary value input could not be found on value properties");
1845+
log::warn!("Value input could not be found in value properties");
18461846
return vec![];
18471847
};
18481848
let mut select_value_widgets = Vec::new();
@@ -1861,20 +1861,16 @@ pub fn value_properties(node_id: NodeId, context: &mut NodePropertiesContext) ->
18611861
return Vec::new();
18621862
};
18631863

1864-
// let committer = || {|_| {
1865-
// let messages = vec![
1866-
// DocumentMessage::AddTransaction.into(),
1867-
// NodeGraphMessage::RunDocumentGraph.into(),
1868-
// ];
1869-
// Message::Batched(messages.into_boxed_slice()).into()
1870-
// };
18711864
let updater = || {
18721865
move |v: &TaggedValueChoice| {
18731866
let value = v.to_tagged_value();
18741867

18751868
let messages = vec![NodeGraphMessage::SetInputValue { node_id, input_index: 0, value }.into(), NodeGraphMessage::SendGraph.into()];
18761869

1877-
Message::Batched(messages.into_boxed_slice()).into()
1870+
Message::Batched {
1871+
messages: messages.into_boxed_slice(),
1872+
}
1873+
.into()
18781874
}
18791875
};
18801876
let value_dropdown = enum_choice::<TaggedValueChoice>().dropdown_menu(choice, updater, || commit_value);

editor/src/messages/portfolio/document/utility_types/network_interface.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ impl NodeNetworkInterface {
503503

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

editor/src/messages/portfolio/document_migration.rs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,13 +959,32 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId],
959959
document.network_interface.set_input(&InputConnector::node(*node_id, 3), old_inputs[4].clone(), network_path);
960960
document.network_interface.set_input(&InputConnector::node(*node_id, 4), old_inputs[5].clone(), network_path);
961961
document.network_interface.set_input(&InputConnector::node(*node_id, 5), old_inputs[3].clone(), network_path);
962-
} else {
963-
// Swap it back if we're not changing anything
964-
let _ = document.network_interface.replace_inputs(node_id, network_path, &mut current_node_template);
962+
963+
upgraded = true;
965964
}
966965
}
966+
967+
if !upgraded {
968+
let _ = document.network_interface.replace_inputs(node_id, network_path, &mut current_node_template);
969+
}
967970
}
968971

972+
// Add the "Depth" parameter to the "Instance Index" node
973+
if reference == "Instance Index" && inputs_count == 0 {
974+
let mut node_template = resolve_document_node_type(reference)?.default_node_template();
975+
document.network_interface.replace_implementation(node_id, network_path, &mut node_template);
976+
977+
let mut node_path = network_path.to_vec();
978+
node_path.push(*node_id);
979+
980+
document.network_interface.add_import(TaggedValue::None, false, 0, "Primary", "", &node_path);
981+
document.network_interface.add_import(TaggedValue::U32(0), false, 1, "Loop Level", "TODO", &node_path);
982+
}
983+
984+
// ==================================
985+
// PUT ALL MIGRATIONS ABOVE THIS LINE
986+
// ==================================
987+
969988
// Ensure layers are positioned as stacks if they are upstream siblings of another layer
970989
document.network_interface.load_structure();
971990
let all_layers = LayerNodeIdentifier::ROOT_PARENT.descendants(document.network_interface.document_metadata()).collect::<Vec<_>>();

node-graph/gcore/src/ops.rs

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,30 @@
1-
use crate::Node;
1+
use crate::{
2+
Node,
3+
registry::{Any, DynFuture, SharedNodeContainer},
4+
};
25
use std::marker::PhantomData;
36

7+
pub struct IdentityNode {
8+
value: SharedNodeContainer,
9+
}
10+
11+
impl<'i> Node<'i, Any<'i>> for IdentityNode {
12+
type Output = DynFuture<'i, Any<'i>>;
13+
fn eval(&'i self, input: Any<'i>) -> Self::Output {
14+
Box::pin(async move { self.value.eval(input).await })
15+
}
16+
}
17+
18+
impl IdentityNode {
19+
pub const fn new(value: SharedNodeContainer) -> Self {
20+
IdentityNode { value }
21+
}
22+
}
23+
24+
pub mod identity {
25+
pub const IDENTIFIER: crate::ProtoNodeIdentifier = crate::ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode");
26+
}
27+
428
// Type
529
// TODO: Document this
630
#[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>
135159
Box::pin(async move { input.convert() })
136160
}
137161
}
138-
139-
#[cfg(test)]
140-
mod test {
141-
use super::*;
142-
143-
#[test]
144-
pub fn identity_node() {
145-
assert_eq!(identity(&4), &4);
146-
}
147-
}

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

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use glam::IVec2;
77
use graphene_core::memo::MemoHashGuard;
88
pub use graphene_core::uuid::NodeId;
99
pub use graphene_core::uuid::generate_uuid;
10-
use graphene_core::{Cow, MemoHash, ProtoNodeIdentifier, Type};
10+
use graphene_core::{Cow, MemoHash, ProtoNodeIdentifier, Type, ops};
1111
use log::Metadata;
1212
use rustc_hash::FxHashMap;
1313
use std::collections::HashMap;
@@ -460,7 +460,7 @@ pub enum DocumentNodeImplementation {
460460

461461
impl Default for DocumentNodeImplementation {
462462
fn default() -> Self {
463-
Self::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"))
463+
Self::ProtoNode(ops::identity::IDENTIFIER)
464464
}
465465
}
466466

@@ -916,7 +916,7 @@ impl NodeNetwork {
916916
return;
917917
};
918918
// If the node is hidden, replace it with an identity node
919-
let identity_node = DocumentNodeImplementation::ProtoNode("graphene_std::any::IdentityNode".into());
919+
let identity_node = DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER);
920920
if !node.visible && node.implementation != identity_node {
921921
node.implementation = identity_node;
922922

@@ -1092,7 +1092,7 @@ impl NodeNetwork {
10921092
fn remove_id_node(&mut self, id: NodeId) -> Result<(), String> {
10931093
let node = self.nodes.get(&id).ok_or_else(|| format!("Node with id {id} does not exist"))?.clone();
10941094
if let DocumentNodeImplementation::ProtoNode(ident) = &node.implementation {
1095-
if ident.name == "graphene_std::any::IdentityNode" {
1095+
if ident.name == ops::identity::IDENTIFIER.name {
10961096
assert_eq!(node.inputs.len(), 1, "Id node has more than one input");
10971097
if let NodeInput::Node { node_id, output_index, .. } = node.inputs[0] {
10981098
let node_input_output_index = output_index;
@@ -1139,13 +1139,13 @@ impl NodeNetwork {
11391139
Ok(())
11401140
}
11411141

1142-
/// Strips out any [`graphene_std::any::IdentityNode`]s that are unnecessary.
1142+
/// Strips out any [`graphene_std::ops::IdentityNode`]s that are unnecessary.
11431143
pub fn remove_redundant_id_nodes(&mut self) {
11441144
let id_nodes = self
11451145
.nodes
11461146
.iter()
11471147
.filter(|(_, node)| {
1148-
matches!(&node.implementation, DocumentNodeImplementation::ProtoNode(ident) if ident == &ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"))
1148+
matches!(&node.implementation, DocumentNodeImplementation::ProtoNode(ident) if ident == &ops::identity::IDENTIFIER)
11491149
&& node.inputs.len() == 1
11501150
&& matches!(node.inputs[0], NodeInput::Node { .. })
11511151
})
@@ -1333,7 +1333,7 @@ mod test {
13331333
fn extract_node() {
13341334
let id_node = DocumentNode {
13351335
inputs: vec![],
1336-
implementation: DocumentNodeImplementation::ProtoNode("graphene_std::any::IdentityNode".into()),
1336+
implementation: DocumentNodeImplementation::ProtoNode(ops::identity::IDENTIFIER),
13371337
..Default::default()
13381338
};
13391339
// TODO: Extend test cases to test nested network
@@ -1535,27 +1535,15 @@ mod test {
15351535
NodeId(1),
15361536
DocumentNode {
15371537
inputs: vec![NodeInput::network(concrete!(u32), 0)],
1538-
<<<<<<< HEAD
15391538
implementation: DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER),
1540-
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
1541-
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
1542-
=======
1543-
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
1544-
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
15451539
..Default::default()
15461540
},
15471541
),
15481542
(
15491543
NodeId(2),
15501544
DocumentNode {
15511545
inputs: vec![NodeInput::network(concrete!(u32), 1)],
1552-
<<<<<<< HEAD
15531546
implementation: DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER),
1554-
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
1555-
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
1556-
=======
1557-
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
1558-
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
15591547
..Default::default()
15601548
},
15611549
),
@@ -1582,13 +1570,7 @@ mod test {
15821570
NodeId(2),
15831571
DocumentNode {
15841572
inputs: vec![result_node_input],
1585-
<<<<<<< HEAD
15861573
implementation: DocumentNodeImplementation::ProtoNode(graphene_core::ops::identity::IDENTIFIER),
1587-
||||||| parent of 8e045313 (Migrate pass through and value node to identity implementation)
1588-
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode")),
1589-
=======
1590-
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::any::IdentityNode")),
1591-
>>>>>>> 8e045313 (Migrate pass through and value node to identity implementation)
15921574
..Default::default()
15931575
},
15941576
),

node-graph/graph-craft/src/proto.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ pub struct ProtoNode {
143143
impl Default for ProtoNode {
144144
fn default() -> Self {
145145
Self {
146-
identifier: ProtoNodeIdentifier::new("graphene_std::any::IdentityNode"),
146+
identifier: ops::identity::IDENTIFIER,
147147
construction_args: ConstructionArgs::Value(value::TaggedValue::U32(0).into()),
148148
input: ProtoNodeInput::None,
149149
original_location: OriginalLocation::default(),

node-graph/gstd/src/any.rs

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,3 @@ pub fn input_node<O: StaticType>(n: SharedNodeContainer) -> DowncastBothNode<(),
4646
pub fn downcast_node<I: StaticType, O: StaticType>(n: SharedNodeContainer) -> DowncastBothNode<I, O> {
4747
DowncastBothNode::new(n)
4848
}
49-
50-
pub struct IdentityNode {
51-
value: SharedNodeContainer,
52-
}
53-
54-
impl<'i> Node<'i, Any<'i>> for IdentityNode {
55-
type Output = DynFuture<'i, Any<'i>>;
56-
fn eval(&'i self, input: Any<'i>) -> Self::Output {
57-
Box::pin(async move { self.value.eval(input).await })
58-
}
59-
}
60-
61-
impl IdentityNode {
62-
pub const fn new(value: SharedNodeContainer) -> Self {
63-
IdentityNode { value }
64-
}
65-
}

0 commit comments

Comments
 (0)