Skip to content

Commit 7b9c480

Browse files
TrueDoctorTrueDoctor
authored
Make ResourceStorage trait &self-based and remove 'static requirement (#4188)
* Make ResourceStorage trait &self-based with elided-lifetime ResourceFuture * Adress review comments
1 parent e32ff4b commit 7b9c480

12 files changed

Lines changed: 52 additions & 48 deletions

File tree

‎desktop/src/app.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@ impl App {
9696

9797
// Wake the winit event loop when an editor future completes.
9898
let wake_scheduler = app_event_scheduler.clone();
99-
let wake = std::sync::Arc::new(move || {
99+
let wake = Arc::new(move || {
100100
wake_scheduler.schedule(AppEvent::DesktopWrapperMessage(DesktopWrapperMessage::Wake));
101101
});
102-
let desktop_wrapper = DesktopWrapper::new(rand::rng().random(), Box::new(resource_storage), wgpu_context.clone(), wake);
102+
let desktop_wrapper = DesktopWrapper::new(rand::rng().random(), Arc::new(resource_storage), wgpu_context.clone(), wake);
103103

104104
Self {
105105
render_state: None,

‎desktop/wrapper/src/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use graphite_editor::application::{Editor, Environment, Host, Platform};
44
use graphite_editor::messages::prelude::{FrontendMessage, Message, Wake};
55
use message_dispatcher::DesktopWrapperMessageDispatcher;
66
use messages::{DesktopFrontendMessage, DesktopWrapperMessage};
7+
use std::sync::Arc;
78

89
pub use graph_craft::application_io::resource::MmapResourceStorage;
910
pub use graphite_editor::consts::{DOUBLE_CLICK_MILLISECONDS, FILE_EXTENSION};
@@ -24,7 +25,7 @@ pub struct DesktopWrapper {
2425
}
2526

2627
impl DesktopWrapper {
27-
pub fn new(uuid_random_seed: u64, resource_storage: Box<dyn ResourceStorage>, wgpu_context: WgpuContext, schedule_wake: Wake) -> Self {
28+
pub fn new(uuid_random_seed: u64, resource_storage: Arc<dyn ResourceStorage>, wgpu_context: WgpuContext, schedule_wake: Wake) -> Self {
2829
#[cfg(target_os = "windows")]
2930
let host = Host::Windows;
3031
#[cfg(target_os = "macos")]

‎editor/src/application.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@ use crate::messages::prelude::*;
33
use graph_craft::application_io::PlatformApplicationIo;
44
use graph_craft::application_io::resource::ResourceStorage;
55
pub use graphene_std::uuid::*;
6-
use std::sync::OnceLock;
6+
use std::sync::{Arc, OnceLock};
77

88
pub struct Editor {
99
pub dispatcher: Dispatcher,
1010
}
1111

1212
impl Editor {
13-
pub fn new(environment: Environment, uuid_random_seed: u64, resource_storage: Box<dyn ResourceStorage>, mut application_io: PlatformApplicationIo, wake: Wake) -> Self {
13+
pub fn new(environment: Environment, uuid_random_seed: u64, resource_storage: Arc<dyn ResourceStorage>, mut application_io: PlatformApplicationIo, wake: Wake) -> Self {
1414
ENVIRONMENT.set(environment).expect("Editor shoud only be initialized once");
1515
graphene_std::uuid::set_uuid_seed(uuid_random_seed);
1616

‎editor/src/dispatcher.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::messages::preferences::preferences_message_handler::PreferencesMessag
77
use crate::messages::prelude::*;
88
use crate::messages::tool::common_functionality::utility_functions::make_path_editable_is_allowed;
99
use graph_craft::application_io::resource::ResourceStorage;
10+
use std::sync::Arc;
1011

1112
#[derive(Debug, Default)]
1213
pub struct Dispatcher {
@@ -39,7 +40,7 @@ pub struct DispatcherMessageHandlers {
3940
}
4041

4142
impl DispatcherMessageHandlers {
42-
pub fn with_resource_storage(resource_storage: Box<dyn ResourceStorage>) -> Self {
43+
pub fn with_resource_storage(resource_storage: Arc<dyn ResourceStorage>) -> Self {
4344
Self {
4445
resource_storage_message_handler: ResourceStorageMessageHandler::new(resource_storage),
4546
..Self::default()
@@ -88,7 +89,7 @@ const DEBUG_MESSAGE_BLOCK_LIST: &[MessageDiscriminant] = &[
8889
const DEBUG_MESSAGE_ENDING_BLOCK_LIST: &[&str] = &["PointerMove", "PointerOutsideViewport", "Overlays", "Draw", "CurrentTime", "Time"];
8990

9091
impl Dispatcher {
91-
pub fn new(resource_storage: Box<dyn ResourceStorage>) -> Self {
92+
pub fn new(resource_storage: Arc<dyn ResourceStorage>) -> Self {
9293
let mut s = Self::default();
9394
s.message_handlers.resource_storage_message_handler = ResourceStorageMessageHandler::new(resource_storage);
9495
s

‎editor/src/messages/resource_storage/resource_storage_message_handler.rs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,26 @@
11
use crate::messages::prelude::*;
22
use graph_craft::application_io::resource::{LoadResource, ResourceFuture, ResourceHash, ResourceStorage};
3-
use std::sync::{Arc, RwLock};
3+
use std::sync::Arc;
44

55
#[derive(Clone)]
66
pub struct ResourcesHandle {
7-
inner: Arc<RwLock<Box<dyn ResourceStorage>>>,
7+
inner: Arc<dyn ResourceStorage>,
88
}
99

1010
impl LoadResource for ResourcesHandle {
11-
fn load(&self, hash: ResourceHash) -> ResourceFuture {
12-
let guard = self.inner.read().unwrap();
13-
guard.load(hash)
11+
fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
12+
self.inner.load(hash)
1413
}
1514
}
1615

1716
#[derive(ExtractField)]
1817
pub struct ResourceStorageMessageHandler {
19-
storage: Option<Arc<RwLock<Box<dyn ResourceStorage>>>>,
18+
storage: Option<Arc<dyn ResourceStorage>>,
2019
}
2120

2221
impl ResourceStorageMessageHandler {
23-
pub fn new(resource_storage: Box<dyn ResourceStorage>) -> Self {
24-
Self {
25-
storage: Some(Arc::new(RwLock::new(resource_storage))),
26-
}
22+
pub fn new(resource_storage: Arc<dyn ResourceStorage>) -> Self {
23+
Self { storage: Some(resource_storage) }
2724
}
2825

2926
pub fn resources(&self) -> Box<dyn LoadResource> {
@@ -48,7 +45,7 @@ impl Default for ResourceStorageMessageHandler {
4845
#[cfg(test)]
4946
fn default() -> Self {
5047
Self {
51-
storage: Some(Arc::new(RwLock::new(Box::new(graph_craft::application_io::resource::HashMapResourceStorage::new())))),
48+
storage: Some(Arc::new(graph_craft::application_io::resource::HashMapResourceStorage::new())),
5249
}
5350
}
5451
}
@@ -63,7 +60,6 @@ impl MessageHandler<ResourceStorageMessage, ResourceStorageMessageContext> for R
6360
log::error!("Received resource message but storage is not initialized");
6461
return;
6562
};
66-
let mut storage = storage.write().unwrap();
6763

6864
match message {
6965
ResourceStorageMessage::Store { data } => {

‎frontend/wrapper/src/editor_wrapper.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ impl EditorWrapper {
8585
_ => unreachable!(),
8686
};
8787

88-
let storage: Box<dyn ResourceStorage> = match OpfsResourceStorage::load("resources").await {
89-
Ok(storage) => Box::new(storage),
88+
let storage: std::sync::Arc<dyn ResourceStorage> = match OpfsResourceStorage::load("resources").await {
89+
Ok(storage) => std::sync::Arc::new(storage),
9090
Err(error) => {
9191
log::error!("Failed to open OPFS resource storage, falling back to in-memory: {error:?}");
92-
Box::new(graph_craft::application_io::resource::HashMapResourceStorage::new())
92+
std::sync::Arc::new(graph_craft::application_io::resource::HashMapResourceStorage::new())
9393
}
9494
};
9595

‎node-graph/graph-craft/src/application_io.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,14 @@ impl ApplicationIo for PlatformApplicationIo {
6060
self.gpu_executor.as_ref()
6161
}
6262

63-
fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture {
64-
self.resources.as_ref().expect("Resource storage not initialized").load(hash)
63+
fn load_resource(&self, hash: resource::ResourceHash) -> resource::ResourceFuture<'_> {
64+
match self.resources.as_ref() {
65+
Some(resources) => resources.load(hash),
66+
None => {
67+
log::error!("load_resource called before resource storage was initialized");
68+
Box::pin(std::future::ready(None))
69+
}
70+
}
6571
}
6672
}
6773

‎node-graph/graph-craft/src/application_io/resource.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,25 @@ impl HashMapResourceStorage {
2323
}
2424

2525
impl LoadResource for HashMapResourceStorage {
26-
fn load(&self, hash: ResourceHash) -> ResourceFuture {
26+
fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
2727
let result = self.resources.lock().unwrap().get(&hash).cloned();
2828
Box::pin(async move { result })
2929
}
3030
}
3131

3232
impl ResourceStorage for HashMapResourceStorage {
33-
fn store(&mut self, data: &[u8]) -> ResourceHash {
33+
fn store(&self, data: &[u8]) -> ResourceHash {
3434
let hash = ResourceHash::from(data);
35-
self.resources.get_mut().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data)));
35+
self.resources.lock().unwrap().insert(hash, Resource::new(Arc::<[u8]>::from(data)));
3636
hash
3737
}
3838

39-
fn contains(&mut self, hash: &ResourceHash) -> bool {
40-
self.resources.get_mut().unwrap().contains_key(hash)
39+
fn contains(&self, hash: &ResourceHash) -> bool {
40+
self.resources.lock().unwrap().contains_key(hash)
4141
}
4242

43-
fn garbage_collect(&mut self, used: &[ResourceHash]) {
43+
fn garbage_collect(&self, used: &[ResourceHash]) {
4444
let used_set: std::collections::HashSet<&ResourceHash> = used.iter().collect();
45-
self.resources.get_mut().unwrap().retain(|hash, _| used_set.contains(hash));
45+
self.resources.lock().unwrap().retain(|hash, _| used_set.contains(hash));
4646
}
4747
}

‎node-graph/graph-craft/src/application_io/resource/mmap.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,14 @@ impl MmapResourceStorage {
6161
}
6262

6363
impl LoadResource for MmapResourceStorage {
64-
fn load(&self, hash: ResourceHash) -> ResourceFuture {
64+
fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
6565
let result = self.lookup(&hash);
6666
Box::pin(async move { result })
6767
}
6868
}
6969

7070
impl ResourceStorage for MmapResourceStorage {
71-
fn store(&mut self, data: &[u8]) -> ResourceHash {
71+
fn store(&self, data: &[u8]) -> ResourceHash {
7272
let hash = ResourceHash::from(data);
7373
let path = self.path_for(&hash);
7474

@@ -109,13 +109,13 @@ impl ResourceStorage for MmapResourceStorage {
109109
hash
110110
}
111111

112-
fn contains(&mut self, hash: &ResourceHash) -> bool {
113-
self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists()
112+
fn contains(&self, hash: &ResourceHash) -> bool {
113+
self.cache.read().unwrap_or_else(|poisoned| poisoned.into_inner()).contains_key(hash) || self.path_for(hash).exists()
114114
}
115115

116-
fn garbage_collect(&mut self, used: &[ResourceHash]) {
116+
fn garbage_collect(&self, used: &[ResourceHash]) {
117117
let used_set: std::collections::HashSet<ResourceHash> = used.iter().cloned().collect();
118-
self.cache.get_mut().unwrap_or_else(|poisoned| poisoned.into_inner()).retain(|hash, _| used_set.contains(hash));
118+
self.cache.write().unwrap_or_else(|poisoned| poisoned.into_inner()).retain(|hash, _| used_set.contains(hash));
119119

120120
let Ok(top_entries) = fs::read_dir(&self.root) else { return };
121121
for top_entry in top_entries.flatten() {

‎node-graph/graph-craft/src/application_io/resource/opfs.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl OpfsResourceStorage {
5050
}
5151

5252
impl LoadResource for OpfsResourceStorage {
53-
fn load(&self, hash: ResourceHash) -> ResourceFuture {
53+
fn load(&self, hash: ResourceHash) -> ResourceFuture<'_> {
5454
let inner = self.inner.clone();
5555

5656
{
@@ -74,7 +74,7 @@ impl LoadResource for OpfsResourceStorage {
7474
}
7575

7676
impl ResourceStorage for OpfsResourceStorage {
77-
fn store(&mut self, data: &[u8]) -> ResourceHash {
77+
fn store(&self, data: &[u8]) -> ResourceHash {
7878
let hash = ResourceHash::from(data);
7979
let mut guard = self.inner.lock().unwrap();
8080

@@ -95,12 +95,12 @@ impl ResourceStorage for OpfsResourceStorage {
9595
hash
9696
}
9797

98-
fn contains(&mut self, hash: &ResourceHash) -> bool {
98+
fn contains(&self, hash: &ResourceHash) -> bool {
9999
let guard = self.inner.lock().unwrap();
100100
guard.cache.contains_key(hash) || guard.on_disk.contains(hash)
101101
}
102102

103-
fn garbage_collect(&mut self, used: &[ResourceHash]) {
103+
fn garbage_collect(&self, used: &[ResourceHash]) {
104104
let used: HashSet<ResourceHash> = used.iter().copied().collect();
105105
let mut guard = self.inner.lock().unwrap();
106106

0 commit comments

Comments
 (0)