Provide getters for fields of ReflectFromPtr (#9748)

# Objective

The reasoning is similar to #8687.

I'm building a dynamic query. Currently, I store the ReflectFromPtr in
my dynamic `Fetch` type.

[See relevant
code](97ba68ae1e/src/fetches.rs (L14-L17))

However, `ReflectFromPtr` is:

- 16 bytes for TypeId
- 8 bytes for the non-mutable function pointer
- 8 bytes for the mutable function pointer

It's a lot, it adds 32 bytes to my base `Fetch` which is only
`ComponendId` (8 bytes) for a total of 40 bytes.

I only need one function per fetch, reducing the total dynamic fetch
size to 16 bytes.

Since I'm querying the components by the ComponendId associated with the
function pointer I'm using, I don't need the TypeId, it's a redundant
check.

In fact, I've difficulties coming up with situations where checking the
TypeId beforehand is relevant. So to me, if ReflectFromPtr makes sense
as a public API, exposing the function pointers also makes sense.

## Solution

- Make the fields public through methods.

---

## Changelog

- Add `from_ptr` and `from_ptr_mut` methods to `ReflectFromPtr` to
access the underlying function pointers
- `ReflectFromPtr::as_reflect_ptr` is now `ReflectFromPtr::as_reflect`
- `ReflectFromPtr::as_reflect_ptr_mut` is now
`ReflectFromPtr::as_reflect_mut`

## Migration guide

- `ReflectFromPtr::as_reflect_ptr` is now `ReflectFromPtr::as_reflect`
- `ReflectFromPtr::as_reflect_ptr_mut` is now
`ReflectFromPtr::as_reflect_mut`
This commit is contained in:
Nicola Papale 2023-09-18 15:41:51 +02:00 committed by GitHub
parent dc124ee498
commit 0bd4ea7ced
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 18 deletions

View File

@ -833,7 +833,7 @@ impl<'a> MutUntyped<'a> {
/// # let mut_untyped: MutUntyped = unimplemented!(); /// # let mut_untyped: MutUntyped = unimplemented!();
/// # let reflect_from_ptr: bevy_reflect::ReflectFromPtr = unimplemented!(); /// # let reflect_from_ptr: bevy_reflect::ReflectFromPtr = unimplemented!();
/// // SAFETY: from the context it is known that `ReflectFromPtr` was made for the type of the `MutUntyped` /// // SAFETY: from the context it is known that `ReflectFromPtr` was made for the type of the `MutUntyped`
/// mut_untyped.map_unchanged(|ptr| unsafe { reflect_from_ptr.as_reflect_ptr_mut(ptr) }); /// mut_untyped.map_unchanged(|ptr| unsafe { reflect_from_ptr.as_reflect_mut(ptr) });
/// ``` /// ```
pub fn map_unchanged<T: ?Sized>(self, f: impl FnOnce(PtrMut<'a>) -> &'a mut T) -> Mut<'a, T> { pub fn map_unchanged<T: ?Sized>(self, f: impl FnOnce(PtrMut<'a>) -> &'a mut T) -> Mut<'a, T> {
Mut { Mut {
@ -1168,7 +1168,7 @@ mod tests {
let mut new = value.map_unchanged(|ptr| { let mut new = value.map_unchanged(|ptr| {
// SAFETY: The underlying type of `ptr` matches `reflect_from_ptr`. // SAFETY: The underlying type of `ptr` matches `reflect_from_ptr`.
let value = unsafe { reflect_from_ptr.as_reflect_ptr_mut(ptr) }; let value = unsafe { reflect_from_ptr.as_reflect_mut(ptr) };
value value
}); });

View File

@ -521,37 +521,63 @@ impl<T: for<'a> Deserialize<'a> + Reflect> FromType<T> for ReflectDeserialize {
/// let reflect_data = type_registry.get(std::any::TypeId::of::<Reflected>()).unwrap(); /// let reflect_data = type_registry.get(std::any::TypeId::of::<Reflected>()).unwrap();
/// let reflect_from_ptr = reflect_data.data::<ReflectFromPtr>().unwrap(); /// let reflect_from_ptr = reflect_data.data::<ReflectFromPtr>().unwrap();
/// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for /// // SAFE: `value` is of type `Reflected`, which the `ReflectFromPtr` was created for
/// let value = unsafe { reflect_from_ptr.as_reflect_ptr(value) }; /// let value = unsafe { reflect_from_ptr.as_reflect(value) };
/// ///
/// assert_eq!(value.downcast_ref::<Reflected>().unwrap().0, "Hello world!"); /// assert_eq!(value.downcast_ref::<Reflected>().unwrap().0, "Hello world!");
/// ``` /// ```
#[derive(Clone)] #[derive(Clone)]
pub struct ReflectFromPtr { pub struct ReflectFromPtr {
type_id: TypeId, type_id: TypeId,
to_reflect: for<'a> unsafe fn(Ptr<'a>) -> &'a dyn Reflect, from_ptr: unsafe fn(Ptr) -> &dyn Reflect,
to_reflect_mut: for<'a> unsafe fn(PtrMut<'a>) -> &'a mut dyn Reflect, from_ptr_mut: unsafe fn(PtrMut) -> &mut dyn Reflect,
} }
impl ReflectFromPtr { impl ReflectFromPtr {
/// Returns the [`TypeId`] that the [`ReflectFromPtr`] was constructed for /// Returns the [`TypeId`] that the [`ReflectFromPtr`] was constructed for.
pub fn type_id(&self) -> TypeId { pub fn type_id(&self) -> TypeId {
self.type_id self.type_id
} }
/// Convert `Ptr` into `&dyn Reflect`.
///
/// # Safety /// # Safety
/// ///
/// `val` must be a pointer to value of the type that the [`ReflectFromPtr`] was constructed for. /// `val` must be a pointer to value of the type that the [`ReflectFromPtr`] was constructed for.
/// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one. /// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one.
pub unsafe fn as_reflect_ptr<'a>(&self, val: Ptr<'a>) -> &'a dyn Reflect { pub unsafe fn as_reflect<'a>(&self, val: Ptr<'a>) -> &'a dyn Reflect {
(self.to_reflect)(val) (self.from_ptr)(val)
} }
/// Convert `PtrMut` into `&mut dyn Reflect`.
///
/// # Safety /// # Safety
/// ///
/// `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for /// `val` must be a pointer to a value of the type that the [`ReflectFromPtr`] was constructed for
/// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one. /// This can be verified by checking that the type id returned by [`ReflectFromPtr::type_id`] is the expected one.
pub unsafe fn as_reflect_ptr_mut<'a>(&self, val: PtrMut<'a>) -> &'a mut dyn Reflect { pub unsafe fn as_reflect_mut<'a>(&self, val: PtrMut<'a>) -> &'a mut dyn Reflect {
(self.to_reflect_mut)(val) (self.from_ptr_mut)(val)
}
/// Get a function pointer to turn a `Ptr` into `&dyn Reflect` for
/// the type this [`ReflectFromPtr`] was constructed for.
///
/// # Safety
///
/// When calling the unsafe function returned by this method you must ensure that:
/// - The input `Ptr` points to the `Reflect` type this `ReflectFromPtr`
/// was constructed for.
pub fn from_ptr(&self) -> unsafe fn(Ptr) -> &dyn Reflect {
self.from_ptr
}
/// Get a function pointer to turn a `PtrMut` into `&mut dyn Reflect` for
/// the type this [`ReflectFromPtr`] was constructed for.
///
/// # Safety
///
/// When calling the unsafe function returned by this method you must ensure that:
/// - The input `PtrMut` points to the `Reflect` type this `ReflectFromPtr`
/// was constructed for.
pub fn from_ptr_mut(&self) -> unsafe fn(PtrMut) -> &mut dyn Reflect {
self.from_ptr_mut
} }
} }
@ -559,14 +585,14 @@ impl<T: Reflect> FromType<T> for ReflectFromPtr {
fn from_type() -> Self { fn from_type() -> Self {
ReflectFromPtr { ReflectFromPtr {
type_id: std::any::TypeId::of::<T>(), type_id: std::any::TypeId::of::<T>(),
to_reflect: |ptr| { from_ptr: |ptr| {
// SAFE: only called from `as_reflect`, where the `ptr` is guaranteed to be of type `T`, // SAFETY: `from_ptr_mut` is either called in `ReflectFromPtr::as_reflect`
// and `as_reflect_ptr`, where the caller promises to call it with type `T` // or returned by `ReflectFromPtr::from_ptr`, both lay out the invariants
// required by `deref`
unsafe { ptr.deref::<T>() as &dyn Reflect } unsafe { ptr.deref::<T>() as &dyn Reflect }
}, },
to_reflect_mut: |ptr| { from_ptr_mut: |ptr| {
// SAFE: only called from `as_reflect_mut`, where the `ptr` is guaranteed to be of type `T`, // SAFETY: same as above, but foor `as_reflect_mut`, `from_ptr_mut` and `deref_mut`.
// and `as_reflect_ptr_mut`, where the caller promises to call it with type `T`
unsafe { ptr.deref_mut::<T>() as &mut dyn Reflect } unsafe { ptr.deref_mut::<T>() as &mut dyn Reflect }
}, },
} }
@ -601,7 +627,7 @@ mod test {
{ {
let value = PtrMut::from(&mut value); let value = PtrMut::from(&mut value);
// SAFETY: reflect_from_ptr was constructed for the correct type // SAFETY: reflect_from_ptr was constructed for the correct type
let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_ptr_mut(value) }; let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_mut(value) };
match dyn_reflect.reflect_mut() { match dyn_reflect.reflect_mut() {
bevy_reflect::ReflectMut::Struct(strukt) => { bevy_reflect::ReflectMut::Struct(strukt) => {
strukt.field_mut("a").unwrap().apply(&2.0f32); strukt.field_mut("a").unwrap().apply(&2.0f32);
@ -612,7 +638,7 @@ mod test {
{ {
// SAFETY: reflect_from_ptr was constructed for the correct type // SAFETY: reflect_from_ptr was constructed for the correct type
let dyn_reflect = unsafe { reflect_from_ptr.as_reflect_ptr(Ptr::from(&value)) }; let dyn_reflect = unsafe { reflect_from_ptr.as_reflect(Ptr::from(&value)) };
match dyn_reflect.reflect_ref() { match dyn_reflect.reflect_ref() {
bevy_reflect::ReflectRef::Struct(strukt) => { bevy_reflect::ReflectRef::Struct(strukt) => {
let a = strukt.field("a").unwrap().downcast_ref::<f32>().unwrap(); let a = strukt.field("a").unwrap().downcast_ref::<f32>().unwrap();