From e0e8218a7229d31634f3e775bbb575b17641ac02 Mon Sep 17 00:00:00 2001 From: LuoZheng2002 <98627375+LuoZheng2002@users.noreply.github.com> Date: Sun, 28 Sep 2025 14:30:48 -0700 Subject: [PATCH] Replaced the BorrowAs trait with ObjectOrVariant trait. (#841) * replaced the BorrowAs trait with ObjectOrVariant trait; added explicit trait bounds in pool and graph's typed_ref method. * fixed tests * fixed restore_integrity test - it was failing spontaneously due do conflicting resource registries from two tests * fixed clippy warnings * install rustfmt + clippy on a CI machine before running the tests * fixed confusing documentation about batching * updated changelog * changelog fixes * ability to set runs via respective message * replaced the BorrowAs trait with ObjectOrVariant trait; added explicit trait bounds in pool and graph's typed_ref method. * removed unused imports * satisfied cargo doc --------- Co-authored-by: Dmitry --- fyrox-core/src/pool/mod.rs | 97 +++++++++++++++++++++---------- fyrox-graph/src/lib.rs | 41 +++++++------ fyrox-impl/src/scene/graph/mod.rs | 43 ++++---------- fyrox-impl/src/scene/node/mod.rs | 14 ++++- fyrox-ui/src/control.rs | 14 ++++- fyrox-ui/src/lib.rs | 29 ++------- 6 files changed, 132 insertions(+), 106 deletions(-) diff --git a/fyrox-core/src/pool/mod.rs b/fyrox-core/src/pool/mod.rs index c41659a9b..c98939584 100644 --- a/fyrox-core/src/pool/mod.rs +++ b/fyrox-core/src/pool/mod.rs @@ -74,23 +74,60 @@ where free_stack: Vec, } -pub trait BorrowAs> { - type Target; - fn borrow_as_ref(self, pool: &Pool) -> Option<&Self::Target>; - fn borrow_as_mut(self, pool: &mut Pool) -> Option<&mut Self::Target>; +/// This trait unifies pool objects and their variants. +/// +/// If T is the pool object type, [`ObjectOrVariant::convert_to_dest_type`] returns the pool object itself. +/// +/// If T is a variant of the pool object type, [`ObjectOrVariant::convert_to_dest_type`] returns the variant of the pool object. +/// +/// The [`Pool`] struct uses this trait to unify the logic of retrieving both pool objects and their variants. +pub trait ObjectOrVariant { + fn convert_to_dest_type(object: &T) -> Option<&Self>; + fn convert_to_dest_type_mut(object: &mut T) -> Option<&mut Self>; } -impl + 'static> BorrowAs - for Handle -{ - type Target = Object; +/// This trait is a helper trait for implementing [`ObjectOrVariant`] indirectly in child crates. +/// +/// To implement [`ObjectOrVariant`] for type `U` in a child crate, you need to implement [`ObjectOrVariantHelper`] for [`PhantomData`]. +/// +/// This is necessary because Rust does not support `impl ForeignTrait for T` +/// +/// But Rust does support `impl ForeignTrait for ForeignType` +/// +/// Details can be found in [here](https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html#concrete-orphan-rules). +/// +/// Therefore, we cannot implement [`ObjectOrVariant`] directly using `impl ObjectOrVariant for U` +/// +/// but we can do `impl ObjectOrVariantHelper for PhantomData` +/// +/// There is an indirection to get rid of [`PhantomData`] because we want to have a clean API for pool methods +/// where you can pass in `U` directly instead of [`PhantomData`]. +pub trait ObjectOrVariantHelper { + fn convert_to_dest_type_helper(object: &T) -> Option<&U>; + fn convert_to_dest_type_helper_mut(object: &mut T) -> Option<&mut U>; +} - fn borrow_as_ref(self, pool: &Pool) -> Option<&Object> { - pool.try_borrow(self) +// This is the default implementation for pool object types. +// For pool object variant types, they need to be implemented case by case, since Rust does not support multiple blanket implementations. +impl ObjectOrVariantHelper for PhantomData { + fn convert_to_dest_type_helper(object: &T) -> Option<&T> { + Some(object) } + fn convert_to_dest_type_helper_mut(object: &mut T) -> Option<&mut T> { + Some(object) + } +} - fn borrow_as_mut(self, pool: &mut Pool) -> Option<&mut Object> { - pool.try_borrow_mut(self) +// This blanket implementation wraps types that implement ObjectOrVariantHelper with the ObjectOrVariant trait. +impl ObjectOrVariant for U +where + PhantomData: ObjectOrVariantHelper, +{ + fn convert_to_dest_type(object: &T) -> Option<&Self> { + PhantomData::::convert_to_dest_type_helper(object) + } + fn convert_to_dest_type_mut(object: &mut T) -> Option<&mut Self> { + PhantomData::::convert_to_dest_type_helper_mut(object) } } @@ -414,16 +451,15 @@ where } #[inline] - pub fn typed_ref(&self, handle: impl BorrowAs) -> Option<&Ref> { - handle.borrow_as_ref(self) + pub fn typed_ref>(&self, handle: Handle) -> Option<&U> { + let pool_object = self.try_borrow(handle.transmute())?; + U::convert_to_dest_type(pool_object) } #[inline] - pub fn typed_mut( - &mut self, - handle: impl BorrowAs, - ) -> Option<&mut Ref> { - handle.borrow_as_mut(self) + pub fn typed_mut>(&mut self, handle: Handle) -> Option<&mut U> { + let pool_object = self.try_borrow_mut(handle.transmute())?; + U::convert_to_dest_type_mut(pool_object) } #[inline] @@ -1367,28 +1403,27 @@ where } } -impl Index for Pool +impl Index> for Pool where - Object: 'static, - Container: PayloadContainer + 'static, - Borrow: BorrowAs, + T: 'static, + U: ObjectOrVariant, + Container: PayloadContainer + 'static, { - type Output = Ref; - + type Output = U; #[inline] - fn index(&self, index: Borrow) -> &Self::Output { + fn index(&self, index: Handle) -> &Self::Output { self.typed_ref(index).expect("The handle must be valid!") } } -impl IndexMut for Pool +impl IndexMut> for Pool where - Object: 'static, - Container: PayloadContainer + 'static, - Borrow: BorrowAs, + T: 'static, + U: ObjectOrVariant, + Container: PayloadContainer + 'static, { #[inline] - fn index_mut(&mut self, index: Borrow) -> &mut Self::Output { + fn index_mut(&mut self, index: Handle) -> &mut Self::Output { self.typed_mut(index).expect("The handle must be valid!") } } diff --git a/fyrox-graph/src/lib.rs b/fyrox-graph/src/lib.rs index 4fcedfde2..b11244d5a 100644 --- a/fyrox-graph/src/lib.rs +++ b/fyrox-graph/src/lib.rs @@ -25,7 +25,7 @@ pub mod constructor; use fxhash::FxHashMap; -use fyrox_core::pool::{BorrowAs, ErasedHandle, PayloadContainer}; +use fyrox_core::pool::{ErasedHandle, ObjectOrVariant, PayloadContainer}; use fyrox_core::reflect::ReflectHandle; use fyrox_core::{ log::{Log, MessageKind}, @@ -757,7 +757,7 @@ pub trait BaseSceneGraph: AbstractSceneGraph { /// |_A_ /// | |_B /// |_Root - /// ``` + /// ``` /// /// This method returns an instance of [`LinkScheme`], that could be used to revert the hierarchy /// back to its original. See [`Self::apply_link_scheme`] for more info. @@ -827,6 +827,7 @@ pub trait BaseSceneGraph: AbstractSceneGraph { } pub trait SceneGraph: BaseSceneGraph { + type ObjectType: Sized; /// Creates new iterator that iterates over internal collection giving (handle; node) pairs. fn pair_iter(&self) -> impl Iterator, &Self::Node)>; @@ -838,15 +839,12 @@ pub trait SceneGraph: BaseSceneGraph { /// of nodes. It does *not* perform any tree traversal! fn linear_iter_mut(&mut self) -> impl Iterator; - fn typed_ref( - &self, - handle: impl BorrowAs, - ) -> Option<&Ref>; + fn typed_ref>(&self, handle: Handle) -> Option<&U>; - fn typed_mut( + fn typed_mut>( &mut self, - handle: impl BorrowAs, - ) -> Option<&mut Ref>; + handle: Handle, + ) -> Option<&mut U>; /// Tries to borrow a node and fetch its component of specified type. #[inline] @@ -1450,7 +1448,7 @@ mod test { AbstractSceneGraph, AbstractSceneNode, BaseSceneGraph, NodeHandleMap, NodeMapping, PrefabData, SceneGraph, SceneGraphNode, }; - use fyrox_core::pool::BorrowAs; + use fyrox_core::pool::{ObjectOrVariant, ObjectOrVariantHelper}; use fyrox_core::{ define_as_any_trait, pool::{ErasedHandle, Handle, PayloadContainer, Pool}, @@ -1460,6 +1458,7 @@ mod test { NameProvider, }; use fyrox_resource::{untyped::UntypedResource, Resource, ResourceData}; + use std::marker::PhantomData; use std::{ any::{Any, TypeId}, error::Error, @@ -1500,6 +1499,17 @@ mod test { pub trait NodeTrait: BaseNodeTrait + Reflect + Visit + ComponentProvider + NodeAsAny {} + // Essentially implements ObjectOrVariant for NodeTrait types. + // See ObjectOrVariantHelper for the cause of the indirection. + impl ObjectOrVariantHelper for PhantomData { + fn convert_to_dest_type_helper(node: &Node) -> Option<&T> { + NodeAsAny::as_any(node.0.deref()).downcast_ref() + } + fn convert_to_dest_type_helper_mut(node: &mut Node) -> Option<&mut T> { + NodeAsAny::as_any_mut(node.0.deref_mut()).downcast_mut() + } + } + #[derive(ComponentProvider, Debug)] pub struct Node(Box); @@ -1881,6 +1891,7 @@ mod test { } impl SceneGraph for Graph { + type ObjectType = Node; fn pair_iter(&self) -> impl Iterator, &Self::Node)> { self.nodes.pair_iter() } @@ -1893,17 +1904,11 @@ mod test { self.nodes.iter_mut() } - fn typed_ref( - &self, - handle: impl BorrowAs, - ) -> Option<&Ref> { + fn typed_ref>(&self, handle: Handle) -> Option<&U> { self.nodes.typed_ref(handle) } - fn typed_mut( - &mut self, - handle: impl BorrowAs, - ) -> Option<&mut Ref> { + fn typed_mut>(&mut self, handle: Handle) -> Option<&mut U> { self.nodes.typed_mut(handle) } } diff --git a/fyrox-impl/src/scene/graph/mod.rs b/fyrox-impl/src/scene/graph/mod.rs index 2ab6b827a..7ccac326c 100644 --- a/fyrox-impl/src/scene/graph/mod.rs +++ b/fyrox-impl/src/scene/graph/mod.rs @@ -66,7 +66,7 @@ use crate::{ }, mesh::Mesh, navmesh, - node::{container::NodeContainer, Node, NodeTrait, SyncContext, UpdateContext}, + node::{container::NodeContainer, Node, SyncContext, UpdateContext}, pivot::Pivot, sound::context::SoundContext, transform::TransformBuilder, @@ -76,9 +76,9 @@ use crate::{ }; use bitflags::bitflags; use fxhash::{FxHashMap, FxHashSet}; -use fyrox_core::pool::BorrowAs; +use fyrox_core::pool::ObjectOrVariant; use fyrox_graph::SceneGraphNode; -use std::ops::{Deref, DerefMut}; +use std::ops::Deref; use std::{ any::{Any, TypeId}, fmt::Debug, @@ -126,20 +126,6 @@ impl GraphPerformanceStatistics { /// A helper type alias for node pool. pub type NodePool = Pool; -impl BorrowAs for Handle { - type Target = T; - - fn borrow_as_ref(self, pool: &NodePool) -> Option<&T> { - pool.try_borrow(self.transmute()) - .and_then(|n| NodeAsAny::as_any(n.0.deref()).downcast_ref::()) - } - - fn borrow_as_mut(self, pool: &mut NodePool) -> Option<&mut T> { - pool.try_borrow_mut(self.transmute()) - .and_then(|n| NodeAsAny::as_any_mut(n.0.deref_mut()).downcast_mut::()) - } -} - /// See module docs. #[derive(Debug, Reflect)] pub struct Graph { @@ -1640,20 +1626,20 @@ impl Graph { } } -impl> Index for Graph { +impl> Index> for Graph { type Output = T; #[inline] - fn index(&self, typed_handle: B) -> &Self::Output { - self.typed_ref(typed_handle) + fn index(&self, index: Handle) -> &Self::Output { + self.typed_ref(index) .expect("The node handle is invalid or the object it points to has different type.") } } -impl> IndexMut for Graph { +impl> IndexMut> for Graph { #[inline] - fn index_mut(&mut self, typed_handle: B) -> &mut Self::Output { - self.typed_mut(typed_handle) + fn index_mut(&mut self, index: Handle) -> &mut Self::Output { + self.typed_mut(index) .expect("The node handle is invalid or the object it points to has different type.") } } @@ -1842,6 +1828,7 @@ impl BaseSceneGraph for Graph { } impl SceneGraph for Graph { + type ObjectType = Node; #[inline] fn pair_iter(&self) -> impl Iterator, &Self::Node)> { self.pool.pair_iter() @@ -1857,17 +1844,11 @@ impl SceneGraph for Graph { self.pool.iter_mut() } - fn typed_ref( - &self, - handle: impl BorrowAs, - ) -> Option<&Ref> { + fn typed_ref>(&self, handle: Handle) -> Option<&U> { self.pool.typed_ref(handle) } - fn typed_mut( - &mut self, - handle: impl BorrowAs, - ) -> Option<&mut Ref> { + fn typed_mut>(&mut self, handle: Handle) -> Option<&mut U> { self.pool.typed_mut(handle) } } diff --git a/fyrox-impl/src/scene/node/mod.rs b/fyrox-impl/src/scene/node/mod.rs index d4a336df7..1bd20ae13 100644 --- a/fyrox-impl/src/scene/node/mod.rs +++ b/fyrox-impl/src/scene/node/mod.rs @@ -61,10 +61,11 @@ use crate::{ Scene, }, }; -use fyrox_core::define_as_any_trait; +use fyrox_core::{define_as_any_trait, pool::ObjectOrVariantHelper}; use std::{ any::{Any, TypeId}, fmt::Debug, + marker::PhantomData, ops::{Deref, DerefMut}, }; @@ -235,6 +236,17 @@ pub trait NodeTrait: BaseNodeTrait + Reflect + Visit + ComponentProvider { } } +// Essentially implements ObjectOrVariant for NodeTrait types. +// See ObjectOrVariantHelper for the cause of the indirection. +impl ObjectOrVariantHelper for PhantomData { + fn convert_to_dest_type_helper(node: &Node) -> Option<&T> { + NodeAsAny::as_any(node.0.deref()).downcast_ref() + } + fn convert_to_dest_type_helper_mut(node: &mut Node) -> Option<&mut T> { + NodeAsAny::as_any_mut(node.0.deref_mut()).downcast_mut() + } +} + /// Node is the basic building block for 3D scenes. It has multiple variants, but all of them share some /// common functionality: /// diff --git a/fyrox-ui/src/control.rs b/fyrox-ui/src/control.rs index ce3cf9500..288f539ea 100644 --- a/fyrox-ui/src/control.rs +++ b/fyrox-ui/src/control.rs @@ -26,11 +26,12 @@ use crate::{ widget::Widget, UiNode, UserInterface, }; -use fyrox_core::define_as_any_trait; +use fyrox_core::{define_as_any_trait, pool::ObjectOrVariant}; use fyrox_core::algebra::Matrix3; use std::{ any::Any, + marker::PhantomData, ops::{Deref, DerefMut}, sync::mpsc::Sender, }; @@ -356,3 +357,14 @@ pub trait Control: *self.allow_drop } } + +// Essentially implements ObjectOrVariant for NodeTrait types. +// See ObjectOrVariantHelper for the cause of the indirection. +impl ObjectOrVariant for PhantomData { + fn convert_to_dest_type(node: &UiNode) -> Option<&Self> { + ControlAsAny::as_any(node.0.deref()).downcast_ref() + } + fn convert_to_dest_type_mut(node: &mut UiNode) -> Option<&mut Self> { + ControlAsAny::as_any_mut(node.0.deref_mut()).downcast_mut() + } +} diff --git a/fyrox-ui/src/lib.rs b/fyrox-ui/src/lib.rs index 8fbaebf74..26f787deb 100644 --- a/fyrox-ui/src/lib.rs +++ b/fyrox-ui/src/lib.rs @@ -340,8 +340,8 @@ use strum_macros::{AsRefStr, EnumString, VariantNames}; pub use alignment::*; pub use build::*; pub use control::*; -use fyrox_core::futures::future::join_all; use fyrox_core::log::Log; +use fyrox_core::{futures::future::join_all, pool::ObjectOrVariant}; use fyrox_graph::{ AbstractSceneGraph, AbstractSceneNode, BaseSceneGraph, NodeHandleMap, NodeMapping, PrefabData, SceneGraph, SceneGraphNode, @@ -355,7 +355,7 @@ use crate::style::resource::{StyleResource, StyleResourceExt}; use crate::style::{Style, DEFAULT_STYLE}; use crate::widget::WidgetMaterial; pub use fyrox_animation as generic_animation; -use fyrox_core::pool::{BorrowAs, ErasedHandle}; +use fyrox_core::pool::ErasedHandle; use fyrox_resource::untyped::ResourceKind; pub use fyrox_texture as texture; @@ -674,20 +674,6 @@ pub struct UiUpdateSwitches { pub type WidgetPool = Pool; -impl BorrowAs for Handle { - type Target = T; - - fn borrow_as_ref(self, pool: &WidgetPool) -> Option<&T> { - pool.try_borrow(self.transmute()) - .and_then(|n| ControlAsAny::as_any(n.0.deref()).downcast_ref::()) - } - - fn borrow_as_mut(self, pool: &mut WidgetPool) -> Option<&mut T> { - pool.try_borrow_mut(self.transmute()) - .and_then(|n| ControlAsAny::as_any_mut(n.0.deref_mut()).downcast_mut::()) - } -} - #[derive(Reflect, Debug)] pub struct UserInterface { screen_size: Vector2, @@ -3383,6 +3369,7 @@ impl BaseSceneGraph for UserInterface { } impl SceneGraph for UserInterface { + type ObjectType = UiNode; #[inline] fn pair_iter(&self) -> impl Iterator, &Self::Node)> { self.nodes.pair_iter() @@ -3398,17 +3385,11 @@ impl SceneGraph for UserInterface { self.nodes.iter_mut() } - fn typed_ref( - &self, - handle: impl BorrowAs, - ) -> Option<&Ref> { + fn typed_ref>(&self, handle: Handle) -> Option<&U> { self.nodes.typed_ref(handle) } - fn typed_mut( - &mut self, - handle: impl BorrowAs, - ) -> Option<&mut Ref> { + fn typed_mut>(&mut self, handle: Handle) -> Option<&mut U> { self.nodes.typed_mut(handle) } }