Skip to content

Commit

Permalink
Improve older document upgrading compatibility and make node type err…
Browse files Browse the repository at this point in the history 
…ors clearer (#2201)

* Improve older document upgrading compatibility and make node type errors clearer

Misc.

* Fixes

* Avoid unwrap
  • Loading branch information
@Keavon
Keavon authored Jan 21, 2025
1 parent eec0ef7 commit 8505ed3
Show file tree
Hide file tree
Showing 13 changed files with 154 additions and 74 deletions.
3 changes: 1 addition & 2 deletions .devcontainer/devcontainer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
},
"ghcr.io/devcontainers/features/node:1": {}
},
"onCreateCommand": "cargo install wasm-pack cargo-watch cargo-about",
"onCreateCommand": "cargo install cargo-watch wasm-pack cargo-about && cargo install -f [email protected]",
"customizations": {
"vscode": {
// NOTE: Keep this in sync with `.vscode/extensions.json`
"extensions": [
// Rust
"rust-lang.rust-analyzer",
"tamasfe.even-better-toml",
"serayuzgur.crates",
// Web
"dbaeumer.vscode-eslint",
"svelte.svelte-vscode",
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/comment-clippy-warnings.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ jobs:
name: Run Clippy
runs-on: ubuntu-latest
# TODO(Keavon): Find a workaround (passing the output text to a separate action with permission to read the secrets?) that allows this to work on fork PRs
if: ${{ !github.event.pull_request.draft && !github.event.pull_request.head.repo.fork }}
if: false
# if: ${{ !github.event.pull_request.draft && !github.event.pull_request.head.repo.fork }}
permissions:
contents: read
pull-requests: write
Expand Down
6 changes: 3 additions & 3 deletions editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct NodePropertiesContext<'a> {
impl NodePropertiesContext<'_> {
pub fn call_widget_override(&mut self, node_id: &NodeId, index: usize) -> Option<Vec<LayoutGroup>> {
let Some(input_properties_row) = self.network_interface.input_properties_row(node_id, index, self.selection_network_path) else {
log::error!("Could not get input properties row in call_widget_override");
log::error!("Could not get input properties row at the beginning of call_widget_override");
return None;
};
if let Some(widget_override) = &input_properties_row.widget_override {
Expand Down Expand Up @@ -2466,7 +2466,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
nodes: [
DocumentNode {
inputs: vec![NodeInput::network(concrete!(graphene_core::vector::VectorData), 0)],
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::vector_nodes::SubpathSegmentLengthsNode")),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::SubpathSegmentLengthsNode")),
manual_composition: Some(generic!(T)),
..Default::default()
},
Expand All @@ -2479,7 +2479,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
NodeInput::network(concrete!(bool), 4), // From the document node's parameters
NodeInput::node(NodeId(0), 0), // From output 0 of SubpathSegmentLengthsNode
],
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::vector_nodes::SamplePointsNode")),
implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_core::vector::SamplePointsNode")),
manual_composition: Some(generic!(T)),
..Default::default()
},
Expand Down
3 changes: 1 addition & 2 deletions editor/src/messages/portfolio/document/node_graph/node_properties.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,7 @@ pub(crate) fn property_from_type(node_id: NodeId, index: usize, ty: &Type, conte
Separator::new(SeparatorType::Unrelated).widget_holder(),
TextLabel::new("-")
.tooltip(format!(
"This data can only be supplied through the\n\
node graph because no widget exists for its type:\n\
"This data can only be supplied through the node graph because no widget exists for its type:\n\
{}",
concrete_type.name
))
Expand Down
98 changes: 84 additions & 14 deletions editor/src/messages/portfolio/portfolio_message_handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::messages::tool::utility_types::{HintData, HintGroup, ToolType};
use crate::node_graph_executor::{ExportConfig, NodeGraphExecutor};

use graph_craft::document::value::TaggedValue;
use graph_craft::document::{NodeId, NodeInput};
use graph_craft::document::{DocumentNodeImplementation, NodeId, NodeInput};
use graphene_core::text::{Font, TypesettingConfig};
use graphene_std::vector::style::{Fill, FillType, Gradient};
use interpreted_executor::dynamic_executor::IntrospectError;
Expand Down Expand Up @@ -383,11 +383,25 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
document_is_saved,
document_serialized_content,
} => {
// TODO: Eventually remove this document upgrade code
// This big code block contains lots of hacky code for upgrading old documents to the new format

// It can be helpful to temporarily set `upgrade_from_before_editable_subgraphs` to true if it's desired to upgrade a piece of artwork to use fresh copies of all nodes
let replace_implementations_from_definition = document_serialized_content.contains("node_output_index");
// Upgrade layer implementation from https://github.com/GraphiteEditor/Graphite/pull/1946 (see also `fn fix_nodes()` in `main.rs` of Graphene CLI)
let upgrade_from_before_returning_nested_click_targets =
document_serialized_content.contains("graphene_core::ConstructLayerNode") || document_serialized_content.contains("graphene_core::AddArtboardNode");
let upgrade_vector_manipulation_format = document_serialized_content.contains("ManipulatorGroupIds") && !document_name.contains("__DO_NOT_UPGRADE__");
let document_name = document_name.replace("__DO_NOT_UPGRADE__", "");

const TEXT_REPLACEMENTS: [(&str, &str); 2] = [
("graphene_core::vector::vector_nodes::SamplePointsNode", "graphene_core::vector::SamplePointsNode"),
("graphene_core::vector::vector_nodes::SubpathSegmentLengthsNode", "graphene_core::vector::SubpathSegmentLengthsNode"),
];
let document_serialized_content = TEXT_REPLACEMENTS
.iter()
.fold(document_serialized_content, |document_serialized_content, (old, new)| document_serialized_content.replace(old, new));

let document = DocumentMessageHandler::deserialize_document(&document_serialized_content).map(|mut document| {
document.name.clone_from(&document_name);
document
Expand All @@ -407,9 +421,72 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
}
};

// TODO: Eventually remove this document upgrade code
const REPLACEMENTS: [(&str, &str); 36] = [
("graphene_core::AddArtboardNode", "graphene_core::graphic_element::AppendArtboardNode"),
("graphene_core::ConstructArtboardNode", "graphene_core::graphic_element::ToArtboardNode"),
("graphene_core::ToGraphicElementNode", "graphene_core::graphic_element::ToElementNode"),
("graphene_core::ToGraphicGroupNode", "graphene_core::graphic_element::ToGroupNode"),
("graphene_core::logic::LogicAndNode", "graphene_core::ops::LogicAndNode"),
("graphene_core::logic::LogicNotNode", "graphene_core::ops::LogicNotNode"),
("graphene_core::logic::LogicOrNode", "graphene_core::ops::LogicOrNode"),
("graphene_core::ops::ConstructVector2", "graphene_core::ops::Vector2ValueNode"),
("graphene_core::raster::BlackAndWhiteNode", "graphene_core::raster::adjustments::BlackAndWhiteNode"),
("graphene_core::raster::BlendNode", "graphene_core::raster::adjustments::BlendNode"),
("graphene_core::raster::ChannelMixerNode", "graphene_core::raster::adjustments::ChannelMixerNode"),
("graphene_core::raster::adjustments::ColorOverlayNode", "graphene_core::raster::adjustments::ColorOverlayNode"),
("graphene_core::raster::ExposureNode", "graphene_core::raster::adjustments::ExposureNode"),
("graphene_core::raster::ExtractChannelNode", "graphene_core::raster::adjustments::ExtractChannelNode"),
("graphene_core::raster::GradientMapNode", "graphene_core::raster::adjustments::GradientMapNode"),
("graphene_core::raster::HueSaturationNode", "graphene_core::raster::adjustments::HueSaturationNode"),
("graphene_core::raster::IndexNode", "graphene_core::raster::adjustments::IndexNode"),
("graphene_core::raster::InvertNode", "graphene_core::raster::adjustments::InvertNode"),
("graphene_core::raster::InvertRGBNode", "graphene_core::raster::adjustments::InvertNode"),
("graphene_core::raster::LevelsNode", "graphene_core::raster::adjustments::LevelsNode"),
("graphene_core::raster::LuminanceNode", "graphene_core::raster::adjustments::LuminanceNode"),
("graphene_core::raster::ExtractOpaqueNode", "graphene_core::raster::adjustments::MakeOpaqueNode"),
("graphene_core::raster::PosterizeNode", "graphene_core::raster::adjustments::PosterizeNode"),
("graphene_core::raster::ThresholdNode", "graphene_core::raster::adjustments::ThresholdNode"),
("graphene_core::raster::VibranceNode", "graphene_core::raster::adjustments::VibranceNode"),
("graphene_core::text::TextGeneratorNode", "graphene_core::text::TextNode"),
("graphene_core::transform::SetTransformNode", "graphene_core::transform::ReplaceTransformNode"),
("graphene_core::vector::generator_nodes::EllipseGenerator", "graphene_core::vector::generator_nodes::EllipseNode"),
("graphene_core::vector::generator_nodes::LineGenerator", "graphene_core::vector::generator_nodes::LineNode"),
("graphene_core::vector::generator_nodes::PathGenerator", "graphene_core::vector::generator_nodes::PathNode"),
("graphene_core::vector::generator_nodes::RectangleGenerator", "graphene_core::vector::generator_nodes::RectangleNode"),
(
"graphene_core::vector::generator_nodes::RegularPolygonGenerator",
"graphene_core::vector::generator_nodes::RegularPolygonNode",
),
("graphene_core::vector::generator_nodes::SplineGenerator", "graphene_core::vector::generator_nodes::SplineNode"),
("graphene_core::vector::generator_nodes::StarGenerator", "graphene_core::vector::generator_nodes::StarNode"),
("graphene_std::executor::BlendGpuImageNode", "graphene_std::gpu_nodes::BlendGpuImageNode"),
("graphene_std::raster::SampleNode", "graphene_std::raster::SampleImageNode"),
];
for node_id in &document
.network_interface
.network_metadata(&[])
.unwrap()
.persistent_metadata
.node_metadata
.keys()
.cloned()
.collect::<Vec<NodeId>>()
{
if let Some(DocumentNodeImplementation::ProtoNode(protonode_id)) = document.network_interface.network(&[]).unwrap().nodes.get(node_id).map(|node| node.implementation.clone()) {
for (old, new) in REPLACEMENTS {
let node_path_without_type_args = protonode_id.name.split('<').next();
if node_path_without_type_args == Some(old) {
document
.network_interface
.replace_implementation(node_id, &[], DocumentNodeImplementation::ProtoNode(new.to_string().into()));
document.network_interface.set_manual_compostion(node_id, &[], Some(graph_craft::Type::Generic("T".into())));
}
}
}
}

// Upgrade all old nodes to support editable subgraphs introduced in #1750
if replace_implementations_from_definition {
if replace_implementations_from_definition || upgrade_from_before_returning_nested_click_targets {
// This can be used, if uncommented, to upgrade demo artwork with outdated document node internals from their definitions. Delete when it's no longer needed.
// Used for upgrading old internal networks for demo artwork nodes. Will reset all node internals for any opened file
for node_id in &document
Expand All @@ -431,12 +508,13 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
.get(node_id)
.and_then(|node| node.persistent_metadata.reference.as_ref())
{
let node_definition = resolve_document_node_type(reference).unwrap();
let Some(node_definition) = resolve_document_node_type(reference) else { continue };
let default_definition_node = node_definition.default_node_template();
document.network_interface.replace_implementation(node_id, &[], default_definition_node.document_node.implementation);
document
.network_interface
.replace_implementation_metadata(node_id, &[], default_definition_node.persistent_node_metadata);
document.network_interface.set_manual_compostion(node_id, &[], default_definition_node.document_node.manual_composition);
}
}
}
Expand All @@ -460,8 +538,6 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
continue;
};

// Upgrade Fill nodes to the format change in #1778
// TODO: Eventually remove this document upgrade code
let Some(ref reference) = node_metadata.persistent_metadata.reference.clone() else {
continue;
};
Expand All @@ -472,6 +548,7 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
};
let inputs_count = node.inputs.len();

// Upgrade Fill nodes to the format change in #1778
if reference == "Fill" && inputs_count == 8 {
let node_definition = resolve_document_node_type(reference).unwrap();
let document_node = node_definition.default_node_template().document_node;
Expand Down Expand Up @@ -600,15 +677,8 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
.set_input(&InputConnector::node(*node_id, 2), NodeInput::value(TaggedValue::Bool(false), false), &[]);
}

// Upgrade layer implementation from https://github.com/GraphiteEditor/Graphite/pull/1946
if reference == "Merge" || reference == "Artboard" {
let node_definition = crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_document_node_type(reference).unwrap();
let new_merge_node = node_definition.default_node_template();
document.network_interface.replace_implementation(node_id, &[], new_merge_node.document_node.implementation)
}

// Upgrade artboard name being passed as hidden value input to "To Artboard"
if reference == "Artboard" {
if reference == "Artboard" && upgrade_from_before_returning_nested_click_targets {
let label = document.network_interface.frontend_display_name(node_id, &[]);
document
.network_interface
Expand Down
Loading

0 comments on commit 8505ed3

Please sign in to comment.