Make ResourceStorage trait &self-based and remove 'static requirement (#4188)

* Make ResourceStorage trait &self-based with elided-lifetime ResourceFuture

* Adress review comments
This commit is contained in:
Dennis Kobert
2026-06-01 10:37:05 +00:00
committed by GitHub
parent e32ff4b7f0
commit 7b9c480a8d
12 changed files with 52 additions and 48 deletions
+2 -2
View File
@@ -96,10 +96,10 @@ impl App {
// Wake the winit event loop when an editor future completes. // Wake the winit event loop when an editor future completes.
let wake_scheduler = app_event_scheduler.clone(); 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)); 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(), Arc::new(resource_storage), wgpu_context.clone(), wake);
Self { Self {
render_state: None, render_state: None,
+2 -1
View File
@@ -4,6 +4,7 @@ use graphite_editor::application::{Editor, Environment, Host, Platform};
use graphite_editor::messages::prelude::{FrontendMessage, Message, Wake}; use graphite_editor::messages::prelude::{FrontendMessage, Message, Wake};
use message_dispatcher::DesktopWrapperMessageDispatcher; use message_dispatcher::DesktopWrapperMessageDispatcher;
use messages::{DesktopFrontendMessage, DesktopWrapperMessage}; use messages::{DesktopFrontendMessage, DesktopWrapperMessage};
use std::sync::Arc;
pub use graph_craft::application_io::resource::MmapResourceStorage; pub use graph_craft::application_io::resource::MmapResourceStorage;
pub use graphite_editor::consts::{DOUBLE_CLICK_MILLISECONDS, FILE_EXTENSION}; pub use graphite_editor::consts::{DOUBLE_CLICK_MILLISECONDS, FILE_EXTENSION};
@@ -24,7 +25,7 @@ pub struct DesktopWrapper {
} }
impl DesktopWrapper { impl DesktopWrapper {
pub fn new(uuid_random_seed: u64, resource_storage: Box<dyn ResourceStorage>, wgpu_context: WgpuContext, schedule_wake: Wake) -> Self { pub fn new(uuid_random_seed: u64, resource_storage: Arc<dyn ResourceStorage>, wgpu_context: WgpuContext, schedule_wake: Wake) -> Self {
#[cfg(target_os = "windows")] #[cfg(target_os = "windows")]
let host = Host::Windows; let host = Host::Windows;
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
+2 -2
View File
@@ -3,14 +3,14 @@ use crate::messages::prelude::*;
use graph_craft::application_io::PlatformApplicationIo; use graph_craft::application_io::PlatformApplicationIo;
use graph_craft::application_io::resource::ResourceStorage; use graph_craft::application_io::resource::ResourceStorage;
pub use graphene_std::uuid::*; pub use graphene_std::uuid::*;
use std::sync::OnceLock; use std::sync::{Arc, OnceLock};
pub struct Editor { pub struct Editor {
pub dispatcher: Dispatcher, pub dispatcher: Dispatcher,
} }
impl Editor { impl Editor {
pub fn new(environment: Environment, uuid_random_seed: u64, resource_storage: Box<dyn ResourceStorage>, mut application_io: PlatformApplicationIo, wake: Wake) -> Self { pub fn new(environment: Environment, uuid_random_seed: u64, resource_storage: Arc<dyn ResourceStorage>, mut application_io: PlatformApplicationIo, wake: Wake) -> Self {
ENVIRONMENT.set(environment).expect("Editor shoud only be initialized once"); ENVIRONMENT.set(environment).expect("Editor shoud only be initialized once");
graphene_std::uuid::set_uuid_seed(uuid_random_seed); graphene_std::uuid::set_uuid_seed(uuid_random_seed);
+3 -2
View File
@@ -7,6 +7,7 @@ use crate::messages::preferences::preferences_message_handler::PreferencesMessag
use crate::messages::prelude::*; use crate::messages::prelude::*;
use crate::messages::tool::common_functionality::utility_functions::make_path_editable_is_allowed; use crate::messages::tool::common_functionality::utility_functions::make_path_editable_is_allowed;
use graph_craft::application_io::resource::ResourceStorage; use graph_craft::application_io::resource::ResourceStorage;
use std::sync::Arc;
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct Dispatcher { pub struct Dispatcher {
@@ -39,7 +40,7 @@ pub struct DispatcherMessageHandlers {
} }
impl DispatcherMessageHandlers { impl DispatcherMessageHandlers {
pub fn with_resource_storage(resource_storage: Box<dyn ResourceStorage>) -> Self { pub fn with_resource_storage(resource_storage: Arc<dyn ResourceStorage>) -> Self {
Self { Self {
resource_storage_message_handler: ResourceStorageMessageHandler::new(resource_storage), resource_storage_message_handler: ResourceStorageMessageHandler::new(resource_storage),
..Self::default() ..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"]; const DEBUG_MESSAGE_ENDING_BLOCK_LIST: &[&str] = &["PointerMove", "PointerOutsideViewport", "Overlays", "Draw", "CurrentTime", "Time"];
impl Dispatcher { impl Dispatcher {
pub fn new(resource_storage: Box<dyn ResourceStorage>) -> Self { pub fn new(resource_storage: Arc<dyn ResourceStorage>) -> Self {
let mut s = Self::default(); let mut s = Self::default();
s.message_handlers.resource_storage_message_handler = ResourceStorageMessageHandler::new(resource_storage); s.message_handlers.resource_storage_message_handler = ResourceStorageMessageHandler::new(resource_storage);
s s
@@ -1,29 +1,26 @@
use crate::messages::prelude::*; use crate::messages::prelude::*;
use graph_craft::application_io::resource::{LoadResource, ResourceFuture, ResourceHash, ResourceStorage}; use graph_craft::application_io::resource::{LoadResource, ResourceFuture, ResourceHash, ResourceStorage};
use std::sync::{Arc, RwLock}; use std::sync::Arc;
#[derive(Clone)] #[derive(Clone)]
pub struct ResourcesHandle { pub struct ResourcesHandle {
inner: Arc<RwLock<Box<dyn ResourceStorage>>>, inner: Arc<dyn ResourceStorage>,
} }
impl LoadResource for ResourcesHandle { impl LoadResource for ResourcesHandle {
fn load(&self, hash: ResourceHash) -> ResourceFuture { fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
let guard = self.inner.read().unwrap(); self.inner.load(hash)
guard.load(hash)
} }
} }
#[derive(ExtractField)] #[derive(ExtractField)]
pub struct ResourceStorageMessageHandler { pub struct ResourceStorageMessageHandler {
storage: Option<Arc<RwLock<Box<dyn ResourceStorage>>>>, storage: Option<Arc<dyn ResourceStorage>>,
} }
impl ResourceStorageMessageHandler { impl ResourceStorageMessageHandler {
pub fn new(resource_storage: Box<dyn ResourceStorage>) -> Self { pub fn new(resource_storage: Arc<dyn ResourceStorage>) -> Self {
Self { Self { storage: Some(resource_storage) }
storage: Some(Arc::new(RwLock::new(resource_storage))),
}
} }
pub fn resources(&self) -> Box<dyn LoadResource> { pub fn resources(&self) -> Box<dyn LoadResource> {
@@ -48,7 +45,7 @@ impl Default for ResourceStorageMessageHandler {
#[cfg(test)] #[cfg(test)]
fn default() -> Self { fn default() -> Self {
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<ResourceStorageMessage, ResourceStorageMessageContext> for R
log::error!("Received resource message but storage is not initialized"); log::error!("Received resource message but storage is not initialized");
return; return;
}; };
let mut storage = storage.write().unwrap();
match message { match message {
ResourceStorageMessage::Store { data } => { ResourceStorageMessage::Store { data } => {
+3 -3
View File
@@ -85,11 +85,11 @@ impl EditorWrapper {
_ => unreachable!(), _ => unreachable!(),
}; };
let storage: Box<dyn ResourceStorage> = match OpfsResourceStorage::load("resources").await { let storage: std::sync::Arc<dyn ResourceStorage> = match OpfsResourceStorage::load("resources").await {
Ok(storage) => Box::new(storage), Ok(storage) => std::sync::Arc::new(storage),
Err(error) => { Err(error) => {
log::error!("Failed to open OPFS resource storage, falling back to in-memory: {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())
} }
}; };
+8 -2
View File
@@ -60,8 +60,14 @@ impl ApplicationIo for PlatformApplicationIo {
self.gpu_executor.as_ref() 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) 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))
}
}
} }
} }
@@ -23,25 +23,25 @@ impl HashMapResourceStorage {
} }
impl LoadResource for 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(); let result = self.resources.lock().unwrap().get(&hash).cloned();
Box::pin(async move { result }) Box::pin(async move { result })
} }
} }
impl ResourceStorage for HashMapResourceStorage { impl ResourceStorage for HashMapResourceStorage {
fn store(&mut self, data: &[u8]) -> ResourceHash { fn store(&self, data: &[u8]) -> ResourceHash {
let hash = ResourceHash::from(data); 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 hash
} }
fn contains(&mut self, hash: &ResourceHash) -> bool { fn contains(&self, hash: &ResourceHash) -> bool {
self.resources.get_mut().unwrap().contains_key(hash) 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(); 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));
} }
} }
@@ -61,14 +61,14 @@ impl MmapResourceStorage {
} }
impl LoadResource for MmapResourceStorage { impl LoadResource for MmapResourceStorage {
fn load(&self, hash: ResourceHash) -> ResourceFuture { fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
let result = self.lookup(&hash); let result = self.lookup(&hash);
Box::pin(async move { result }) Box::pin(async move { result })
} }
} }
impl ResourceStorage for MmapResourceStorage { impl ResourceStorage for MmapResourceStorage {
fn store(&mut self, data: &[u8]) -> ResourceHash { fn store(&self, data: &[u8]) -> ResourceHash {
let hash = ResourceHash::from(data); let hash = ResourceHash::from(data);
let path = self.path_for(&hash); let path = self.path_for(&hash);
@@ -109,13 +109,13 @@ impl ResourceStorage for MmapResourceStorage {
hash hash
} }
fn contains(&mut self, hash: &ResourceHash) -> bool { fn contains(&self, hash: &ResourceHash) -> bool {
self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists() 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<ResourceHash> = used.iter().cloned().collect(); let used_set: std::collections::HashSet<ResourceHash> = 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 }; let Ok(top_entries) = fs::read_dir(&self.root) else { return };
for top_entry in top_entries.flatten() { for top_entry in top_entries.flatten() {
@@ -50,7 +50,7 @@ impl OpfsResourceStorage {
} }
impl LoadResource for OpfsResourceStorage { impl LoadResource for OpfsResourceStorage {
fn load(&self, hash: ResourceHash) -> ResourceFuture { fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
let inner = self.inner.clone(); let inner = self.inner.clone();
{ {
@@ -74,7 +74,7 @@ impl LoadResource for OpfsResourceStorage {
} }
impl ResourceStorage 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 hash = ResourceHash::from(data);
let mut guard = self.inner.lock().unwrap(); let mut guard = self.inner.lock().unwrap();
@@ -95,12 +95,12 @@ impl ResourceStorage for OpfsResourceStorage {
hash hash
} }
fn contains(&mut self, hash: &ResourceHash) -> bool { fn contains(&self, hash: &ResourceHash) -> bool {
let guard = self.inner.lock().unwrap(); let guard = self.inner.lock().unwrap();
guard.cache.contains_key(hash) || guard.on_disk.contains(hash) 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<ResourceHash> = used.iter().copied().collect(); let used: HashSet<ResourceHash> = used.iter().copied().collect();
let mut guard = self.inner.lock().unwrap(); let mut guard = self.inner.lock().unwrap();
@@ -47,7 +47,7 @@ pub trait ApplicationIo {
fn gpu_executor(&self) -> Option<&Self::Executor> { fn gpu_executor(&self) -> Option<&Self::Executor> {
None None
} }
fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture; fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture<'_>;
} }
impl<T: ApplicationIo> ApplicationIo for &T { impl<T: ApplicationIo> ApplicationIo for &T {
@@ -57,7 +57,7 @@ impl<T: ApplicationIo> ApplicationIo for &T {
(**self).gpu_executor() (**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) (**self).load_resource(hash)
} }
} }
+5 -5
View File
@@ -216,15 +216,15 @@ impl CacheHash for ResourceHash {
} }
pub trait LoadResource: Send + Sync { pub trait LoadResource: Send + Sync {
fn load(&self, hash: ResourceHash) -> ResourceFuture; fn load(&self, hash: ResourceHash) -> ResourceFuture<'_>;
} }
pub type ResourceFuture = Pin<Box<dyn Future<Output = Option<Resource>> + Send + 'static>>; pub type ResourceFuture<'a> = Pin<Box<dyn Future<Output = Option<Resource>> + Send + 'a>>;
pub trait ResourceStorage: LoadResource { pub trait ResourceStorage: LoadResource {
fn store(&mut self, data: &[u8]) -> ResourceHash; fn store(&self, data: &[u8]) -> ResourceHash;
fn contains(&mut self, hash: &ResourceHash) -> bool; fn contains(&self, hash: &ResourceHash) -> bool;
fn garbage_collect(&mut self, used: &[ResourceHash]); fn garbage_collect(&self, used: &[ResourceHash]);
} }
#[repr(transparent)] #[repr(transparent)]