Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve older document upgrading compatibility and make node type errors clearer #2201

Merged
merged 3 commits into from
Jan 21, 2025
Merged

Improve older document upgrading compatibility and make node type errors clearer #2201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
913b5e2
Improve older document upgrading compatibility and make node type err…
Keavon Jan 16, 2025
73c144a
Fixes
Keavon Jan 21, 2025
2b95a21
Avoid unwrap
Keavon Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .devcontainer/devcontainer.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading