From 00629a9636ca2888a5abc28f6f685a07bea6cbde Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Sat, 30 May 2026 10:52:45 +0200 Subject: [PATCH 1/2] Make ResourceStorage trait &self-based with elided-lifetime ResourceFuture --- desktop/src/app.rs | 2 +- desktop/wrapper/src/lib.rs | 3 ++- editor/src/application.rs | 4 ++-- editor/src/dispatcher.rs | 5 +++-- .../resource_storage_message_handler.rs | 20 ++++++++----------- frontend/wrapper/src/editor_wrapper.rs | 6 +++--- node-graph/graph-craft/src/application_io.rs | 2 +- .../src/application_io/resource.rs | 14 ++++++------- .../src/application_io/resource/mmap.rs | 12 +++++------ .../src/application_io/resource/opfs.rs | 8 ++++---- .../libraries/application-io/src/lib.rs | 4 ++-- node-graph/libraries/resources/src/lib.rs | 10 +++++----- 12 files changed, 44 insertions(+), 46 deletions(-) diff --git a/desktop/src/app.rs b/desktop/src/app.rs index 19e452304c..16c2022b21 100644 --- a/desktop/src/app.rs +++ b/desktop/src/app.rs @@ -99,7 +99,7 @@ impl App { let wake = std::sync::Arc::new(move || { wake_scheduler.schedule(AppEvent::DesktopWrapperMessage(DesktopWrapperMessage::Wake)); }); - let desktop_wrapper = DesktopWrapper::new(rand::rng().random(), Box::new(resource_storage), wgpu_context.clone(), wake); + let desktop_wrapper = DesktopWrapper::new(rand::rng().random(), std::sync::Arc::new(resource_storage), wgpu_context.clone(), wake); Self { render_state: None, diff --git a/desktop/wrapper/src/lib.rs b/desktop/wrapper/src/lib.rs index 0a861d46ef..ad4c3f13c4 100644 --- a/desktop/wrapper/src/lib.rs +++ b/desktop/wrapper/src/lib.rs @@ -4,6 +4,7 @@ use graphite_editor::application::{Editor, Environment, Host, Platform}; use graphite_editor::messages::prelude::{FrontendMessage, Message, Wake}; use message_dispatcher::DesktopWrapperMessageDispatcher; use messages::{DesktopFrontendMessage, DesktopWrapperMessage}; +use std::sync::Arc; pub use graph_craft::application_io::resource::MmapResourceStorage; pub use graphite_editor::consts::{DOUBLE_CLICK_MILLISECONDS, FILE_EXTENSION}; @@ -24,7 +25,7 @@ pub struct DesktopWrapper { } impl DesktopWrapper { - pub fn new(uuid_random_seed: u64, resource_storage: Box, wgpu_context: WgpuContext, schedule_wake: Wake) -> Self { + pub fn new(uuid_random_seed: u64, resource_storage: Arc, wgpu_context: WgpuContext, schedule_wake: Wake) -> Self { #[cfg(target_os = "windows")] let host = Host::Windows; #[cfg(target_os = "macos")] diff --git a/editor/src/application.rs b/editor/src/application.rs index 81bbd0ef6e..81d4fe0157 100644 --- a/editor/src/application.rs +++ b/editor/src/application.rs @@ -3,14 +3,14 @@ use crate::messages::prelude::*; use graph_craft::application_io::PlatformApplicationIo; use graph_craft::application_io::resource::ResourceStorage; pub use graphene_std::uuid::*; -use std::sync::OnceLock; +use std::sync::{Arc, OnceLock}; pub struct Editor { pub dispatcher: Dispatcher, } impl Editor { - pub fn new(environment: Environment, uuid_random_seed: u64, resource_storage: Box, mut application_io: PlatformApplicationIo, wake: Wake) -> Self { + pub fn new(environment: Environment, uuid_random_seed: u64, resource_storage: Arc, mut application_io: PlatformApplicationIo, wake: Wake) -> Self { ENVIRONMENT.set(environment).expect("Editor shoud only be initialized once"); graphene_std::uuid::set_uuid_seed(uuid_random_seed); diff --git a/editor/src/dispatcher.rs b/editor/src/dispatcher.rs index b59b42fabb..229fc2e974 100644 --- a/editor/src/dispatcher.rs +++ b/editor/src/dispatcher.rs @@ -7,6 +7,7 @@ use crate::messages::preferences::preferences_message_handler::PreferencesMessag use crate::messages::prelude::*; use crate::messages::tool::common_functionality::utility_functions::make_path_editable_is_allowed; use graph_craft::application_io::resource::ResourceStorage; +use std::sync::Arc; #[derive(Debug, Default)] pub struct Dispatcher { @@ -39,7 +40,7 @@ pub struct DispatcherMessageHandlers { } impl DispatcherMessageHandlers { - pub fn with_resource_storage(resource_storage: Box) -> Self { + pub fn with_resource_storage(resource_storage: Arc) -> Self { Self { resource_storage_message_handler: ResourceStorageMessageHandler::new(resource_storage), ..Self::default() @@ -88,7 +89,7 @@ const DEBUG_MESSAGE_BLOCK_LIST: &[MessageDiscriminant] = &[ const DEBUG_MESSAGE_ENDING_BLOCK_LIST: &[&str] = &["PointerMove", "PointerOutsideViewport", "Overlays", "Draw", "CurrentTime", "Time"]; impl Dispatcher { - pub fn new(resource_storage: Box) -> Self { + pub fn new(resource_storage: Arc) -> Self { let mut s = Self::default(); s.message_handlers.resource_storage_message_handler = ResourceStorageMessageHandler::new(resource_storage); s 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 d0d00ab3c1..d4dd7405b8 100644 --- a/editor/src/messages/resource_storage/resource_storage_message_handler.rs +++ b/editor/src/messages/resource_storage/resource_storage_message_handler.rs @@ -1,29 +1,26 @@ use crate::messages::prelude::*; use graph_craft::application_io::resource::{LoadResource, ResourceFuture, ResourceHash, ResourceStorage}; -use std::sync::{Arc, RwLock}; +use std::sync::Arc; #[derive(Clone)] pub struct ResourcesHandle { - inner: Arc>>, + inner: Arc, } impl LoadResource for ResourcesHandle { - fn load(&self, hash: ResourceHash) -> ResourceFuture { - let guard = self.inner.read().unwrap(); - guard.load(hash) + fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> { + self.inner.load(hash) } } #[derive(ExtractField)] pub struct ResourceStorageMessageHandler { - storage: Option>>>, + storage: Option>, } impl ResourceStorageMessageHandler { - pub fn new(resource_storage: Box) -> Self { - Self { - storage: Some(Arc::new(RwLock::new(resource_storage))), - } + pub fn new(resource_storage: Arc) -> Self { + Self { storage: Some(resource_storage) } } pub fn resources(&self) -> Box { @@ -48,7 +45,7 @@ impl Default for ResourceStorageMessageHandler { #[cfg(test)] fn default() -> Self { Self { - storage: Some(Arc::new(RwLock::new(Box::new(graph_craft::application_io::resource::HashMapResourceStorage::new())))), + storage: Some(Arc::new(graph_craft::application_io::resource::HashMapResourceStorage::new())), } } } @@ -63,7 +60,6 @@ impl MessageHandler for R log::error!("Received resource message but storage is not initialized"); return; }; - let mut storage = storage.write().unwrap(); match message { ResourceStorageMessage::Store { data } => { diff --git a/frontend/wrapper/src/editor_wrapper.rs b/frontend/wrapper/src/editor_wrapper.rs index fe5ca658d0..3de6709876 100644 --- a/frontend/wrapper/src/editor_wrapper.rs +++ b/frontend/wrapper/src/editor_wrapper.rs @@ -85,11 +85,11 @@ impl EditorWrapper { _ => unreachable!(), }; - let storage: Box = match OpfsResourceStorage::load("resources").await { - Ok(storage) => Box::new(storage), + let storage: std::sync::Arc = match OpfsResourceStorage::load("resources").await { + Ok(storage) => std::sync::Arc::new(storage), Err(error) => { log::error!("Failed to open OPFS resource storage, falling back to in-memory: {error:?}"); - Box::new(graph_craft::application_io::resource::HashMapResourceStorage::new()) + std::sync::Arc::new(graph_craft::application_io::resource::HashMapResourceStorage::new()) } }; diff --git a/node-graph/graph-craft/src/application_io.rs b/node-graph/graph-craft/src/application_io.rs index ef5f9faacc..7462283404 100644 --- a/node-graph/graph-craft/src/application_io.rs +++ b/node-graph/graph-craft/src/application_io.rs @@ -60,7 +60,7 @@ impl ApplicationIo for PlatformApplicationIo { self.gpu_executor.as_ref() } - fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture { + fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture<'_> { self.resources.as_ref().expect("Resource storage not initialized").load(hash) } } diff --git a/node-graph/graph-craft/src/application_io/resource.rs b/node-graph/graph-craft/src/application_io/resource.rs index 63972f3e5a..46ab6d5c29 100644 --- a/node-graph/graph-craft/src/application_io/resource.rs +++ b/node-graph/graph-craft/src/application_io/resource.rs @@ -23,25 +23,25 @@ impl HashMapResourceStorage { } impl LoadResource for HashMapResourceStorage { - fn load(&self, hash: ResourceHash) -> ResourceFuture { + fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> { let result = self.resources.lock().unwrap().get(&hash).cloned(); Box::pin(async move { result }) } } impl ResourceStorage for HashMapResourceStorage { - fn store(&mut self, data: &[u8]) -> ResourceHash { + fn store(&self, data: &[u8]) -> ResourceHash { let hash = ResourceHash::from(data); - self.resources.get_mut().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data))); + self.resources.lock().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data))); hash } - fn contains(&mut self, hash: &ResourceHash) -> bool { - self.resources.get_mut().unwrap().contains_key(hash) + fn contains(&self, hash: &ResourceHash) -> bool { + self.resources.lock().unwrap().contains_key(hash) } - fn garbage_collect(&mut self, used: &[ResourceHash]) { + fn garbage_collect(&self, used: &[ResourceHash]) { let used_set: std::collections::HashSet<&ResourceHash> = used.iter().collect(); - self.resources.get_mut().unwrap().retain(|hash, _| used_set.contains(hash)); + self.resources.lock().unwrap().retain(|hash, _| used_set.contains(hash)); } } diff --git a/node-graph/graph-craft/src/application_io/resource/mmap.rs b/node-graph/graph-craft/src/application_io/resource/mmap.rs index 8f75ac4512..c5d618f469 100644 --- a/node-graph/graph-craft/src/application_io/resource/mmap.rs +++ b/node-graph/graph-craft/src/application_io/resource/mmap.rs @@ -61,14 +61,14 @@ impl MmapResourceStorage { } impl LoadResource for MmapResourceStorage { - fn load(&self, hash: ResourceHash) -> ResourceFuture { + fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> { let result = self.lookup(&hash); Box::pin(async move { result }) } } impl ResourceStorage for MmapResourceStorage { - fn store(&mut self, data: &[u8]) -> ResourceHash { + fn store(&self, data: &[u8]) -> ResourceHash { let hash = ResourceHash::from(data); let path = self.path_for(&hash); @@ -109,13 +109,13 @@ impl ResourceStorage for MmapResourceStorage { hash } - fn contains(&mut self, hash: &ResourceHash) -> bool { - self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists() + fn contains(&self, hash: &ResourceHash) -> bool { + self.cache.read().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists() } - fn garbage_collect(&mut self, used: &[ResourceHash]) { + fn garbage_collect(&self, used: &[ResourceHash]) { let used_set: std::collections::HashSet = used.iter().cloned().collect(); - self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).retain(|hash, _| used_set.contains(hash)); + self.cache.write().unwrap_or_else(|poisoned| poisoned.into_inner()).retain(|hash, _| used_set.contains(hash)); let Ok(top_entries) = fs::read_dir(&self.root) else { return }; for top_entry in top_entries.flatten() { 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 ab0fcf2492..261a172b64 100644 --- a/node-graph/graph-craft/src/application_io/resource/opfs.rs +++ b/node-graph/graph-craft/src/application_io/resource/opfs.rs @@ -50,7 +50,7 @@ impl OpfsResourceStorage { } impl LoadResource for OpfsResourceStorage { - fn load(&self, hash: ResourceHash) -> ResourceFuture { + fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> { let inner = self.inner.clone(); { @@ -74,7 +74,7 @@ impl LoadResource for OpfsResourceStorage { } impl ResourceStorage for OpfsResourceStorage { - fn store(&mut self, data: &[u8]) -> ResourceHash { + fn store(&self, data: &[u8]) -> ResourceHash { let hash = ResourceHash::from(data); let mut guard = self.inner.lock().unwrap(); @@ -95,12 +95,12 @@ impl ResourceStorage for OpfsResourceStorage { hash } - fn contains(&mut self, hash: &ResourceHash) -> bool { + fn contains(&self, hash: &ResourceHash) -> bool { let guard = self.inner.lock().unwrap(); guard.cache.contains_key(hash) || guard.on_disk.contains(hash) } - fn garbage_collect(&mut self, used: &[ResourceHash]) { + fn garbage_collect(&self, used: &[ResourceHash]) { let used: HashSet = used.iter().copied().collect(); let mut guard = self.inner.lock().unwrap(); diff --git a/node-graph/libraries/application-io/src/lib.rs b/node-graph/libraries/application-io/src/lib.rs index 915fba2d7c..88d13211c4 100644 --- a/node-graph/libraries/application-io/src/lib.rs +++ b/node-graph/libraries/application-io/src/lib.rs @@ -47,7 +47,7 @@ pub trait ApplicationIo { fn gpu_executor(&self) -> Option<&Self::Executor> { None } - fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture; + fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture<'_>; } impl ApplicationIo for &T { @@ -57,7 +57,7 @@ impl ApplicationIo for &T { (**self).gpu_executor() } - fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture { + fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture<'_> { (**self).load_resource(hash) } } diff --git a/node-graph/libraries/resources/src/lib.rs b/node-graph/libraries/resources/src/lib.rs index 780e00d041..319d4f2580 100644 --- a/node-graph/libraries/resources/src/lib.rs +++ b/node-graph/libraries/resources/src/lib.rs @@ -216,15 +216,15 @@ impl CacheHash for ResourceHash { } pub trait LoadResource: Send + Sync { - fn load(&self, hash: ResourceHash) -> ResourceFuture; + fn load(&self, hash: ResourceHash) -> ResourceFuture<'_>; } -pub type ResourceFuture = Pin> + Send + 'static>>; +pub type ResourceFuture<'a> = Pin> + Send + 'a>>; pub trait ResourceStorage: LoadResource { - fn store(&mut self, data: &[u8]) -> ResourceHash; - fn contains(&mut self, hash: &ResourceHash) -> bool; - fn garbage_collect(&mut self, used: &[ResourceHash]); + fn store(&self, data: &[u8]) -> ResourceHash; + fn contains(&self, hash: &ResourceHash) -> bool; + fn garbage_collect(&self, used: &[ResourceHash]); } #[repr(transparent)] From 32a96703b64400ec5048fea77fa0b49cd7bf6a07 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Mon, 1 Jun 2026 10:06:27 +0200 Subject: [PATCH 2/2] Adress review comments --- desktop/src/app.rs | 4 ++-- node-graph/graph-craft/src/application_io.rs | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/desktop/src/app.rs b/desktop/src/app.rs index 16c2022b21..fc417afa00 100644 --- a/desktop/src/app.rs +++ b/desktop/src/app.rs @@ -96,10 +96,10 @@ impl App { // Wake the winit event loop when an editor future completes. let wake_scheduler = app_event_scheduler.clone(); - let wake = std::sync::Arc::new(move || { + let wake = Arc::new(move || { wake_scheduler.schedule(AppEvent::DesktopWrapperMessage(DesktopWrapperMessage::Wake)); }); - let desktop_wrapper = DesktopWrapper::new(rand::rng().random(), std::sync::Arc::new(resource_storage), wgpu_context.clone(), wake); + let desktop_wrapper = DesktopWrapper::new(rand::rng().random(), Arc::new(resource_storage), wgpu_context.clone(), wake); Self { render_state: None, diff --git a/node-graph/graph-craft/src/application_io.rs b/node-graph/graph-craft/src/application_io.rs index 7462283404..1235d771d9 100644 --- a/node-graph/graph-craft/src/application_io.rs +++ b/node-graph/graph-craft/src/application_io.rs @@ -61,7 +61,13 @@ impl ApplicationIo for PlatformApplicationIo { } fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture<'_> { - self.resources.as_ref().expect("Resource storage not initialized").load(hash) + match self.resources.as_ref() { + Some(resources) => resources.load(hash), + None => { + log::error!("load_resource called before resource storage was initialized"); + Box::pin(std::future::ready(None)) + } + } } }