From 1ac08335c278054f2608a16dbcc3294cd44cfcaa Mon Sep 17 00:00:00 2001 From: Felipe Jorge Date: Sat, 5 Jun 2021 21:46:02 -0300 Subject: [PATCH 1/6] add unsafe handle serialization --- crates/bevy_asset/src/lib.rs | 2 + crates/bevy_asset/src/ser.rs | 91 ++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 crates/bevy_asset/src/ser.rs diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 62808e38a42c2..1d2a278767e18 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -11,6 +11,7 @@ mod info; mod io; mod loader; mod path; +mod ser; pub mod prelude { #[doc(hidden)] @@ -25,6 +26,7 @@ pub use info::*; pub use io::*; pub use loader::*; pub use path::*; +pub use ser::*; use bevy_app::{prelude::Plugin, AppBuilder}; use bevy_ecs::{ diff --git a/crates/bevy_asset/src/ser.rs b/crates/bevy_asset/src/ser.rs new file mode 100644 index 0000000000000..2abc9b5976604 --- /dev/null +++ b/crates/bevy_asset/src/ser.rs @@ -0,0 +1,91 @@ +use std::{cell::Cell, ptr::NonNull}; + +use serde::{Deserialize, Deserializer, Serialize, Serializer}; + +use crate::{Asset, AssetServer, Handle}; + +/////////////////////////////////////////////////////////////////////////////// + +thread_local! { + static ASSET_SERVER: Cell>> = Cell::new(None); +} + +impl Serialize for Handle { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let path = ASSET_SERVER.with(|server| { + server.get().and_then(|ptr| { + // SAFETY: The thread local [`ASSET_SERVER`] can only + // be set by a valid [`AssetServer`] instance that will + // also make sure to set it back to [`None`] once it's no longer is valid + let server = unsafe { ptr.as_ref() }; + // TODO: `get_handle_path` does absolutely nothing issue #1290 + server.get_handle_path(self).map(|asset_path| { + let mut path = asset_path.path().to_string_lossy().to_string(); + if let Some(label) = asset_path.label() { + path.push('#'); + path.push_str(label); + } + path + }) + }) + }); + + path.serialize(serializer) + } +} + +impl<'de, T: Asset> Deserialize<'de> for Handle { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok(Option::::deserialize(deserializer)? + .and_then(|path| { + ASSET_SERVER.with(|server| { + server.get().map(|ptr| { + // SAFETY: The thread local [`ASSET_SERVER`] can only + // be set by a valid [`AssetServer`] instance that will + // also make sure to set it to back [`None`] once it's no longer is valid + let server = unsafe { ptr.as_ref() }; + server.load(path.as_str()) + }) + }) + }) + .unwrap_or_default()) + } +} + +impl AssetServer { + pub fn serialize_with_asset_refs( + &self, + serializer: S, + value: &T, + ) -> Result + where + S: Serializer, + T: Serialize, + { + ASSET_SERVER.with(|key| { + key.replace(NonNull::new(self as *const _ as *mut _)); + let result = value.serialize(serializer); + key.replace(None); + result + }) + } + + pub fn deserialize_with_asset_refs<'de, D, T>(&self, deserializer: D) -> Result + where + D: Deserializer<'de>, + T: Deserialize<'de>, + { + ASSET_SERVER.with(|key| { + key.replace(NonNull::new(self as *const _ as *mut _)); + let result = T::deserialize(deserializer); + key.replace(None); + result + }) + } +} From 3e09b8c7da01f63a2f708d5c2c0f83272864d971 Mon Sep 17 00:00:00 2001 From: Felipe Jorge Date: Sat, 5 Jun 2021 21:50:44 -0300 Subject: [PATCH 2/6] safe handle serialization --- crates/bevy_asset/src/ser.rs | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/crates/bevy_asset/src/ser.rs b/crates/bevy_asset/src/ser.rs index 2abc9b5976604..6284f7bcf1a6f 100644 --- a/crates/bevy_asset/src/ser.rs +++ b/crates/bevy_asset/src/ser.rs @@ -1,4 +1,4 @@ -use std::{cell::Cell, ptr::NonNull}; +use std::cell::Cell; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -7,7 +7,7 @@ use crate::{Asset, AssetServer, Handle}; /////////////////////////////////////////////////////////////////////////////// thread_local! { - static ASSET_SERVER: Cell>> = Cell::new(None); + static ASSET_SERVER: Cell> = Cell::new(None); } impl Serialize for Handle { @@ -15,13 +15,10 @@ impl Serialize for Handle { where S: Serializer, { - let path = ASSET_SERVER.with(|server| { - server.get().and_then(|ptr| { - // SAFETY: The thread local [`ASSET_SERVER`] can only - // be set by a valid [`AssetServer`] instance that will - // also make sure to set it back to [`None`] once it's no longer is valid - let server = unsafe { ptr.as_ref() }; - // TODO: `get_handle_path` does absolutely nothing issue #1290 + let path = ASSET_SERVER.with(|cell| { + let server = cell.replace(None); + let path = server.as_ref().and_then(|server| { + // TODO: `get_handle_path` does absolutely nothing issue #1290 server.get_handle_path(self).map(|asset_path| { let mut path = asset_path.path().to_string_lossy().to_string(); if let Some(label) = asset_path.label() { @@ -30,7 +27,9 @@ impl Serialize for Handle { } path }) - }) + }); + cell.replace(server); + path }); path.serialize(serializer) @@ -44,14 +43,11 @@ impl<'de, T: Asset> Deserialize<'de> for Handle { { Ok(Option::::deserialize(deserializer)? .and_then(|path| { - ASSET_SERVER.with(|server| { - server.get().map(|ptr| { - // SAFETY: The thread local [`ASSET_SERVER`] can only - // be set by a valid [`AssetServer`] instance that will - // also make sure to set it to back [`None`] once it's no longer is valid - let server = unsafe { ptr.as_ref() }; - server.load(path.as_str()) - }) + ASSET_SERVER.with(|cell| { + let server = cell.replace(None); + let handle = server.as_ref().map(|server| server.load(path.as_str())); + cell.replace(server); + handle }) }) .unwrap_or_default()) @@ -69,7 +65,7 @@ impl AssetServer { T: Serialize, { ASSET_SERVER.with(|key| { - key.replace(NonNull::new(self as *const _ as *mut _)); + key.replace(Some(self.clone())); let result = value.serialize(serializer); key.replace(None); result @@ -82,7 +78,7 @@ impl AssetServer { T: Deserialize<'de>, { ASSET_SERVER.with(|key| { - key.replace(NonNull::new(self as *const _ as *mut _)); + key.replace(Some(self.clone())); let result = T::deserialize(deserializer); key.replace(None); result From 2d8f84946d77ad35c9033f08bd51ad5ae92c6280 Mon Sep 17 00:00:00 2001 From: Felipe Jorge Date: Sun, 6 Jun 2021 00:12:34 -0300 Subject: [PATCH 3/6] serialize local asset references --- crates/bevy_asset/src/ser.rs | 75 +++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/crates/bevy_asset/src/ser.rs b/crates/bevy_asset/src/ser.rs index 6284f7bcf1a6f..2a7313d89080f 100644 --- a/crates/bevy_asset/src/ser.rs +++ b/crates/bevy_asset/src/ser.rs @@ -1,8 +1,9 @@ use std::cell::Cell; +use bevy_reflect::Uuid; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use crate::{Asset, AssetServer, Handle}; +use crate::{Asset, AssetServer, Handle, HandleId, HandleUntyped}; /////////////////////////////////////////////////////////////////////////////// @@ -10,27 +11,44 @@ thread_local! { static ASSET_SERVER: Cell> = Cell::new(None); } +#[derive(Serialize, Deserialize)] +enum AssetRef { + Default, + /// Used for static handles like the `PBR_PIPELINE_HANDLE` or a embedded assets + Local(Uuid, u64), + /// Loads form a file + External(String), +} + impl Serialize for Handle { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - let path = ASSET_SERVER.with(|cell| { - let server = cell.replace(None); - let path = server.as_ref().and_then(|server| { - // TODO: `get_handle_path` does absolutely nothing issue #1290 - server.get_handle_path(self).map(|asset_path| { - let mut path = asset_path.path().to_string_lossy().to_string(); - if let Some(label) = asset_path.label() { - path.push('#'); - path.push_str(label); - } - path - }) + let path = ASSET_SERVER + .with(|cell| { + let server = cell.replace(None); + let path = server.as_ref().and_then(|server| { + // TODO: `get_handle_path` does absolutely nothing issue #1290 + server + .get_handle_path(self) + .map(|asset_path| { + let mut path = asset_path.path().to_string_lossy().to_string(); + if let Some(label) = asset_path.label() { + path.push('#'); + path.push_str(label); + } + path + }) + .and_then(|path| Some(AssetRef::External(path))) + }); + cell.replace(server); + path + }) + .unwrap_or_else(|| match &self.id { + HandleId::Id(type_uuid, id) => AssetRef::Local(*type_uuid, *id), + HandleId::AssetPathId(_) => AssetRef::Default, }); - cell.replace(server); - path - }); path.serialize(serializer) } @@ -41,16 +59,21 @@ impl<'de, T: Asset> Deserialize<'de> for Handle { where D: Deserializer<'de>, { - Ok(Option::::deserialize(deserializer)? - .and_then(|path| { - ASSET_SERVER.with(|cell| { - let server = cell.replace(None); - let handle = server.as_ref().map(|server| server.load(path.as_str())); - cell.replace(server); - handle - }) - }) - .unwrap_or_default()) + match AssetRef::deserialize(deserializer)? { + AssetRef::Default => Ok(Handle::default()), + AssetRef::Local(type_uuid, id) => { + Ok(HandleUntyped::weak_from_u64(type_uuid, id).typed()) + } + AssetRef::External(path) => ASSET_SERVER.with(|cell| { + let server = cell.replace(None); + let handle = server + .as_ref() + .map(|server| server.load(path.as_str())) + .unwrap_or_default(); + cell.replace(server); + Ok(handle) + }), + } } } From 2c4ce2e08d85d1519ad97ca4b8ffd8a228c51ca4 Mon Sep 17 00:00:00 2001 From: Felipe Jorge Date: Sun, 6 Jun 2021 00:18:00 -0300 Subject: [PATCH 4/6] clippy fix --- crates/bevy_asset/src/ser.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/bevy_asset/src/ser.rs b/crates/bevy_asset/src/ser.rs index 2a7313d89080f..4ef4e94d12fcf 100644 --- a/crates/bevy_asset/src/ser.rs +++ b/crates/bevy_asset/src/ser.rs @@ -30,17 +30,14 @@ impl Serialize for Handle { let server = cell.replace(None); let path = server.as_ref().and_then(|server| { // TODO: `get_handle_path` does absolutely nothing issue #1290 - server - .get_handle_path(self) - .map(|asset_path| { - let mut path = asset_path.path().to_string_lossy().to_string(); - if let Some(label) = asset_path.label() { - path.push('#'); - path.push_str(label); - } - path - }) - .and_then(|path| Some(AssetRef::External(path))) + server.get_handle_path(self).map(|asset_path| { + let mut path = asset_path.path().to_string_lossy().to_string(); + if let Some(label) = asset_path.label() { + path.push('#'); + path.push_str(label); + } + AssetRef::External(path) + }) }); cell.replace(server); path From e71c300cd60068a0d2da4049fb484c7566393fa9 Mon Sep 17 00:00:00 2001 From: Felipe Jorge Date: Sun, 6 Jun 2021 00:30:40 -0300 Subject: [PATCH 5/6] typo --- crates/bevy_asset/src/ser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/ser.rs b/crates/bevy_asset/src/ser.rs index 4ef4e94d12fcf..42f2992c6448b 100644 --- a/crates/bevy_asset/src/ser.rs +++ b/crates/bevy_asset/src/ser.rs @@ -16,7 +16,7 @@ enum AssetRef { Default, /// Used for static handles like the `PBR_PIPELINE_HANDLE` or a embedded assets Local(Uuid, u64), - /// Loads form a file + /// Loads from a file External(String), } From bc96a93cdbdad7ac0d76e07379c9a00ee1b00498 Mon Sep 17 00:00:00 2001 From: Felipe Jorge Date: Sun, 6 Jun 2021 13:29:00 -0300 Subject: [PATCH 6/6] make no assumptions on how data will be (de)serialized --- crates/bevy_asset/src/ser.rs | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/crates/bevy_asset/src/ser.rs b/crates/bevy_asset/src/ser.rs index 42f2992c6448b..b134570013b7c 100644 --- a/crates/bevy_asset/src/ser.rs +++ b/crates/bevy_asset/src/ser.rs @@ -75,31 +75,14 @@ impl<'de, T: Asset> Deserialize<'de> for Handle { } impl AssetServer { - pub fn serialize_with_asset_refs( - &self, - serializer: S, - value: &T, - ) -> Result + /// Enables asset references to be serialized or deserialized + pub fn with_asset_refs_serialization(&self, f: F) -> T where - S: Serializer, - T: Serialize, - { - ASSET_SERVER.with(|key| { - key.replace(Some(self.clone())); - let result = value.serialize(serializer); - key.replace(None); - result - }) - } - - pub fn deserialize_with_asset_refs<'de, D, T>(&self, deserializer: D) -> Result - where - D: Deserializer<'de>, - T: Deserialize<'de>, + F: FnOnce() -> T, { ASSET_SERVER.with(|key| { key.replace(Some(self.clone())); - let result = T::deserialize(deserializer); + let result = (f)(); key.replace(None); result })