From 5c8d90477bd9fe55efb521ed24a9d0aa964d906f Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 27 Aug 2026 16:16:05 +0000 Subject: [PATCH 1/6] Revert "Fix corrupted font resource loading in older documents" This reverts commit 404d9f3047711d900e04573548745b869946f240. --- .../document/document_message_handler.rs | 2 +- .../resource/resource_message_handler.rs | 101 +----- .../messages/portfolio/document_migration.rs | 294 ++++++------------ .../resource_storage_message_handler.rs | 5 - .../src/application_io/resource/opfs.rs | 3 - .../nodes/gstd/src/platform_application_io.rs | 15 +- 6 files changed, 115 insertions(+), 305 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index be36df2c65..f0972624cf 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -299,7 +299,7 @@ impl MessageHandler> for DocumentMes graph_operation_message_handler.process_message(message, responses, context); } DocumentMessage::Resource(message) => { - let context = ResourceMessageContext { document_id, fonts, resource_storage }; + let context = ResourceMessageContext { document_id, fonts }; self.resources.process_message(message, responses, context); } DocumentMessage::AlignSelectedLayers { axis, aggregate } => { diff --git a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs index ce1bf94b37..7a45060149 100644 --- a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs +++ b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs @@ -12,7 +12,6 @@ use url::Url; pub struct ResourceMessageContext<'a> { pub document_id: DocumentId, pub fonts: &'a FontsMessageHandler, - pub resource_storage: &'a ResourceStorageMessageHandler, } #[derive(Debug, Clone, PartialEq, Default, serde::Serialize, ExtractField)] @@ -26,7 +25,7 @@ pub struct ResourceMessageHandler { #[message_handler_data] impl MessageHandler> for ResourceMessageHandler { fn process_message(&mut self, message: ResourceMessage, responses: &mut VecDeque, context: ResourceMessageContext) { - let ResourceMessageContext { document_id, fonts, resource_storage } = context; + let ResourceMessageContext { document_id, fonts } = context; match message { ResourceMessage::StoreEmbedded { resource_id, data } => { @@ -49,16 +48,8 @@ impl MessageHandler> for ResourceMes responses.add(ResourceMessage::Resolve { resource_id }); } ResourceMessage::ResolveAll => { - // A resource keeps its hash when storage evicts its data, so only a fetchable source can repair it - let refetchable = self - .registry - .resolved() - .filter(|info| info.hash.is_some_and(|hash| !resource_storage.contains(hash))) - .filter(|info| info.sources.iter().any(|source| matches!(source, DataSource::Url(_) | DataSource::Font { .. }))) - .map(|info| info.id); - let ids: Vec = self.registry.unresolved().map(|info| info.id).chain(refetchable).collect(); - - for id in ids { + let unresolved_ids: Vec = self.registry.unresolved().map(|info| info.id).collect(); + for id in unresolved_ids { if self.pending_resolves.contains(&id) { continue; } @@ -74,9 +65,7 @@ impl MessageHandler> for ResourceMes log::error!("Resolve for {resource_id}: no registry entry"); return; }; - // This hash names the very data that is missing, so it cannot stand in for fetching that data - let data_missing = info.hash.is_some_and(|hash| !resource_storage.contains(hash)); - if info.hash.is_some() && !data_missing { + if info.hash.is_some() { log::warn!("Resource {resource_id} already resolved"); return; } @@ -89,7 +78,7 @@ impl MessageHandler> for ResourceMes .sources .iter() .map(|source| match source { - DataSource::Font { family, style } if !data_missing => { + DataSource::Font { family, style } => { let font = match style { Some(style) => Font::new(family.clone(), style.clone()), None => Font::new_with_default_style(family.clone()), @@ -225,20 +214,16 @@ impl ResourceMessageHandler { .resolved() .filter(|info| info.sources.contains(&DataSource::Embedded)) .filter_map(|info| { - let (id, hash) = (info.id, *info.hash?); - let resource = resources_load_handle.load(hash); - Some(async move { (id, hash, resource.await) }) + if let Some(hash) = info.hash { + let resource = resources_load_handle.load(*hash); + Some(async move { resource.await.map(|resource| (*hash, resource)) }) + } else { + None + } }) .collect::>(); - let loaded = futures::future::join_all(embedded).await; - - // Saving without these bytes writes a document whose registry claims to carry them - for (id, hash, _) in loaded.iter().filter(|(_, _, resource)| resource.is_none()) { - log::error!("Resource {id} ({hash}) is marked as embedded but its data is missing from storage, so the saved document will not contain it"); - } - - self.embedded = EmbeddedResources::from_iter(loaded.into_iter().filter_map(|(_, hash, resource)| resource.map(|resource| (hash, resource)))); + self.embedded = EmbeddedResources::from_iter(futures::future::join_all(embedded).await.into_iter().flatten()); } pub fn collect_garbage(&mut self, used: &[ResourceId]) { @@ -312,65 +297,3 @@ impl<'de> serde::Deserialize<'de> for ResourceMessageHandler { deserializer.deserialize_map(EmbeddedResourcesVisitor { human_readable }) } } - -#[cfg(test)] -mod tests { - use super::*; - use graph_craft::application_io::resource::ResourceStorage; - - /// Storage can lose a resource's data while the document keeps the hash naming it, which leaves the graph - /// pointing at bytes that are gone. Only sources that can be fetched again are worth re-resolving. - #[test] - fn resolve_all_refetches_resources_whose_data_is_missing() { - let mut handler = ResourceMessageHandler::default(); - let storage = ResourceStorageMessageHandler::default(); - - // Present: its bytes are in storage, so it is already usable - let present = ResourceId::new(); - let present_hash = storage.resources_mut().store(b"stored font bytes"); - handler.registry.resolve(&present, present_hash); - handler.registry.push_source_back(&present, DataSource::Embedded); - handler.registry.push_source_back( - &present, - DataSource::Font { - family: "Lato".into(), - style: Some("Regular (400)".into()), - }, - ); - - // Recoverable: its bytes are gone, but the font it came from can be downloaded again - let recoverable = ResourceId::new(); - handler.registry.resolve(&recoverable, ResourceHash::from(b"evicted font bytes".as_slice())); - handler.registry.push_source_back(&recoverable, DataSource::Embedded); - handler.registry.push_source_back( - &recoverable, - DataSource::Font { - family: "Lato".into(), - style: Some("Black (900)".into()), - }, - ); - - // Unrecoverable: its bytes are gone and nothing records where to fetch them from - let unrecoverable = ResourceId::new(); - handler.registry.resolve(&unrecoverable, ResourceHash::from(b"evicted image bytes".as_slice())); - handler.registry.push_source_back(&unrecoverable, DataSource::Embedded); - - let mut responses = VecDeque::new(); - let fonts = FontsMessageHandler::default(); - handler.process_message( - ResourceMessage::ResolveAll, - &mut responses, - ResourceMessageContext { - document_id: DocumentId(0), - fonts: &fonts, - resource_storage: &storage, - }, - ); - - let resolve_requested = |id: ResourceId| responses.contains(&Message::from(ResourceMessage::Resolve { resource_id: id })); - - assert!(resolve_requested(recoverable), "a missing resource with a font source should be fetched again"); - assert!(!resolve_requested(present), "a resource whose data is in storage should be left alone"); - assert!(!resolve_requested(unrecoverable), "a missing resource with no fetchable source has nowhere to fetch from"); - } -} diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index c83e51518a..bbfdfe266b 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -1936,38 +1936,79 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], } } - // Every Text node era before alignment only appended inputs, so each is a prefix of the 11-input layout. - // Alignment (#2920) is the exception: it landed at index 9 and pushed Per-Glyph Instances out to 10. - if reference == DefinitionIdentifier::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")) && (4..=10).contains(&inputs_count) { + // Upgrade Text node to include line height and character spacing, which were previously hardcoded to 1, from https://github.com/GraphiteEditor/Graphite/pull/2016 + if reference == DefinitionIdentifier::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")) && inputs_count == 8 { let mut template: NodeTemplate = legacy_text_node_template()?; document.network_interface.replace_implementation(node_id, network_path, &mut template); let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut template)?; - // Line height and character spacing were hardcoded to 1 in the era before they became inputs - let hardcoded_to_one = || NodeInput::value(TaggedValue::F64(1.), false); - // Zero is how an absent `Option` maximum reads to the split below - let unset_maximum = || NodeInput::value(TaggedValue::F64(0.), false); - - let upgraded_inputs = [ - old_inputs[0].clone(), - old_inputs[1].clone(), - old_inputs[2].clone(), - old_inputs[3].clone(), - old_inputs.get(4).cloned().unwrap_or_else(hardcoded_to_one), - old_inputs.get(5).cloned().unwrap_or_else(hardcoded_to_one), - old_inputs.get(6).cloned().unwrap_or_else(unset_maximum), - old_inputs.get(7).cloned().unwrap_or_else(unset_maximum), - old_inputs - .get(8) - .cloned() - .unwrap_or_else(|| NodeInput::value(TaggedValue::F64(TypesettingConfig::default().letter_tilt), false)), - NodeInput::value(TaggedValue::TextAlign(TextAlign::default()), false), - old_inputs.get(9).cloned().unwrap_or_else(|| NodeInput::value(TaggedValue::Bool(false), false)), - ]; - for (index, input) in upgraded_inputs.into_iter().enumerate() { - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input, network_path); - } - + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 0), old_inputs[0].clone(), network_path); + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 1), old_inputs[1].clone(), network_path); + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 2), old_inputs[2].clone(), network_path); + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 3), old_inputs[3].clone(), network_path); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 4), + if inputs_count == 6 { + old_inputs[4].clone() + } else { + NodeInput::value(TaggedValue::F64(TypesettingConfig::default().line_height_ratio), false) + }, + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 5), + if inputs_count == 6 { + old_inputs[5].clone() + } else { + NodeInput::value(TaggedValue::F64(TypesettingConfig::default().letter_spacing), false) + }, + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 6), + if inputs_count >= 7 { + old_inputs[6].clone() + } else { + NodeInput::value(TaggedValue::F64(TypesettingConfig::default().max_width.unwrap_or_default()), false) + }, + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 7), + if inputs_count >= 8 { + old_inputs[7].clone() + } else { + NodeInput::value(TaggedValue::F64(TypesettingConfig::default().max_width.unwrap_or_default()), false) + }, + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 8), + if inputs_count >= 9 { + old_inputs[8].clone() + } else { + NodeInput::value(TaggedValue::F64(TypesettingConfig::default().letter_tilt), false) + }, + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 9), + if inputs_count >= 10 { + old_inputs[9].clone() + } else { + NodeInput::value(TaggedValue::TextAlign(TextAlign::default()), false) + }, + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 10), + if inputs_count >= 11 { + old_inputs[10].clone() + } else { + NodeInput::value(TaggedValue::Bool(false), false) + }, + network_path, + ); inputs_count = 11 } @@ -1984,26 +2025,31 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document.network_interface.set_input(&InputConnector::node_at_index(*node_id, i), old_inputs[i].clone(), network_path); } - // The old `Option` maximum becomes a bool plus a value, with zero standing in for the absent option. - // A wired maximum has no value to read, so it keeps its connection and counts as present. - let split_maximum = |input: &NodeInput| match input.as_value() { - Some(&TaggedValue::F64(maximum)) => (maximum != 0., NodeInput::value(TaggedValue::F64(if maximum == 0. { 100. } else { maximum }), false)), - _ => (true, input.clone()), - }; - // Max Width - let (has_max_width, max_width) = split_maximum(&old_inputs[6]); - document - .network_interface - .set_input(&InputConnector::node_at_index(*node_id, 6), NodeInput::value(TaggedValue::Bool(has_max_width), false), network_path); - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 7), max_width, network_path); + let Some(&TaggedValue::F64(old_max_width)) = old_inputs[6].as_value() else { return None }; + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 6), + NodeInput::value(TaggedValue::Bool(old_max_width != 0.), false), + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 7), + NodeInput::value(TaggedValue::F64(if old_max_width == 0. { 100. } else { old_max_width }), false), + network_path, + ); // Max Height - let (has_max_height, max_height) = split_maximum(&old_inputs[7]); - document - .network_interface - .set_input(&InputConnector::node_at_index(*node_id, 8), NodeInput::value(TaggedValue::Bool(has_max_height), false), network_path); - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 9), max_height, network_path); + let Some(&TaggedValue::F64(old_max_height)) = old_inputs[7].as_value() else { return None }; + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 8), + NodeInput::value(TaggedValue::Bool(old_max_height != 0.), false), + network_path, + ); + document.network_interface.set_input( + &InputConnector::node_at_index(*node_id, 9), + NodeInput::value(TaggedValue::F64(if old_max_height == 0. { 100. } else { old_max_height }), false), + network_path, + ); // Copy over old inputs #[allow(clippy::needless_range_loop)] @@ -2322,39 +2368,6 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document.network_interface.add_import(TaggedValue::U32(0), false, 1, "Loop Level", "TODO", &node_path); } - // Drop the placeholder primary input the "Read Vector" node used to carry, since it reads its value from the context - if reference == DefinitionIdentifier::ProtoNode(graphene_std::context::read_vector::IDENTIFIER) && 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); - document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; - } - - // The "Dot Product" node gained a "Normalize" toggle, which older nodes predate by always taking the raw dot product - if reference == DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::dot_product::IDENTIFIER) && inputs_count == 2 { - let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); - let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; - - for (index, input) in old_inputs.iter().take(2).enumerate() { - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input.clone(), network_path); - } - document - .network_interface - .set_input(&InputConnector::node_at_index(*node_id, 2), NodeInput::value(TaggedValue::Bool(false), false), network_path); - } - - // The "Query JSON" node succeeded "JSON Get", whose object lookups always returned their strings unquoted - if reference == DefinitionIdentifier::ProtoNode(graphene_std::text_nodes::json::query_json::IDENTIFIER) && inputs_count == 2 { - let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); - let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; - - for (index, input) in old_inputs.iter().take(2).enumerate() { - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input.clone(), network_path); - } - document - .network_interface - .set_input(&InputConnector::node_at_index(*node_id, 2), NodeInput::value(TaggedValue::Bool(true), false), network_path); - } - // Upgrade the "Animation" node to add the "Rate" input if reference == DefinitionIdentifier::ProtoNode(graphene_std::animation::animation_time::IDENTIFIER) && inputs_count < 2 { let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); @@ -2797,39 +2810,19 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], /// definition by its old reference name, swaps it to a still-supported implementation, and preserves the user's inputs. /// After this runs, the node's reference resolves cleanly so the rest of `migrate_node` proceeds normally. fn migrate_removed_catalog_definitions(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document: &mut DocumentMessageHandler) -> Option<()> { - // Collapse the legacy "Sample Points" and "Sample Polyline" wrapper networks into the standalone `sample_polyline` - // proto node, which now computes per-bezpath segment lengths inline instead of through the wrapper's helper nodes. - // The oldest documents lose their stored reference on load, so the wrapper is recognized by the nodes it encloses. - let wrapper_inputs = match &node.implementation { - DocumentNodeImplementation::Network(inner) => { - let helpers = [graphene_std::ops::passthrough::IDENTIFIER, graphene_std::memo::memoize::IDENTIFIER]; - let mut sample_nodes = 0; - let only_helpers = inner.nodes.values().all(|inner_node| match &inner_node.implementation { - DocumentNodeImplementation::ProtoNode(identifier) if *identifier == graphene_std::vector::sample_polyline::IDENTIFIER => { - sample_nodes += 1; - true - } - DocumentNodeImplementation::ProtoNode(identifier) => helpers.contains(identifier), - _ => false, - }); - (only_helpers && sample_nodes == 1).then_some(node.inputs.len()) - } - _ => None, - }; - if let Some(wrapper_inputs) = wrapper_inputs - && (wrapper_inputs == 5 || wrapper_inputs == 7) + // Collapse the legacy "Sample Polyline" wrapper network into the standalone `sample_polyline` proto node. + // The proto node now computes per-bezpath segment lengths inline, so the wrapper's separate `subpath_segment_lengths` + // and `Memoize` nodes are no longer needed. The 7 user-facing inputs are positionally identical between the + // old wrapper and the new proto node. + if let Some(DefinitionIdentifier::Network(name)) = document.network_interface.reference(node_id, network_path) + && name == "Sample Polyline" + && node.inputs.len() == 7 { let mut node_template = resolve_proto_node_type(graphene_std::vector::sample_polyline::IDENTIFIER)?.default_node_template(); document.network_interface.replace_implementation(node_id, network_path, &mut node_template); let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; - - // The 5-input era predates the separation/quantity choice, so its lone spacing distance becomes the separation - let upgraded_inputs: Vec<(usize, NodeInput)> = match wrapper_inputs { - 5 => [0, 2, 4, 5, 6].into_iter().zip(old_inputs.iter().cloned()).collect(), - _ => old_inputs.iter().take(7).cloned().enumerate().collect(), - }; - for (index, input) in upgraded_inputs { - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input, network_path); + for (index, input) in old_inputs.iter().take(7).enumerate() { + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input.clone(), network_path); } } @@ -2918,93 +2911,6 @@ mod tests { } } - // The Text node produced geometry until it became a string source, so every shape it ever had must reach the - // current one and gain the converter that turns its string back into geometry - #[test] - fn every_legacy_text_shape_gains_its_geometry_converter() { - use crate::messages::portfolio::document::utility_types::network_interface::NodeTemplate; - use graphene_std::NodeParameter; - use graphene_std::text::Font; - - // Each era only appended to the one before it, so the shorter shapes are prefixes of this longest pre-alignment one - let legacy_inputs = [ - NodeInput::scope("editor-api"), - NodeInput::value(TaggedValue::String("Lorem".into()), false), - NodeInput::value(TaggedValue::Font(Font::new("Lato".to_string(), "Regular (400)".to_string())), false), - NodeInput::value(TaggedValue::F64(48.), false), - NodeInput::value(TaggedValue::F64(1.5), false), - NodeInput::value(TaggedValue::F64(2.), false), - NodeInput::value(TaggedValue::F64(0.), false), - NodeInput::value(TaggedValue::F64(0.), false), - NodeInput::value(TaggedValue::F64(10.), false), - NodeInput::value(TaggedValue::Bool(false), false), - ]; - - for shape in [4, 6, 8, 9, 10] { - let (text_id, consumer_id) = (NodeId(1), NodeId(2)); - let mut document = DocumentMessageHandler::default(); - document.network_interface.insert_node( - text_id, - NodeTemplate { - implementation: NodeTemplateImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")), - inputs: legacy_inputs[..shape].to_vec(), - ..Default::default() - }, - &[], - ); - document.network_interface.insert_node( - consumer_id, - NodeTemplate { - inputs: vec![NodeInput::value(TaggedValue::None, false)], - ..Default::default() - }, - &[], - ); - document.network_interface.set_input(&InputConnector::node_at_index(consumer_id, 0), NodeInput::node(text_id, 0), &[]); - - document_migration_upgrades(&mut document, false); - - let network = document.network_interface.document_network(); - let text_node = network.nodes.get(&text_id).expect("the upgraded text node should keep its ID"); - assert_eq!(text_node.inputs.len(), 12, "a {shape}-input text node should reach the current shape"); - - // The converter is a new node, so it is found by identity rather than by ID - let converter = network - .nodes - .iter() - .find(|(_, node)| matches!(&node.implementation, DocumentNodeImplementation::ProtoNode(identifier) if *identifier == graphene_std::text::text_to_vector::IDENTIFIER)) - .map(|(converter_id, _)| *converter_id) - .unwrap_or_else(|| panic!("a {shape}-input text node should gain a string converter")); - assert_eq!( - network.nodes[&consumer_id].inputs.first(), - Some(&NodeInput::node(converter, 0)), - "the converter should be spliced onto the wire leaving a {shape}-input text node" - ); - - let input_value = |index: usize| text_node.inputs.get(index).and_then(|input| input.as_value()).cloned(); - assert_eq!(input_value(graphene_std::text::text::SizeInput::INDEX), Some(TaggedValue::F64(48.)), "shape {shape} lost its size"); - if shape >= 6 { - assert_eq!( - input_value(graphene_std::text::text::LineHeightInput::INDEX), - Some(TaggedValue::F64(1.5)), - "shape {shape} lost its line height" - ); - assert_eq!( - input_value(graphene_std::text::text::LetterSpacingInput::INDEX), - Some(TaggedValue::F64(2.)), - "shape {shape} lost its letter spacing" - ); - } - if shape >= 9 { - assert_eq!( - input_value(graphene_std::text::text::LetterTiltInput::INDEX), - Some(TaggedValue::F64(10.)), - "shape {shape} lost its letter tilt" - ); - } - } - } - #[test] fn test_no_duplicate_node_replacements() { let mut hashmap = HashMap::::new(); diff --git a/editor/src/messages/resource_storage/resource_storage_message_handler.rs b/editor/src/messages/resource_storage/resource_storage_message_handler.rs index 0f5dc35516..26001fb948 100644 --- a/editor/src/messages/resource_storage/resource_storage_message_handler.rs +++ b/editor/src/messages/resource_storage/resource_storage_message_handler.rs @@ -48,11 +48,6 @@ impl ResourceStorageMessageHandler { inner: self.storage.clone().expect("Resource storage not initialized"), } } - - /// Whether the resource's data is held in storage, assuming it is until storage is initialized. - pub fn contains(&self, hash: &ResourceHash) -> bool { - self.storage.as_ref().is_none_or(|storage| storage.contains(hash)) - } } impl std::fmt::Debug for ResourceStorageMessageHandler { diff --git a/node-graph/graph-craft/src/application_io/resource/opfs.rs b/node-graph/graph-craft/src/application_io/resource/opfs.rs index e023db78b5..261a172b64 100644 --- a/node-graph/graph-craft/src/application_io/resource/opfs.rs +++ b/node-graph/graph-craft/src/application_io/resource/opfs.rs @@ -153,9 +153,6 @@ async fn drain_queue(inner: Arc>) { Mutation::Write { hash, bytes } => { if let Err(error) = write_file(&directory, &hash, &bytes).await { log::error!("OPFS write for {hash} failed: {error:?}"); - - // Nothing reached disk, so leaving the hash listed would claim a file that later sessions cannot read - inner.lock().unwrap().on_disk.remove(&hash); } } Mutation::Delete { hash } => { diff --git a/node-graph/nodes/gstd/src/platform_application_io.rs b/node-graph/nodes/gstd/src/platform_application_io.rs index 13ad491d0e..8b71b4f7c5 100644 --- a/node-graph/nodes/gstd/src/platform_application_io.rs +++ b/node-graph/nodes/gstd/src/platform_application_io.rs @@ -269,19 +269,8 @@ pub async fn resource<'a: 'n>( hash: Item, ) -> Item { let hash = hash.into_element(); - let placeholder = || -> Item { Item::new_from_element(Resource::empty()) }; - - let Some(application_io) = editor_api.into_element().application_io.as_ref() else { - log::error!("Resource {hash} is unavailable because the platform's application IO is missing"); - return placeholder(); - }; - - // Stored bytes go missing when the browser evicts its storage or a write is interrupted - let Some(resource) = application_io.load_resource(hash).await else { - log::error!("Resource {hash} was not found in storage"); - return placeholder(); - }; - + let application_io = editor_api.into_element().application_io.as_ref().expect("ApplicationIo must be available when using resources"); + let resource = application_io.load_resource(hash).await.unwrap_or_else(|| panic!("Resource {hash} not found")); Item::new_from_element(resource) } From af287c736be2884fb5064a97bb3191b6e72570c3 Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 27 Aug 2026 18:20:09 +0000 Subject: [PATCH 2/6] Fix migrations for the Read Vector, Dot Product, Query JSON, and Sample Polyline nodes --- .../messages/portfolio/document_migration.rs | 71 ++++++++++++++++--- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index bbfdfe266b..854e72b7f8 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -2368,6 +2368,39 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document.network_interface.add_import(TaggedValue::U32(0), false, 1, "Loop Level", "TODO", &node_path); } + // Drop the placeholder primary input the "Read Vector" node used to carry, since it reads its value from the context + if reference == DefinitionIdentifier::ProtoNode(graphene_std::context::read_vector::IDENTIFIER) && 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); + document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + } + + // The "Dot Product" node gained a "Normalize" toggle, which older nodes predate by always taking the raw dot product + if reference == DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::dot_product::IDENTIFIER) && inputs_count == 2 { + let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + + for (index, input) in old_inputs.iter().take(2).enumerate() { + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input.clone(), network_path); + } + document + .network_interface + .set_input(&InputConnector::node_at_index(*node_id, 2), NodeInput::value(TaggedValue::Bool(false), false), network_path); + } + + // The "Query JSON" node succeeded "JSON Get", whose object lookups always returned their strings unquoted + if reference == DefinitionIdentifier::ProtoNode(graphene_std::text_nodes::json::query_json::IDENTIFIER) && inputs_count == 2 { + let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + + for (index, input) in old_inputs.iter().take(2).enumerate() { + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input.clone(), network_path); + } + document + .network_interface + .set_input(&InputConnector::node_at_index(*node_id, 2), NodeInput::value(TaggedValue::Bool(true), false), network_path); + } + // Upgrade the "Animation" node to add the "Rate" input if reference == DefinitionIdentifier::ProtoNode(graphene_std::animation::animation_time::IDENTIFIER) && inputs_count < 2 { let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); @@ -2810,19 +2843,39 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], /// definition by its old reference name, swaps it to a still-supported implementation, and preserves the user's inputs. /// After this runs, the node's reference resolves cleanly so the rest of `migrate_node` proceeds normally. fn migrate_removed_catalog_definitions(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document: &mut DocumentMessageHandler) -> Option<()> { - // Collapse the legacy "Sample Polyline" wrapper network into the standalone `sample_polyline` proto node. - // The proto node now computes per-bezpath segment lengths inline, so the wrapper's separate `subpath_segment_lengths` - // and `Memoize` nodes are no longer needed. The 7 user-facing inputs are positionally identical between the - // old wrapper and the new proto node. - if let Some(DefinitionIdentifier::Network(name)) = document.network_interface.reference(node_id, network_path) - && name == "Sample Polyline" - && node.inputs.len() == 7 + // Collapse the legacy "Sample Points" and "Sample Polyline" wrapper networks into the standalone `sample_polyline` + // proto node, which now computes per-bezpath segment lengths inline instead of through the wrapper's helper nodes. + // The oldest documents lose their stored reference on load, so the wrapper is recognized by the nodes it encloses. + let wrapper_inputs = match &node.implementation { + DocumentNodeImplementation::Network(inner) => { + let helpers = [graphene_std::ops::passthrough::IDENTIFIER, graphene_std::memo::memoize::IDENTIFIER]; + let mut sample_nodes = 0; + let only_helpers = inner.nodes.values().all(|inner_node| match &inner_node.implementation { + DocumentNodeImplementation::ProtoNode(identifier) if *identifier == graphene_std::vector::sample_polyline::IDENTIFIER => { + sample_nodes += 1; + true + } + DocumentNodeImplementation::ProtoNode(identifier) => helpers.contains(identifier), + _ => false, + }); + (only_helpers && sample_nodes == 1).then_some(node.inputs.len()) + } + _ => None, + }; + if let Some(wrapper_inputs) = wrapper_inputs + && (wrapper_inputs == 5 || wrapper_inputs == 7) { let mut node_template = resolve_proto_node_type(graphene_std::vector::sample_polyline::IDENTIFIER)?.default_node_template(); document.network_interface.replace_implementation(node_id, network_path, &mut node_template); let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; - for (index, input) in old_inputs.iter().take(7).enumerate() { - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input.clone(), network_path); + + // The 5-input era predates the separation/quantity choice, so its lone spacing distance becomes the separation + let upgraded_inputs: Vec<(usize, NodeInput)> = match wrapper_inputs { + 5 => [0, 2, 4, 5, 6].into_iter().zip(old_inputs.iter().cloned()).collect(), + _ => old_inputs.iter().take(7).cloned().enumerate().collect(), + }; + for (index, input) in upgraded_inputs { + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input, network_path); } } From 78184d395ef35ae7f8577aef5a955330399f0c0e Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 27 Aug 2026 18:19:04 +0000 Subject: [PATCH 3/6] Fix text node migrations --- .../messages/portfolio/document_migration.rs | 223 +++++++++++------- 1 file changed, 132 insertions(+), 91 deletions(-) diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index 854e72b7f8..c83e51518a 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -1936,79 +1936,38 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], } } - // Upgrade Text node to include line height and character spacing, which were previously hardcoded to 1, from https://github.com/GraphiteEditor/Graphite/pull/2016 - if reference == DefinitionIdentifier::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")) && inputs_count == 8 { + // Every Text node era before alignment only appended inputs, so each is a prefix of the 11-input layout. + // Alignment (#2920) is the exception: it landed at index 9 and pushed Per-Glyph Instances out to 10. + if reference == DefinitionIdentifier::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")) && (4..=10).contains(&inputs_count) { let mut template: NodeTemplate = legacy_text_node_template()?; document.network_interface.replace_implementation(node_id, network_path, &mut template); let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut template)?; - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 0), old_inputs[0].clone(), network_path); - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 1), old_inputs[1].clone(), network_path); - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 2), old_inputs[2].clone(), network_path); - document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 3), old_inputs[3].clone(), network_path); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 4), - if inputs_count == 6 { - old_inputs[4].clone() - } else { - NodeInput::value(TaggedValue::F64(TypesettingConfig::default().line_height_ratio), false) - }, - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 5), - if inputs_count == 6 { - old_inputs[5].clone() - } else { - NodeInput::value(TaggedValue::F64(TypesettingConfig::default().letter_spacing), false) - }, - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 6), - if inputs_count >= 7 { - old_inputs[6].clone() - } else { - NodeInput::value(TaggedValue::F64(TypesettingConfig::default().max_width.unwrap_or_default()), false) - }, - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 7), - if inputs_count >= 8 { - old_inputs[7].clone() - } else { - NodeInput::value(TaggedValue::F64(TypesettingConfig::default().max_width.unwrap_or_default()), false) - }, - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 8), - if inputs_count >= 9 { - old_inputs[8].clone() - } else { - NodeInput::value(TaggedValue::F64(TypesettingConfig::default().letter_tilt), false) - }, - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 9), - if inputs_count >= 10 { - old_inputs[9].clone() - } else { - NodeInput::value(TaggedValue::TextAlign(TextAlign::default()), false) - }, - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 10), - if inputs_count >= 11 { - old_inputs[10].clone() - } else { - NodeInput::value(TaggedValue::Bool(false), false) - }, - network_path, - ); + // Line height and character spacing were hardcoded to 1 in the era before they became inputs + let hardcoded_to_one = || NodeInput::value(TaggedValue::F64(1.), false); + // Zero is how an absent `Option` maximum reads to the split below + let unset_maximum = || NodeInput::value(TaggedValue::F64(0.), false); + + let upgraded_inputs = [ + old_inputs[0].clone(), + old_inputs[1].clone(), + old_inputs[2].clone(), + old_inputs[3].clone(), + old_inputs.get(4).cloned().unwrap_or_else(hardcoded_to_one), + old_inputs.get(5).cloned().unwrap_or_else(hardcoded_to_one), + old_inputs.get(6).cloned().unwrap_or_else(unset_maximum), + old_inputs.get(7).cloned().unwrap_or_else(unset_maximum), + old_inputs + .get(8) + .cloned() + .unwrap_or_else(|| NodeInput::value(TaggedValue::F64(TypesettingConfig::default().letter_tilt), false)), + NodeInput::value(TaggedValue::TextAlign(TextAlign::default()), false), + old_inputs.get(9).cloned().unwrap_or_else(|| NodeInput::value(TaggedValue::Bool(false), false)), + ]; + for (index, input) in upgraded_inputs.into_iter().enumerate() { + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, index), input, network_path); + } + inputs_count = 11 } @@ -2025,31 +1984,26 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document.network_interface.set_input(&InputConnector::node_at_index(*node_id, i), old_inputs[i].clone(), network_path); } + // The old `Option` maximum becomes a bool plus a value, with zero standing in for the absent option. + // A wired maximum has no value to read, so it keeps its connection and counts as present. + let split_maximum = |input: &NodeInput| match input.as_value() { + Some(&TaggedValue::F64(maximum)) => (maximum != 0., NodeInput::value(TaggedValue::F64(if maximum == 0. { 100. } else { maximum }), false)), + _ => (true, input.clone()), + }; + // Max Width - let Some(&TaggedValue::F64(old_max_width)) = old_inputs[6].as_value() else { return None }; - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 6), - NodeInput::value(TaggedValue::Bool(old_max_width != 0.), false), - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 7), - NodeInput::value(TaggedValue::F64(if old_max_width == 0. { 100. } else { old_max_width }), false), - network_path, - ); + let (has_max_width, max_width) = split_maximum(&old_inputs[6]); + document + .network_interface + .set_input(&InputConnector::node_at_index(*node_id, 6), NodeInput::value(TaggedValue::Bool(has_max_width), false), network_path); + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 7), max_width, network_path); // Max Height - let Some(&TaggedValue::F64(old_max_height)) = old_inputs[7].as_value() else { return None }; - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 8), - NodeInput::value(TaggedValue::Bool(old_max_height != 0.), false), - network_path, - ); - document.network_interface.set_input( - &InputConnector::node_at_index(*node_id, 9), - NodeInput::value(TaggedValue::F64(if old_max_height == 0. { 100. } else { old_max_height }), false), - network_path, - ); + let (has_max_height, max_height) = split_maximum(&old_inputs[7]); + document + .network_interface + .set_input(&InputConnector::node_at_index(*node_id, 8), NodeInput::value(TaggedValue::Bool(has_max_height), false), network_path); + document.network_interface.set_input(&InputConnector::node_at_index(*node_id, 9), max_height, network_path); // Copy over old inputs #[allow(clippy::needless_range_loop)] @@ -2964,6 +2918,93 @@ mod tests { } } + // The Text node produced geometry until it became a string source, so every shape it ever had must reach the + // current one and gain the converter that turns its string back into geometry + #[test] + fn every_legacy_text_shape_gains_its_geometry_converter() { + use crate::messages::portfolio::document::utility_types::network_interface::NodeTemplate; + use graphene_std::NodeParameter; + use graphene_std::text::Font; + + // Each era only appended to the one before it, so the shorter shapes are prefixes of this longest pre-alignment one + let legacy_inputs = [ + NodeInput::scope("editor-api"), + NodeInput::value(TaggedValue::String("Lorem".into()), false), + NodeInput::value(TaggedValue::Font(Font::new("Lato".to_string(), "Regular (400)".to_string())), false), + NodeInput::value(TaggedValue::F64(48.), false), + NodeInput::value(TaggedValue::F64(1.5), false), + NodeInput::value(TaggedValue::F64(2.), false), + NodeInput::value(TaggedValue::F64(0.), false), + NodeInput::value(TaggedValue::F64(0.), false), + NodeInput::value(TaggedValue::F64(10.), false), + NodeInput::value(TaggedValue::Bool(false), false), + ]; + + for shape in [4, 6, 8, 9, 10] { + let (text_id, consumer_id) = (NodeId(1), NodeId(2)); + let mut document = DocumentMessageHandler::default(); + document.network_interface.insert_node( + text_id, + NodeTemplate { + implementation: NodeTemplateImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::text::TextNode")), + inputs: legacy_inputs[..shape].to_vec(), + ..Default::default() + }, + &[], + ); + document.network_interface.insert_node( + consumer_id, + NodeTemplate { + inputs: vec![NodeInput::value(TaggedValue::None, false)], + ..Default::default() + }, + &[], + ); + document.network_interface.set_input(&InputConnector::node_at_index(consumer_id, 0), NodeInput::node(text_id, 0), &[]); + + document_migration_upgrades(&mut document, false); + + let network = document.network_interface.document_network(); + let text_node = network.nodes.get(&text_id).expect("the upgraded text node should keep its ID"); + assert_eq!(text_node.inputs.len(), 12, "a {shape}-input text node should reach the current shape"); + + // The converter is a new node, so it is found by identity rather than by ID + let converter = network + .nodes + .iter() + .find(|(_, node)| matches!(&node.implementation, DocumentNodeImplementation::ProtoNode(identifier) if *identifier == graphene_std::text::text_to_vector::IDENTIFIER)) + .map(|(converter_id, _)| *converter_id) + .unwrap_or_else(|| panic!("a {shape}-input text node should gain a string converter")); + assert_eq!( + network.nodes[&consumer_id].inputs.first(), + Some(&NodeInput::node(converter, 0)), + "the converter should be spliced onto the wire leaving a {shape}-input text node" + ); + + let input_value = |index: usize| text_node.inputs.get(index).and_then(|input| input.as_value()).cloned(); + assert_eq!(input_value(graphene_std::text::text::SizeInput::INDEX), Some(TaggedValue::F64(48.)), "shape {shape} lost its size"); + if shape >= 6 { + assert_eq!( + input_value(graphene_std::text::text::LineHeightInput::INDEX), + Some(TaggedValue::F64(1.5)), + "shape {shape} lost its line height" + ); + assert_eq!( + input_value(graphene_std::text::text::LetterSpacingInput::INDEX), + Some(TaggedValue::F64(2.)), + "shape {shape} lost its letter spacing" + ); + } + if shape >= 9 { + assert_eq!( + input_value(graphene_std::text::text::LetterTiltInput::INDEX), + Some(TaggedValue::F64(10.)), + "shape {shape} lost its letter tilt" + ); + } + } + } + #[test] fn test_no_duplicate_node_replacements() { let mut hashmap = HashMap::::new(); From 33a7fd7e8b568762d929d372c6710a9929172b5b Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 27 Aug 2026 17:22:58 +0000 Subject: [PATCH 4/6] Provide resource storage to the resource message handler --- editor/src/dispatcher.rs | 3 ++- .../messages/portfolio/document/document_message_handler.rs | 2 +- .../portfolio/document/resource/resource_message_handler.rs | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/editor/src/dispatcher.rs b/editor/src/dispatcher.rs index f2c665648d..4941768b2a 100644 --- a/editor/src/dispatcher.rs +++ b/editor/src/dispatcher.rs @@ -96,8 +96,9 @@ impl Dispatcher { s } + #[cfg(test)] pub fn with_executor(executor: crate::node_graph_executor::NodeGraphExecutor) -> Self { - let mut s = Self::default(); + let mut s = Self::new(Arc::new(graph_craft::application_io::resource::HashMapResourceStorage::new()), None); s.message_handlers.portfolio_message_handler = PortfolioMessageHandler::with_executor(executor); s } diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index f0972624cf..be36df2c65 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -299,7 +299,7 @@ impl MessageHandler> for DocumentMes graph_operation_message_handler.process_message(message, responses, context); } DocumentMessage::Resource(message) => { - let context = ResourceMessageContext { document_id, fonts }; + let context = ResourceMessageContext { document_id, fonts, resource_storage }; self.resources.process_message(message, responses, context); } DocumentMessage::AlignSelectedLayers { axis, aggregate } => { diff --git a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs index 7a45060149..7b9189e3e8 100644 --- a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs +++ b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs @@ -12,6 +12,7 @@ use url::Url; pub struct ResourceMessageContext<'a> { pub document_id: DocumentId, pub fonts: &'a FontsMessageHandler, + pub resource_storage: &'a ResourceStorageMessageHandler, } #[derive(Debug, Clone, PartialEq, Default, serde::Serialize, ExtractField)] @@ -25,7 +26,7 @@ pub struct ResourceMessageHandler { #[message_handler_data] impl MessageHandler> for ResourceMessageHandler { fn process_message(&mut self, message: ResourceMessage, responses: &mut VecDeque, context: ResourceMessageContext) { - let ResourceMessageContext { document_id, fonts } = context; + let ResourceMessageContext { document_id, fonts, resource_storage } = context; match message { ResourceMessage::StoreEmbedded { resource_id, data } => { From 1e59b40acb4fb0291f9cfe0078890b2d42b685ed Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 27 Aug 2026 17:52:23 +0000 Subject: [PATCH 5/6] Refetch resources whose stored bytes are missing --- .../document/resource/resource_message_handler.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs index 7b9189e3e8..210c7eb9a9 100644 --- a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs +++ b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs @@ -3,7 +3,7 @@ use crate::messages::portfolio::{document::resource::utility_types::EmbeddedReso use crate::messages::prelude::*; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64; -use graph_craft::application_io::resource::{DataSource, LoadResource, Resource, ResourceHash, ResourceId, ResourceRegistry}; +use graph_craft::application_io::resource::{DataSource, LoadResource, Resource, ResourceHash, ResourceId, ResourceRegistry, ResourceStorage}; use graphene_std::text::Font; use std::sync::Arc; use url::Url; @@ -49,8 +49,9 @@ impl MessageHandler> for ResourceMes responses.add(ResourceMessage::Resolve { resource_id }); } ResourceMessage::ResolveAll => { - let unresolved_ids: Vec = self.registry.unresolved().map(|info| info.id).collect(); - for id in unresolved_ids { + let storage = resource_storage.resources_mut(); + let ids: Vec = self.registry.ids().filter(|id| !self.registry.hash(id).is_some_and(|hash| storage.contains(&hash))).collect(); + for id in ids { if self.pending_resolves.contains(&id) { continue; } @@ -66,7 +67,7 @@ impl MessageHandler> for ResourceMes log::error!("Resolve for {resource_id}: no registry entry"); return; }; - if info.hash.is_some() { + if info.hash.is_some_and(|hash| resource_storage.resources_mut().contains(hash)) { log::warn!("Resource {resource_id} already resolved"); return; } From 0239846666086432aea622a9a530fc921af0af52 Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 27 Aug 2026 18:07:15 +0000 Subject: [PATCH 6/6] Add test for refetching resources with missing bytes --- .../resource/resource_message_handler.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs index 210c7eb9a9..a5df618a69 100644 --- a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs +++ b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs @@ -299,3 +299,47 @@ impl<'de> serde::Deserialize<'de> for ResourceMessageHandler { deserializer.deserialize_map(EmbeddedResourcesVisitor { human_readable }) } } + +#[cfg(test)] +mod tests { + use super::*; + use graph_craft::application_io::resource::ResourceStorage; + + #[test] + fn resolve_all_refetches_resources_whose_bytes_are_missing() { + let mut handler = ResourceMessageHandler::default(); + let storage = ResourceStorageMessageHandler::default(); + let fonts = FontsMessageHandler::default(); + + let font = |style: &str| DataSource::Font { + family: "Lato".into(), + style: Some(style.into()), + }; + + let cached = ResourceId::from(1); + let cached_hash = storage.resources_mut().store(b"stored font bytes"); + handler.registry.resolve(&cached, cached_hash); + handler.registry.push_source_back(&cached, font("Regular (400)")); + + let evicted = ResourceId::from(2); + let evicted_hash = ResourceHash::from(b"evicted font bytes".as_slice()); + handler.registry.resolve(&evicted, evicted_hash); + handler.registry.push_source_back(&evicted, font("Black (900)")); + + let mut responses = VecDeque::new(); + handler.process_message( + ResourceMessage::ResolveAll, + &mut responses, + ResourceMessageContext { + document_id: DocumentId(0), + fonts: &fonts, + resource_storage: &storage, + }, + ); + + let resolve_requested = |id: ResourceId| responses.contains(&Message::from(ResourceMessage::Resolve { resource_id: id })); + assert!(resolve_requested(evicted), "a resource whose bytes are gone should be resolved again"); + assert!(!resolve_requested(cached), "a resource whose bytes are in storage should be left alone"); + assert_eq!(handler.registry.hash(&evicted), Some(evicted_hash), "the hash keeps naming the content"); + } +}