diff --git a/godot-codegen/src/generator/central_files.rs b/godot-codegen/src/generator/central_files.rs index 5e2af66d9..abf8c04dd 100644 --- a/godot-codegen/src/generator/central_files.rs +++ b/godot-codegen/src/generator/central_files.rs @@ -19,6 +19,7 @@ pub fn make_sys_central_code(api: &ExtensionApi) -> TokenStream { let variant_type_enum = make_variant_type_enum(api, true); let [opaque_32bit, opaque_64bit] = make_opaque_types(api); let godot_type_name_method = make_godot_type_name_method(api); + let has_destructor_method = make_has_destructor_method(api); quote! { #[cfg(target_pointer_width = "32")] @@ -49,6 +50,7 @@ pub fn make_sys_central_code(api: &ExtensionApi) -> TokenStream { } #godot_type_name_method + #has_destructor_method } } } @@ -228,6 +230,68 @@ fn make_variant_type_enum(api: &ExtensionApi, is_definition: bool) -> TokenStrea enums::make_enum_definition_with(variant_type_enum, define_enum, define_traits) } +/// Generates the `VariantType::needs_ffi_destruction()` method from the builtins list. +/// +/// Returns `true` if a variant of this type requires `variant_destroy` to be called when dropped. +/// Two reasons a type needs destruction: +/// 1. It has a non-trivial destructor in `extension_api.json` (refcounted: `String`, `Array`, `Dictionary`, `Object`, ...). +/// 2. It is stored heap-allocated inside `Variant` because it does not fit in the inline data segment +/// (`Transform2D`, `Transform3D`, `Basis`, `Aabb`, `Projection`). Godot's API reports `has_destructor=false` for these +/// (the C++ type is trivially destructible), but the `Variant` wrapper still owns the heap allocation. +/// +/// The inline data segment is 16 bytes in single precision and 32 bytes in double precision; types larger than the segment in *either* +/// configuration are always heap-allocated inside `Variant`. +fn make_has_destructor_method(api: &ExtensionApi) -> TokenStream { + use crate::models::domain::BuildConfiguration; + + // The problem: Godot stores every Variant value in a small fixed-size buffer. A type that fits is held by value; a type that does not + // is heap-allocated by Variant, and the Variant then owns that allocation -- even when the type itself is trivially destructible and + // thus reports `has_destructor=false`. So besides the `has_destructor` types, we must also flag any type too large for the buffer. + // + // Detecting "too large" needs the type's byte size, which differs per precision. Two facts here: + // * The buffer is 16 bytes in single precision and 32 bytes in double. + // * A type's size at most doubles when switching to double (real grows 4 -> 8 bytes; int/ptr stays), so size_double <= 2 * size_single. + // + // This implies: `size_single > 16` iff `size_double > 32`. A type that fits the buffer in single precision also fits in double, + // and one too large in single is also too large in double. So checking against the single-precision sizes gives the right answer for + // both builds. We therefore read the Float32 config. The flagged types (Transform2D, Aabb, Basis, Transform3D, Projection) are + // pure-float and too large for the buffer either way. + const INLINE_DATA_SIZE_SINGLE: usize = 16; + + let sizes: std::collections::HashMap<&str, usize> = api + .builtin_sizes + .iter() + .filter(|s| s.config == BuildConfiguration::Float32) + .map(|s| (s.builtin_original_name.as_str(), s.size)) + .collect(); + + let mut destructor_ordinals = vec![]; + + for builtin in api.builtins.iter() { + let size = sizes + .get(builtin.godot_original_name()) + .copied() + .unwrap_or(0); + let is_heap_in_variant = size > INLINE_DATA_SIZE_SINGLE; + + if builtin.has_destructor || is_heap_in_variant { + destructor_ordinals.push(builtin.variant_type_ord); + } + } + + quote! { + /// Returns `true` if variants of this type require destruction (i.e. are not plain-old-data). + /// + /// Combines two sources of truth: `has_destructor` from Godot's `extension_api.json` (for refcounted types) and + /// builtin size (for heap-allocated types like `Transform2D`/`Basis`/`Aabb`/`Transform3D`/`Projection` that + /// the C++ side wraps in a heap allocation when stored in `Variant`). + #[doc(hidden)] + pub fn needs_ffi_destruction(&self) -> bool { + matches!(self.ord, #( #destructor_ordinals )|*) + } + } +} + /// Generates the `VariantType::godot_type_name()` method from the builtins list. fn make_godot_type_name_method(api: &ExtensionApi) -> TokenStream { // NIL is not in api.builtins; handle it manually. diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index 45d60b10b..6bb4eed68 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -186,6 +186,7 @@ pub struct BuiltinVariant { pub builtin_class: Option, pub variant_type_ord: i32, + pub has_destructor: bool, } impl BuiltinVariant { diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index 5b1d843bc..a20de60a4 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -294,15 +294,18 @@ impl BuiltinVariant { ) -> Self { let builtin_class; let godot_original_name; + let has_destructor; // Nil, int, float etc. are not represented by a BuiltinVariant. // Object has no BuiltinClass, but still gets its BuiltinVariant instance. if let Some(json_builtin) = json_builtin_class { + has_destructor = json_builtin.has_destructor; godot_original_name = json_builtin.name.clone(); builtin_class = BuiltinClass::from_json(json_builtin, ctx); } else { assert_eq!(json_variant_enumerator_name, "OBJECT"); + has_destructor = true; // Object requires ref-count management. builtin_class = None; godot_original_name = "Object".to_string(); }; @@ -313,6 +316,7 @@ impl BuiltinVariant { godot_snake_name: conv::to_snake_case(json_variant_enumerator_name), builtin_class, variant_type_ord: json_variant_enumerator_ord, + has_destructor, } } } diff --git a/godot-core/Cargo.toml b/godot-core/Cargo.toml index 4ea99fd81..b3fb7f207 100644 --- a/godot-core/Cargo.toml +++ b/godot-core/Cargo.toml @@ -27,6 +27,8 @@ experimental-threads = ["godot-ffi/experimental-threads", "godot-codegen/experim experimental-wasm-nothreads = ["godot-ffi/experimental-wasm-nothreads"] debug-log = ["godot-ffi/debug-log"] trace = [] +# Disables Variant's RustMarshal optimization, falling back to FFI. Intended for A/B tests + benchmarks. +variant-ffi-marshal = [] api-custom = ["godot-ffi/api-custom", "godot-codegen/api-custom"] api-custom-json = ["godot-codegen/api-custom-json"] diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index 29230581b..96d2a58ec 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -466,13 +466,7 @@ impl Array { /// /// If you know that the new size is smaller, then consider using [`shrink`][AnyArray::shrink] instead. pub fn resize(&mut self, new_size: usize, value: impl AsArg) { - self.balanced_ensure_mutable(); - - let original_size = self.len(); - - // SAFETY: While we do insert `Variant::nil()` if the new size is larger, we then fill it with `value` ensuring that all values in the - // array are of type `T` still. - unsafe { self.as_inner_mut() }.resize(to_i64(new_size)); + let original_size = self.resize_inner(new_size); meta::arg_into_ref!(value: T); @@ -491,6 +485,30 @@ impl Array { } } + /// Resizes the array, limited to certain element types and values. + /// + /// Limited to default-constructible `Copy` types, to allow efficient batch initialization. Use [`resize()`][Self::resize] if you need + /// more flexibility. + pub fn resize_default(&mut self, new_size: usize) + where + // Do not remove these bounds. Allowing e.g. Gd would create null elements. + // Limiting to Copy only would be possible, but inconsistent for types like Plane that don't support default-initialization in Rust. + T: Default + Copy, + { + self.resize_inner(new_size); + } + + fn resize_inner(&mut self, new_size: usize) -> usize { + self.balanced_ensure_mutable(); + + let original_size = self.len(); + + // SAFETY: Godot's `resize()` fills new slots with type-appropriate defaults for typed arrays (e.g. 0 for int, nil for Variant). + // Callers that need non-default values (like `resize()`) must overwrite the new slots afterward. + unsafe { self.as_inner_mut() }.resize(to_i64(new_size)); + original_size + } + /// Appends another array at the end of this array. Equivalent of `append_array` in GDScript. pub fn extend_array(&mut self, other: &Array) { self.balanced_ensure_mutable(); @@ -828,8 +846,11 @@ impl Array { /// Returns a mutable pointer to the element at the given index. /// /// # Panics - /// /// If `index` is out of bounds. + /// + /// # Note on mut slices + /// Do not form `&mut [Variant]` spanning multiple slots from this pointer. `Array` is ref-counted, so other handles may alias the same + /// backing storage; a multi-element `&mut` slice would violate Rust's aliasing rules. Write individual elements via `ptr::write` instead. fn ptr_mut(&mut self, index: usize) -> sys::GDExtensionVariantPtr { let ptr = self.ptr_mut_or_null(index); assert!( @@ -1374,9 +1395,8 @@ impl From<&[T]> for Array { // the nulls with values of type `T`. unsafe { array.as_inner_mut() }.resize(to_i64(len)); - // SAFETY: `array` has `len` elements since we just resized it, and they are all valid `Variant`s. Additionally, since - // the array was created in this function, and we do not access the array while this slice exists, the slice has unique - // access to the elements. + // SAFETY: `array` has `len` elements since we just resized it, all valid Variants. The array was just created here and is not yet + // shared (refcount == 1), so no other handle aliases the backing buffer; forming &mut [Variant] is sound in this unique-ownership context. let elements = unsafe { Variant::borrow_slice_mut(array.ptr_mut(0), len) }; for (element, array_slot) in slice.iter().zip(elements.iter_mut()) { *array_slot = element.to_variant(); @@ -1398,13 +1418,34 @@ impl FromIterator for Array { /// Extends a `Array` with the contents of an iterator. impl Extend for Array { fn extend>(&mut self, iter: I) { - // Unfortunately the GDExtension API does not offer the equivalent of `Vec::reserve`. - // Otherwise, we could use it to pre-allocate based on `iter.size_hint()`. - // - // A faster implementation using `resize()` and direct pointer writes might still be possible. - // Note that this could technically also use iter(), since no moves need to happen (however Extend requires IntoIterator). - for item in iter.into_iter() { - // self.push(AsArg::into_arg(&item)); + let mut iter = iter.into_iter(); + let (lower, _upper) = iter.size_hint(); + + // Fast path: pre-allocate space for the lower bound and write variants directly, avoiding per-element resize calls. + if lower > 0 { + let current_len = self.len(); + let new_len = current_len + lower; + + self.resize_inner(new_len); + + for i in current_len..new_len { + let item = iter + .next() + .expect("iterator returned fewer than size_hint().0 elements"); + + let elem_ptr: *mut Variant = self.ptr_mut(i).cast::(); + + // SAFETY: + // * i is in bounds after resize_inner(). + // * `resize_inner` fills new slots with default value. Assignment through = (as opposed to ptr::write) drops that default + // before writing -> no leaks. + // * Single-element place assignment avoids forming a &mut [Variant], which is unsound -- Array is refcounted, other refs may alias. + unsafe { *elem_ptr = item.to_variant() }; + } + } + + // Push any remaining elements (for inexact-size iterators, or when lower == 0). + for item in iter { self.push(meta::owned_into_arg(item)); } } diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index 967731411..24603fb35 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -8,6 +8,7 @@ use godot_ffi as sys; use sys::GodotFfi; +use super::rust_variant::{RustVariant, USE_RUST_MARSHAL}; use crate::builtin::*; use crate::meta::error::{ConvertError, FromVariantError}; use crate::meta::sealed::Sealed; @@ -25,38 +26,105 @@ use crate::task::{DynamicSend, IntoDynamicSend, ThreadConfined, impl_dynamic_sen // However, those same types would cause memory leaks in Godot 4.1 if pre-initialized. A compat layer `new_with_uninit_or_init()` addressed this. // As these Godot versions are no longer supported, the current implementation uses `new_with_uninit()` uniformly for all versions. macro_rules! impl_ffi_variant { - // With explicit metadata (e.g. for i64, f64). - (ref $T:ty, $from_fn:ident, $to_fn:ident; $metadata:expr) => { - impl_ffi_variant!(@impls by_ref, $metadata; $T, $from_fn, $to_fn); + // Entry points with RustVariant optimization. + (ref $T:ty, $from_fn:ident, $to_fn:ident; $metadata:expr, @rust_variant) => { + impl_ffi_variant!(@rust_variant_impls by_ref, $metadata; $T, $from_fn, $to_fn); }; - ($T:ty, $from_fn:ident, $to_fn:ident; $metadata:expr) => { - impl_ffi_variant!(@impls by_val, $metadata; $T, $from_fn, $to_fn); + ($T:ty, $from_fn:ident, $to_fn:ident; $metadata:expr, @rust_variant) => { + impl_ffi_variant!(@rust_variant_impls by_val, $metadata; $T, $from_fn, $to_fn); + }; + (ref $T:ty, $from_fn:ident, $to_fn:ident, @rust_variant) => { + impl_ffi_variant!(@rust_variant_impls by_ref, ParamMetadata::NONE; $T, $from_fn, $to_fn); + }; + ($T:ty, $from_fn:ident, $to_fn:ident, @rust_variant) => { + impl_ffi_variant!(@rust_variant_impls by_val, ParamMetadata::NONE; $T, $from_fn, $to_fn); }; - // Without metadata (defaults to ParamMetadata::NONE). - (ref $T:ty, $from_fn:ident, $to_fn:ident) => { - impl_ffi_variant!(@impls by_ref, ParamMetadata::NONE; $T, $from_fn, $to_fn); + // Entry points without RustVariant (standard FFI path). Use @ffi to opt in explicitly. + (ref $T:ty, $from_fn:ident, $to_fn:ident; $metadata:expr, @ffi) => { + impl_ffi_variant!(@ffi_only_impls by_ref, $metadata; $T, $from_fn, $to_fn); + }; + ($T:ty, $from_fn:ident, $to_fn:ident; $metadata:expr, @ffi) => { + impl_ffi_variant!(@ffi_only_impls by_val, $metadata; $T, $from_fn, $to_fn); + }; + (ref $T:ty, $from_fn:ident, $to_fn:ident, @ffi) => { + impl_ffi_variant!(@ffi_only_impls by_ref, ParamMetadata::NONE; $T, $from_fn, $to_fn); + }; + ($T:ty, $from_fn:ident, $to_fn:ident, @ffi) => { + impl_ffi_variant!(@ffi_only_impls by_val, ParamMetadata::NONE; $T, $from_fn, $to_fn); }; - ($T:ty, $from_fn:ident, $to_fn:ident) => { - impl_ffi_variant!(@impls by_val, ParamMetadata::NONE; $T, $from_fn, $to_fn); + + // Implementation with RustVariant optimization. + (@rust_variant_impls $by_ref_or_val:ident, $metadata:expr; $T:ty, $from_fn:ident, $to_fn:ident) => { + // Compile-time size check at macro expansion site -- validated regardless of feature flags. + const _: () = { + use crate::builtin::variant::rust_variant::VARIANT_DATA_SIZE; + assert!( + std::mem::size_of::<$T>() <= VARIANT_DATA_SIZE, + "Type is too large for RustVariant" + ); + }; + + impl GodotFfiVariant for $T { + fn ffi_to_variant(&self) -> Variant { + if USE_RUST_MARSHAL { + RustVariant::from_pod(*self) + } else { + unsafe { + Variant::new_with_var_uninit(|variant_ptr| { + let converter = sys::builtin_fn!($from_fn); + converter(variant_ptr, sys::SysPtr::force_mut(self.sys())); + }) + } + } + } + + fn ffi_from_variant(variant: &Variant) -> Result { + if USE_RUST_MARSHAL { + return RustVariant::view(variant).get_value::().ok_or_else(|| { + FromVariantError::BadType { + expected: Self::VARIANT_TYPE.variant_as_nil(), + actual: variant.get_type(), + } + .into_error(variant.clone()) + }); + } + + if variant.get_type() != Self::VARIANT_TYPE.variant_as_nil() { + return Err(FromVariantError::BadType { + expected: Self::VARIANT_TYPE.variant_as_nil(), + actual: variant.get_type(), + } + .into_error(variant.clone())); + } + + let result = unsafe { + Self::new_with_uninit(|self_ptr| { + let converter = sys::builtin_fn!($to_fn); + converter(self_ptr, sys::SysPtr::force_mut(variant.var_sys())); + }) + }; + + Ok(result) + } + } + + impl_ffi_variant!(@shared_impls $by_ref_or_val, $metadata; $T); }; - // Implementations - (@impls $by_ref_or_val:ident, $metadata:expr; $T:ty, $from_fn:ident, $to_fn:ident) => { + // Implementation without RustVariant (standard FFI only). + (@ffi_only_impls $by_ref_or_val:ident, $metadata:expr; $T:ty, $from_fn:ident, $to_fn:ident) => { impl GodotFfiVariant for $T { fn ffi_to_variant(&self) -> Variant { - let variant = unsafe { + unsafe { Variant::new_with_var_uninit(|variant_ptr| { let converter = sys::builtin_fn!($from_fn); converter(variant_ptr, sys::SysPtr::force_mut(self.sys())); }) - }; - - variant + } } fn ffi_from_variant(variant: &Variant) -> Result { - // Type check -- at the moment, a strict match is required. if variant.get_type() != Self::VARIANT_TYPE.variant_as_nil() { return Err(FromVariantError::BadType { expected: Self::VARIANT_TYPE.variant_as_nil(), @@ -76,6 +144,11 @@ macro_rules! impl_ffi_variant { } } + impl_ffi_variant!(@shared_impls $by_ref_or_val, $metadata; $T); + }; + + // Shared implementations (GodotType, Element). + (@shared_impls $by_ref_or_val:ident, $metadata:expr; $T:ty) => { impl GodotType for $T { type Ffi = Self; impl_ffi_variant!(@assoc_to_ffi $by_ref_or_val); @@ -124,31 +197,39 @@ mod impls { // IMPORTANT: the presence/absence of `ref` here should be aligned with the ArgPassing variant // used in codegen get_builtin_arg_passing(). - impl_ffi_variant!(bool, bool_to_variant, bool_from_variant); - impl_ffi_variant!(i64, int_to_variant, int_from_variant; ParamMetadata::INT_IS_INT64); - impl_ffi_variant!(f64, float_to_variant, float_from_variant; ParamMetadata::REAL_IS_DOUBLE); - impl_ffi_variant!(Vector2, vector2_to_variant, vector2_from_variant); - impl_ffi_variant!(Vector3, vector3_to_variant, vector3_from_variant); - impl_ffi_variant!(Vector4, vector4_to_variant, vector4_from_variant); - impl_ffi_variant!(Vector2i, vector2i_to_variant, vector2i_from_variant); - impl_ffi_variant!(Vector3i, vector3i_to_variant, vector3i_from_variant); - impl_ffi_variant!(Vector4i, vector4i_to_variant, vector4i_from_variant); - impl_ffi_variant!(Quaternion, quaternion_to_variant, quaternion_from_variant); - impl_ffi_variant!(Transform2D, transform_2d_to_variant, transform_2d_from_variant); - impl_ffi_variant!(Transform3D, transform_3d_to_variant, transform_3d_from_variant); - impl_ffi_variant!(Basis, basis_to_variant, basis_from_variant); - impl_ffi_variant!(Projection, projection_to_variant, projection_from_variant); - impl_ffi_variant!(Plane, plane_to_variant, plane_from_variant); - impl_ffi_variant!(Rect2, rect2_to_variant, rect2_from_variant); - impl_ffi_variant!(Rect2i, rect2i_to_variant, rect2i_from_variant); - impl_ffi_variant!(Aabb, aabb_to_variant, aabb_from_variant); - impl_ffi_variant!(Color, color_to_variant, color_from_variant); - impl_ffi_variant!(Rid, rid_to_variant, rid_from_variant); - impl_ffi_variant!(ref GString, string_to_variant, string_from_variant); - impl_ffi_variant!(ref StringName, string_name_to_variant, string_name_from_variant); - impl_ffi_variant!(ref NodePath, node_path_to_variant, node_path_from_variant); - impl_ffi_variant!(ref Signal, signal_to_variant, signal_from_variant); - impl_ffi_variant!(ref Callable, callable_to_variant, callable_from_variant); + // Types with RustVariant optimization (fit in Variant data, no destructors). + impl_ffi_variant!(bool, bool_to_variant, bool_from_variant, @rust_variant); + impl_ffi_variant!(i64, int_to_variant, int_from_variant; ParamMetadata::INT_IS_INT64, @rust_variant); + impl_ffi_variant!(f64, float_to_variant, float_from_variant; ParamMetadata::REAL_IS_DOUBLE, @rust_variant); + impl_ffi_variant!(Vector2i, vector2i_to_variant, vector2i_from_variant, @rust_variant); + impl_ffi_variant!(Vector3i, vector3i_to_variant, vector3i_from_variant, @rust_variant); + impl_ffi_variant!(Vector4i, vector4i_to_variant, vector4i_from_variant, @rust_variant); + impl_ffi_variant!(Color, color_to_variant, color_from_variant, @rust_variant); + impl_ffi_variant!(Rect2i, rect2i_to_variant, rect2i_from_variant, @rust_variant); + impl_ffi_variant!(Rid, rid_to_variant, rid_from_variant, @rust_variant); + + // Precision-dependent types with RustVariant optimization. + impl_ffi_variant!(Vector2, vector2_to_variant, vector2_from_variant, @rust_variant); + impl_ffi_variant!(Vector3, vector3_to_variant, vector3_from_variant, @rust_variant); + impl_ffi_variant!(Vector4, vector4_to_variant, vector4_from_variant, @rust_variant); + impl_ffi_variant!(Quaternion, quaternion_to_variant, quaternion_from_variant, @rust_variant); + impl_ffi_variant!(Plane, plane_to_variant, plane_from_variant, @rust_variant); + impl_ffi_variant!(Rect2, rect2_to_variant, rect2_from_variant, @rust_variant); + + // Large value types: exceed VARIANT_DATA_SIZE, so RustMarshal is not yet implemented. + // TODO: implement RustMarshal for these types and change to @rust_variant. + impl_ffi_variant!(Transform2D, transform_2d_to_variant, transform_2d_from_variant, @ffi); + impl_ffi_variant!(Transform3D, transform_3d_to_variant, transform_3d_from_variant, @ffi); + impl_ffi_variant!(Basis, basis_to_variant, basis_from_variant, @ffi); + impl_ffi_variant!(Projection, projection_to_variant, projection_from_variant, @ffi); + impl_ffi_variant!(Aabb, aabb_to_variant, aabb_from_variant, @ffi); + + // Ref-counted types: require FFI for construction/destruction; RustMarshal is not applicable. + impl_ffi_variant!(ref GString, string_to_variant, string_from_variant, @ffi); + impl_ffi_variant!(ref StringName, string_name_to_variant, string_name_from_variant, @ffi); + impl_ffi_variant!(ref NodePath, node_path_to_variant, node_path_from_variant, @ffi); + impl_ffi_variant!(ref Signal, signal_to_variant, signal_from_variant, @ffi); + impl_ffi_variant!(ref Callable, callable_to_variant, callable_from_variant, @ffi); } // ---------------------------------------------------------------------------------------------------------------------------------------------- diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index 0c8773213..6b7112aa9 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -21,6 +21,15 @@ use crate::meta::{ }; mod impls; +mod rust_variant; + +use rust_variant::{RustVariant, USE_RUST_MARSHAL}; + +#[cfg(feature = "trace")] // Test only -- exposes internals to external test crates. +#[doc(hidden)] +pub mod __test_only { + pub use super::rust_variant::{RustMarshal, RustVariant, SetError}; +} /// Godot variant type, able to store a variety of different types. /// @@ -35,7 +44,7 @@ mod impls; /// # Godot docs /// /// [`Variant` (stable)](https://docs.godotengine.org/en/stable/classes/class_variant.html) -// We rely on the layout of `Variant` being the same as Godot's layout in `borrow_slice` and `borrow_slice_mut`. +// We rely on the layout of `Variant` being the same as Godot's layout in `borrow_slice` and `borrow_slice_mut`, and for Array access. #[repr(transparent)] pub struct Variant { _opaque: sys::types::OpaqueVariant, @@ -289,10 +298,7 @@ impl Variant { } pub(crate) fn sys_type(&self) -> sys::GDExtensionVariantType { - unsafe { - let ty: sys::GDExtensionVariantType = interface_fn!(variant_get_type)(self.var_sys()); - ty - } + RustVariant::view(self).type_tag() } /// Return Godot's string representation of the variant. @@ -533,16 +539,30 @@ crate::meta::impl_godot_as_self!(Variant: ByVariant); impl Default for Variant { fn default() -> Self { - unsafe { - Self::new_with_var_uninit(|variant_ptr| { - interface_fn!(variant_new_nil)(variant_ptr); - }) + if USE_RUST_MARSHAL { + // A zeroed `Variant` is a valid NIL value: type_tag = GDEXTENSION_VARIANT_TYPE_NIL (0), data = 0. + // This matches Godot's `variant_new_nil`, which zero-initializes the variant. + // SAFETY: `Variant` is `#[repr(transparent)]` over `OpaqueVariant` (raw byte buffer); all-zero is valid. + unsafe { std::mem::zeroed() } + } else { + unsafe { + Self::new_with_var_uninit(|variant_ptr| { + interface_fn!(variant_new_nil)(variant_ptr); + }) + } } } } impl Clone for Variant { fn clone(&self) -> Self { + // POD types (`bool`, `i64`, `f64`, vectors, color, ...) have no shared ownership: a bytewise copy is sufficient. + // Both source and copy will skip `variant_destroy` in `Drop` (see below), so no double-free. + if USE_RUST_MARSHAL && RustVariant::view(self).is_pod() { + // SAFETY: `self` is initialized; POD types carry no destructor and copy-by-value is safe. + return unsafe { std::ptr::read(self) }; + } + unsafe { Self::new_with_var_uninit(|variant_ptr| { interface_fn!(variant_new_copy)(variant_ptr, self.var_sys()); @@ -553,6 +573,11 @@ impl Clone for Variant { impl Drop for Variant { fn drop(&mut self) { + // POD types have no Godot-side destructor; `variant_destroy` is a no-op for them. Skip the FFI call entirely. + if USE_RUST_MARSHAL && RustVariant::view(self).is_pod() { + return; + } + unsafe { interface_fn!(variant_destroy)(self.var_sys_mut()); } diff --git a/godot-core/src/builtin/variant/rust_variant.rs b/godot-core/src/builtin/variant/rust_variant.rs new file mode 100644 index 000000000..c2b29d4f9 --- /dev/null +++ b/godot-core/src/builtin/variant/rust_variant.rs @@ -0,0 +1,279 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +//! Pure-Rust view into Variant memory, enabling FFI-free access to scalar types. + +use godot_ffi as sys; + +use crate::builtin::{Variant, VariantType}; + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Layout types (internal) + +// Variant data size depends on precision: +// - Single precision (real_t=float): 16 bytes data, 24 bytes total +// - Double precision (real_t=double): 32 bytes data, 40 bytes total +#[cfg(not(feature = "double-precision"))] +pub(crate) const VARIANT_DATA_SIZE: usize = 16; + +#[cfg(feature = "double-precision")] +pub(crate) const VARIANT_DATA_SIZE: usize = 32; + +// Compile-time size/alignment checks. +const _: () = { + use std::mem::{align_of, size_of}; + + use crate::builtin::{Plane, Quaternion, Rect2, Vector2, Vector3, Vector4}; + + sys::static_assert_eq_size_align!(RustVariant, sys::types::OpaqueVariant); + + assert!(align_of::() == 8); + assert!(std::mem::offset_of!(RustVariant, type_tag) == 0); + assert!(std::mem::offset_of!(RustVariant, data) == 8); + + // Size depends on precision feature. + // Note: Use `assert!` instead of `assert_eq!` in const contexts (`assert_eq!` is not yet const-compatible). + #[cfg(not(feature = "double-precision"))] + assert!(size_of::() == 24); + + #[cfg(feature = "double-precision")] + assert!(size_of::() == 40); + + // Verify precision-dependent types fit in VARIANT_DATA_SIZE. + assert!(size_of::() <= VARIANT_DATA_SIZE); + assert!(size_of::() <= VARIANT_DATA_SIZE); + assert!(size_of::() <= VARIANT_DATA_SIZE); + assert!(size_of::() <= VARIANT_DATA_SIZE); + assert!(size_of::() <= VARIANT_DATA_SIZE); + assert!(size_of::() <= VARIANT_DATA_SIZE); +}; + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Feature toggle + +/// `true` when Rust-side variant marshalling is enabled (default). `false` selects the FFI fallback (`variant-ffi-marshal` feature). +/// +/// Use this constant in `if`-expressions instead of `#[cfg]` blocks: the dead branch is eliminated by LLVM, but both branches type-check +/// under either feature setting, keeping the source uniform. +pub(crate) const USE_RUST_MARSHAL: bool = !cfg!(feature = "variant-ffi-marshal"); + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Public API + +/// Marker trait for plain-old-data types that can be marshalled from/to `Variant` in pure Rust. +/// +/// Enables direct memory marshalling into the `Variant` data union without going through FFI. +/// +/// The supertrait `GodotType` encodes the core contract: the type is its own FFI carrier (no widening needed), which enables +/// sound in-memory marshalling. +/// +/// # Safety +/// Implementors must: +/// - Fit in `VARIANT_DATA_SIZE` (16 or 32 bytes depending on precision; verified at macro expansion site). +/// - Have `#[repr(C)]` layout matching Godot's in-memory representation for the corresponding variant type. +pub unsafe trait RustMarshal: + crate::meta::GodotType + Copy + sys::GodotFfi +{ + /// The Godot [`VariantType`] for this type. + const VARIANT_TYPE: VariantType = ::VARIANT_TYPE.variant_as_nil(); +} + +/// Mutable or immutable view into a [`Variant`], providing FFI-free read/write access for POD types. +/// +/// Supported types implement [`RustMarshal`] and fit within the variant's inline data storage. +/// +/// The [`GodotFfiVariant`][crate::meta::GodotFfiVariant] trait dispatches internally: types implementing [`RustMarshal`] use direct memory +/// access; all others go through Godot's FFI layer. +/// +/// Internal type. Not exposed in the public API; reachable from test crates via the `__trace` feature. +/// +/// # Memory management +/// **POD types** (`bool`, `i64`, `f64`, `Vector2/3/4`, `Color`, etc.) use value copying. Each variant holds independent data; modifying one +/// does not affect clones. These types have no destructor, so `set_value()` can overwrite them without leaking. +/// +/// **Shared types** (`Object`, `Array`, `Dictionary`, `GString`, etc.) use reference counting and require destruction. +/// They cannot be overwritten via `set_value()`, as that would silently leak the ref-counted resource. +#[repr(C, align(8))] +pub struct RustVariant { + // Godot stores the variant type as a C++ enum (32-bit signed). We use u32 here because the Rust binding may expose GDExtensionVariantType + // as either i32 or u32 depending on platform. All valid type ordinals are non-negative (0..=38), so the cast between the two is always safe. + type_tag: u32, // 4 bytes. + _padding: u32, // 4 bytes padding to align the data union to 8 bytes. + data: [u8; VARIANT_DATA_SIZE], // 16 bytes (32 in double-precision). +} + +impl RustVariant { + /// Create an immutable view from a Variant reference. + pub fn view(variant: &Variant) -> &Self { + // SAFETY: OpaqueVariant and RustVariant have the same size/alignment (verified at compile time). + unsafe { std::mem::transmute::<&Variant, &RustVariant>(variant) } + } + + /// Create a mutable view from a Variant reference. + /// + /// This is safe even for variants holding ref-counted types: `&mut Variant` guarantees exclusive access to the handle itself. + /// The guarded `set_value()` method prevents overwriting such types without proper destruction. + /// + /// Gated on `trace` because mutable views (and the `set_value`/`SetError` machinery below) are only used by integration tests + /// in `itest/`. They are not part of the production fast path and would otherwise widen the internal API surface unnecessarily. + #[cfg(feature = "trace")] + pub fn view_mut(variant: &mut Variant) -> &mut Self { + // SAFETY: OpaqueVariant and RustVariant have the same size/alignment (verified at compile time). + unsafe { std::mem::transmute::<&mut Variant, &mut RustVariant>(variant) } + } + + /// Construct a [`Variant`] from a POD value without going through FFI. + /// + /// Writes the type tag and data bytes directly into a stack-allocated `RustVariant`, then transmutes it to `Variant`. + /// `Variant::drop()` calls `variant_destroy`, which is a no-op for POD types (`T: RustMarshal`). + pub(crate) fn from_pod(value: T) -> Variant { + let mut rv = RustVariant { + // VariantType::ord is i32, ordinals are non-negative -> cast to u32 is safe. + type_tag: ::VARIANT_TYPE.ord as u32, + _padding: 0, + data: [0u8; VARIANT_DATA_SIZE], + }; + + // SAFETY: `RustMarshal` guarantees `T` has `#[repr(C)]` layout matching Godot's in-memory representation and fits in `VARIANT_DATA_SIZE`. + unsafe { rv.data_ptr_mut::().write(value) }; + + // SAFETY: `Variant` is `#[repr(transparent)]` over `OpaqueVariant`; `RustVariant` has identical size and alignment (asserted above). + unsafe { std::mem::transmute::(rv) } + } + + /// Get the raw type tag without FFI. + /// + /// Unlike [`Variant::get_type()`], this does not handle the special case of null object pointers. + #[inline] + pub(crate) fn type_tag(&self) -> sys::GDExtensionVariantType { + self.type_tag as sys::GDExtensionVariantType + } + + /// Get the variant type without FFI. + /// + /// Unlike [`Variant::get_type()`], this does not handle the special case of null object pointers. + pub fn get_type(&self) -> VariantType { + VariantType::from_sys(self.type_tag()) + } + + /// Returns `true` if the variant currently holds "plain old data" (no destructor needed). + /// + /// Inverse of [`VariantType::needs_ffi_destruction()`]; used by `Variant`'s lifecycle fast paths (Clone/Drop) and `set_value`. + #[inline] + pub(crate) fn is_pod(&self) -> bool { + !self.get_type().needs_ffi_destruction() + } + + /// Returns `true` if the variant currently holds a value of type `T`. + #[inline] + pub(crate) fn is_type(&self) -> bool { + self.get_type() == ::VARIANT_TYPE + } + + /// Get a typed value from the variant without going through FFI. + /// + /// Returns `None` if the variant's type doesn't match `T`. + pub fn get_value(&self) -> Option { + if !self.is_type::() { + return None; + } + + // SAFETY: type was verified to match T via `is_type`. + let value: T = unsafe { self.data_ptr::().read() }; + Some(value) + } + + /// Set the variant to a value without going through FFI. + /// + /// Returns `Err` if the variant currently holds a shared type (e.g. `Array`, `GString`) that requires FFI destruction. + /// Overwriting such types without calling their destructor would silently leak the ref-counted resource. + /// For those types, use the standard `to_variant()` / `from_variant()` API instead. + #[cfg(feature = "trace")] + pub fn set_value(&mut self, value: T) -> Result<(), SetError> { + if !self.is_pod() { + return Err(SetError { + current_type: self.get_type(), + }); + } + + self.type_tag = ::VARIANT_TYPE.ord as u32; + + // SAFETY: current type is POD (checked above), so no destructor is skipped. `type_tag` now matches `T`. + unsafe { self.data_ptr_mut::().write(value) }; + + Ok(()) + } + + /// Returns a raw typed pointer to the variant's data for reading. + #[inline] + fn data_ptr(&self) -> *const T { + sys::strict_assert!(self.is_type::()); + self.data.as_ptr().cast::() + } + + /// Returns a raw typed mutable pointer to the variant's data for writing. + #[inline] + fn data_ptr_mut(&mut self) -> *mut T { + sys::strict_assert!(self.is_type::()); + self.data.as_mut_ptr().cast::() + } +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Error types + +/// Error returned when trying to overwrite a variant that holds a complex type. +/// +/// Complex types (`String`, `Array`, `Dictionary`, etc.) require destruction before being overwritten. +/// Use the standard `to_variant()` / `from_variant()` API for those types. +#[cfg(feature = "trace")] +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SetError { + /// The type currently held by the variant. + pub current_type: VariantType, +} + +#[cfg(feature = "trace")] +impl std::fmt::Display for SetError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "cannot overwrite variant of type {:?} without destruction", + self.current_type + ) + } +} + +#[cfg(feature = "trace")] +impl std::error::Error for SetError {} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// RustMarshal implementations + +use crate::builtin::{ + Color, Plane, Quaternion, Rect2, Rect2i, Rid, Vector2, Vector2i, Vector3, Vector3i, Vector4, + Vector4i, +}; + +// Following types always fit into Variant data segment (precision-independent): +unsafe impl RustMarshal for bool {} +unsafe impl RustMarshal for i64 {} +unsafe impl RustMarshal for f64 {} +unsafe impl RustMarshal for Vector2i {} +unsafe impl RustMarshal for Vector3i {} +unsafe impl RustMarshal for Vector4i {} +unsafe impl RustMarshal for Color {} +unsafe impl RustMarshal for Rect2i {} +unsafe impl RustMarshal for Rid {} + +// Precision-dependent types that fit in both single and double precision modes. +unsafe impl RustMarshal for Vector2 {} +unsafe impl RustMarshal for Vector3 {} +unsafe impl RustMarshal for Vector4 {} +unsafe impl RustMarshal for Quaternion {} +unsafe impl RustMarshal for Plane {} +unsafe impl RustMarshal for Rect2 {} diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index 9162a947e..5c107febf 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -78,6 +78,8 @@ pub use raw_ptr::{FfiRawPointer, RawPtr}; #[cfg(feature = "trace")] pub use signature::trace; pub use signed_range::{SignedRange, wrapped}; +#[doc(hidden)] +pub use traits::GodotFfiVariant; pub use traits::{Element, GodotImmutable, GodotType, PackedElement, element_variant_type}; pub use uniform_object_deref::UniformObjectDeref; @@ -91,9 +93,7 @@ pub use crate::arg_into_ref; mod reexport_crate { pub(crate) use super::param_tuple::TupleFromGodot; pub(crate) use super::signature::{CallContext, Signature, varcall_return_checked}; - pub(crate) use super::traits::{ - ExtVariantType, GodotFfiVariant, GodotNullableType, ffi_variant_type, - }; + pub(crate) use super::traits::{ExtVariantType, GodotNullableType, ffi_variant_type}; pub(crate) use crate::impl_godot_as_self; // Private imports for this module only. pub(super) use crate::registry::method::MethodParamOrReturnInfo; diff --git a/godot-core/src/meta/traits.rs b/godot-core/src/meta/traits.rs index 235050f0a..29d6dfa1a 100644 --- a/godot-core/src/meta/traits.rs +++ b/godot-core/src/meta/traits.rs @@ -20,9 +20,16 @@ pub use sys::{ExtVariantType, GodotFfi}; pub use crate::builtin::meta_reexport::PackedElement; /// Conversion of [`GodotFfi`] types to/from [`Variant`]. +/// +/// Implementations may use rust-side marshalling for types where [`RustMarshal`][crate::builtin::RustMarshal] is implemented, bypassing FFI. +/// +/// This trait overlaps strongly with `GodotType`, but currently has an `impl ObjectArg<'gd>` on top. Vice versa, following types implement +/// `GodotType` but not `GodotFfiVariant`: `u8, u16, u32, i8, i16, i32, f32, u64`. #[doc(hidden)] pub trait GodotFfiVariant: Sized + GodotFfi { + // TODO(v0.5.x): rename -> rust_{to,from}_variant fn ffi_to_variant(&self) -> Variant; + fn ffi_from_variant(variant: &Variant) -> Result; } diff --git a/godot-macros/src/bench.rs b/godot-macros/src/bench.rs index 741dd7619..78657da2d 100644 --- a/godot-macros/src/bench.rs +++ b/godot-macros/src/bench.rs @@ -53,6 +53,13 @@ pub fn attribute_bench(input_decl: venial::Item) -> ParseResult { let other_attributes: Vec<_> = retain_attributes_except(&func.attributes, "bench").collect(); let generated_fn = if manual { + if func.return_ty.is_none() { + return bail!( + func, + "#[bench(manual)] function must return crate::framework::BenchResult" + ); + } + // Manual mode: user calls bench_measure() directly. let ret = func.return_ty; quote! { diff --git a/godot/Cargo.toml b/godot/Cargo.toml index 625b0d5d0..4bbfd13af 100644 --- a/godot/Cargo.toml +++ b/godot/Cargo.toml @@ -49,6 +49,7 @@ default = ["__codegen-full"] __codegen-full = ["godot-core/codegen-full", "godot-macros/codegen-full"] __debug-log = ["godot-core/debug-log"] __trace = ["godot-core/trace"] +__variant-ffi-marshal = ["godot-core/variant-ffi-marshal"] [dependencies] godot-core = { path = "../godot-core", version = "=0.5.3" } diff --git a/godot/src/lib.rs b/godot/src/lib.rs index fc5e44a4e..a04e8490d 100644 --- a/godot/src/lib.rs +++ b/godot/src/lib.rs @@ -260,6 +260,8 @@ pub mod init { /// Meta-information about Godot types, their properties and conversions between them. pub mod meta { + #[doc(hidden)] + pub use godot_core::meta::GodotFfiVariant; pub use godot_core::meta::{ AsArg, ClassId, Element, EngineFromGodot, EngineToGodot, FromGodot, GodotConvert, GodotImmutable, GodotType, ObjectArg, PackedElement, SignedRange, ToArg, ToGodot, diff --git a/itest/rust/src/benchmarks/array.rs b/itest/rust/src/benchmarks/array.rs new file mode 100644 index 000000000..814318ea6 --- /dev/null +++ b/itest/rust/src/benchmarks/array.rs @@ -0,0 +1,32 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +use std::hint::black_box; + +use godot::builtin::{Array, GString}; + +use crate::framework::{BenchResult, bench, bench_measure}; + +#[bench(manual)] +fn array_extend_i64() -> BenchResult { + bench_measure(1, || { + let mut arr = Array::new(); + arr.extend((10_000i64..20_000).map(black_box)); + black_box(arr) + }) +} + +#[bench(manual)] +fn array_extend_gstring() -> BenchResult { + bench_measure(1, || { + let mut arr = Array::new(); + arr.extend( + (10_000..20_000).map(|i| black_box(GString::from(format!("str_{}", i).as_str()))), + ); + black_box(arr) + }) +} diff --git a/itest/rust/src/benchmarks/mod.rs b/itest/rust/src/benchmarks/mod.rs index 63ddc5ee4..53a2e4b9a 100644 --- a/itest/rust/src/benchmarks/mod.rs +++ b/itest/rust/src/benchmarks/mod.rs @@ -10,15 +10,17 @@ use std::hint::black_box; use godot::builtin::inner::InnerRect2i; -use godot::builtin::{GString, PackedInt32Array, Rect2i, StringName, Vector2i}; +use godot::builtin::{GString, PackedFloat32Array, PackedInt32Array, Rect2i, StringName, Vector2i}; use godot::classes::{Node3D, Os, RefCounted}; use godot::obj::{Gd, InstanceId, NewAlloc, NewGd, Singleton}; use godot::register::GodotClass; -use crate::framework::bench; +use crate::framework::{BenchResult, bench, bench_measure}; +mod array; mod callable; mod color; +mod variant; #[bench] fn builtin_string_ctor() -> GString { @@ -110,6 +112,30 @@ fn packed_array_from_iter_unknown_size() -> PackedInt32Array { })) } +#[bench(manual)] +fn packed_f32_to_array() -> BenchResult { + let packed = PackedFloat32Array::from_iter((0..10_000).map(|v: i32| v as f32)); + bench_measure(1, || packed.to_typed_array()) +} + +#[bench(manual)] +fn packed_f32_to_var_array() -> BenchResult { + let packed = PackedFloat32Array::from_iter((0..10_000).map(|v: i32| v as f32)); + bench_measure(1, || packed.to_var_array()) +} + +#[bench(manual)] +fn packed_f32_to_array_small() -> BenchResult { + let packed = PackedFloat32Array::from_iter((0..100).map(|v: i32| v as f32)); + bench_measure(1, || packed.to_typed_array()) +} + +#[bench(manual)] +fn packed_f32_to_var_array_small() -> BenchResult { + let packed = PackedFloat32Array::from_iter((0..100).map(|v: i32| v as f32)); + bench_measure(1, || packed.to_var_array()) +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Helpers for benchmarks above diff --git a/itest/rust/src/benchmarks/variant.rs b/itest/rust/src/benchmarks/variant.rs new file mode 100644 index 000000000..8be9b0c69 --- /dev/null +++ b/itest/rust/src/benchmarks/variant.rs @@ -0,0 +1,162 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +use std::hint::black_box; + +use godot::builtin::{Color, Rid, Variant, Vector2i, Vector3}; +use godot::meta::{FromGodot, ToGodot}; + +use crate::framework::{BenchResult, bench, bench_measure}; + +// Scalar types. + +#[bench] +fn variant_from_bool() -> Variant { + black_box(true).to_variant() +} + +#[bench] +fn variant_to_bool() -> bool { + bool::from_variant(black_box(&true.to_variant())) +} + +#[bench] +fn variant_from_i64() -> Variant { + black_box(12345_i64).to_variant() +} + +#[bench] +fn variant_to_i64() -> i64 { + i64::from_variant(black_box(&12345_i64.to_variant())) +} + +#[bench] +fn variant_from_f64() -> Variant { + black_box(1.234_f64).to_variant() +} + +#[bench] +fn variant_to_f64() -> f64 { + f64::from_variant(black_box(&1.234_f64.to_variant())) +} + +// Vector types. + +#[bench] +fn variant_from_vector2i() -> Variant { + black_box(Vector2i::new(100, 200)).to_variant() +} + +#[bench] +fn variant_to_vector2i() -> Vector2i { + Vector2i::from_variant(black_box(&Vector2i::new(100, 200).to_variant())) +} + +#[bench] +fn variant_from_vector3() -> Variant { + black_box(Vector3::new(1.5, 2.5, 3.5)).to_variant() +} + +#[bench] +fn variant_to_vector3() -> Vector3 { + Vector3::from_variant(black_box(&Vector3::new(1.5, 2.5, 3.5).to_variant())) +} + +// Other POD types. + +#[bench] +fn variant_from_color() -> Variant { + black_box(Color::from_rgba(0.5, 0.3, 0.8, 1.0)).to_variant() +} + +#[bench] +fn variant_to_color() -> Color { + Color::from_variant(black_box( + &Color::from_rgba(0.5, 0.3, 0.8, 1.0).to_variant(), + )) +} + +#[bench] +fn variant_from_rid() -> Variant { + black_box(Rid::new(12345)).to_variant() +} + +#[bench] +fn variant_to_rid() -> Rid { + Rid::from_variant(black_box(&Rid::new(12345).to_variant())) +} + +// Lifecycle: nil construction, clone, drop. + +#[bench] +fn variant_nil_ctor() -> Variant { + Variant::nil() +} + +#[bench] +fn variant_clone_i64() -> Variant { + let v = black_box(12345_i64).to_variant(); + black_box(&v).clone() +} + +#[bench] +fn variant_clone_vector3() -> Variant { + let v = black_box(Vector3::new(1.0, 2.0, 3.0)).to_variant(); + black_box(&v).clone() +} + +#[bench(manual)] +fn variant_drop_i64_x1000() -> BenchResult { + bench_measure(1, || { + let mut count = 0_i64; + for i in 0..1000_i64 { + let v = black_box(i).to_variant(); + count += 1; + drop(black_box(v)); + } + black_box(count) + }) +} + +#[bench(manual)] +fn variant_drop_vector3_x1000() -> BenchResult { + bench_measure(1, || { + let mut count = 0_i32; + for i in 0..1000_i32 { + let v = black_box(Vector3::new(i as f32, i as f32, i as f32)).to_variant(); + count += 1; + drop(black_box(v)); + } + black_box(count) + }) +} + +// Bulk round-trips (representative of real use: many values through variants). + +#[bench(manual)] +fn variant_roundtrip_i64_x1000() -> BenchResult { + bench_measure(1, || { + let mut sum = 0i64; + for i in 0..1000_i64 { + let v = black_box(i).to_variant(); + sum = sum.wrapping_add(i64::from_variant(black_box(&v))); + } + black_box(sum) + }) +} + +#[bench(manual)] +fn variant_roundtrip_vector3_x1000() -> BenchResult { + bench_measure(1, || { + let mut result = Vector3::ZERO; + for i in 0..1000_i32 { + let v = black_box(Vector3::new(i as f32, i as f32 * 2.0, i as f32 * 3.0)).to_variant(); + result += Vector3::from_variant(black_box(&v)); + } + black_box(result) + }) +} diff --git a/itest/rust/src/builtin_tests/containers/rust_variant_test.rs b/itest/rust/src/builtin_tests/containers/rust_variant_test.rs new file mode 100644 index 000000000..7a7516a02 --- /dev/null +++ b/itest/rust/src/builtin_tests/containers/rust_variant_test.rs @@ -0,0 +1,365 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +use godot::builtin::__test_only::{RustMarshal, RustVariant, SetError}; +use godot::builtin::{ + Color, GString, Plane, Quaternion, Rect2, Rect2i, Rid, Variant, VariantType, Vector2, Vector2i, + Vector3, Vector3i, Vector4, Vector4i, varray, +}; +use godot::meta::{FromGodot, ToGodot}; + +use crate::framework::itest; + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Generic Test Infrastructure + +/// Verify variant round-trip: public conversion API, direct `RustVariant` view, and `set_value` path all agree. +fn verify_variant_roundtrip(value: T, index: usize) +where + T: RustMarshal + FromGodot + ToGodot + PartialEq + std::fmt::Debug + Copy, +{ + let label = || format!("{}[{}]", std::any::type_name::(), index); + let variant = value.to_variant(); + + assert_eq!( + variant.get_type(), + ::VARIANT_TYPE, + "{}: unexpected variant type", + label(), + ); + + // Public path: full conversion API. + let via_public = T::from_variant(&variant); + assert_eq!( + value, + via_public, + "{}: public from_variant mismatch", + label() + ); + + // Direct path: inline memory access via RustVariant (read-only view). + let variant_copy = variant.clone(); + let via_view = RustVariant::view(&variant_copy).get_value::(); + assert_eq!( + Some(value), + via_view, + "{}: RustVariant view mismatch", + label() + ); + + // Set path: write via RustVariant, then read both ways. + let mut set_variant = Variant::nil(); + RustVariant::view_mut(&mut set_variant) + .set_value(value) + .unwrap_or_else(|_| panic!("{}: set_value failed on nil", label())); + assert_eq!( + Some(value), + RustVariant::view(&set_variant).get_value::(), + "{}: set_value -> get_value mismatch", + label(), + ); + assert_eq!( + value, + T::from_variant(&set_variant), + "{}: set_value -> FromGodot mismatch", + label(), + ); +} + +/// Macro to generate comprehensive tests for a type. +macro_rules! impl_variant_test { + ($T:ty, $test_name:ident, [$($test_val:expr),+ $(,)?]) => { + #[itest] + fn $test_name() { + for (i, value) in [$($test_val),+].iter().enumerate() { + verify_variant_roundtrip::<$T>(*value, i); + } + } + }; +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Implementations for RustMarshal Types + +impl_variant_test!(bool, rust_variant_roundtrip_bool, [true, false]); + +impl_variant_test!( + i64, + rust_variant_roundtrip_i64, + [ + 0, + 1, + -1, + 42, + -12345, + i64::MIN, + i64::MAX, + i64::MIN + 1, + i64::MAX - 1 + ] +); + +impl_variant_test!( + f64, + rust_variant_roundtrip_f64, + [ + 0.0, + 1.0, + -1.0, + 3.125, + -1.5e10, + 99.5, + f64::MIN, + f64::MAX, + f64::EPSILON, + -f64::EPSILON + ] +); + +impl_variant_test!( + Vector2i, + rust_variant_roundtrip_vector2i, + [ + Vector2i::ZERO, + Vector2i::ONE, + Vector2i::new(i32::MIN, i32::MAX), + Vector2i::new(-1, -1), + Vector2i::new(100, -200) + ] +); + +impl_variant_test!( + Vector3i, + rust_variant_roundtrip_vector3i, + [ + Vector3i::ZERO, + Vector3i::ONE, + Vector3i::new(-1, i32::MIN, i32::MAX), + Vector3i::new(100, 200, 300), + Vector3i::new(-100, -200, -300) + ] +); + +impl_variant_test!( + Vector4i, + rust_variant_roundtrip_vector4i, + [ + Vector4i::ZERO, + Vector4i::ONE, + Vector4i::new(-1, i32::MIN, i32::MAX, 1000), + Vector4i::new(1, 2, 3, 4), + Vector4i::new(-1, -2, -3, -4) + ] +); + +impl_variant_test!( + Color, + rust_variant_roundtrip_color, + [ + Color::from_rgba(0.0, 0.0, 0.0, 1.0), + Color::from_rgba(1.0, 1.0, 1.0, 1.0), + Color::from_rgba(0.7, 0.5, 0.3, 0.2), + Color::from_rgba(0.0, 0.0, 0.0, 0.0) + ] +); + +impl_variant_test!( + Rect2i, + rust_variant_roundtrip_rect2i, + [ + Rect2i::default(), + Rect2i::new(Vector2i::ZERO, Vector2i::new(100, 200)), + Rect2i::new(Vector2i::new(-50, -50), Vector2i::new(100, 100)) + ] +); + +impl_variant_test!( + Rid, + rust_variant_roundtrip_rid, + [ + Rid::Invalid, + Rid::new(1), + Rid::new(12345), + Rid::new(u64::MAX), + ] +); + +// Precision-dependent types (fit in both single and double precision). +impl_variant_test!( + Vector2, + rust_variant_roundtrip_vector2, + [ + Vector2::ZERO, + Vector2::ONE, + Vector2::new(12.5, -3.5), + Vector2::new(-100.0, 200.0) + ] +); + +impl_variant_test!( + Vector3, + rust_variant_roundtrip_vector3, + [ + Vector3::ZERO, + Vector3::ONE, + Vector3::new(1.5, 2.5, 3.5), + Vector3::new(117.5, 100.0, -323.25), + Vector3::new(-1.0, -2.0, -3.0) + ] +); + +impl_variant_test!( + Vector4, + rust_variant_roundtrip_vector4, + [ + Vector4::ZERO, + Vector4::ONE, + Vector4::new(-18.5, 24.75, -1.25, 777.875), + Vector4::new(1.0, 2.0, 3.0, 4.0) + ] +); + +impl_variant_test!( + Quaternion, + rust_variant_roundtrip_quaternion, + [ + Quaternion::default(), + Quaternion::new(0.0, 0.0, 0.0, 1.0), + Quaternion::new(0.5, 0.5, 0.5, 0.5) + ] +); + +impl_variant_test!( + Plane, + rust_variant_roundtrip_plane, + [ + Plane::new(Vector3::new(1.0, 0.0, 0.0), 0.0), + Plane::new(Vector3::new(0.0, 1.0, 0.0), 10.0), + Plane::new(Vector3::new(0.0, 0.0, 1.0), -5.0) + ] +); + +impl_variant_test!( + Rect2, + rust_variant_roundtrip_rect2, + [ + Rect2::default(), + Rect2::new(Vector2::ZERO, Vector2::new(100.0, 200.0)), + Rect2::new(Vector2::new(-50.0, -50.0), Vector2::new(100.0, 100.0)) + ] +); + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Edge Case Tests + +#[itest] +fn rust_variant_type_mismatch() { + // Reading wrong type returns None, both for nil source and typed source. + let nil_variant = Variant::nil(); + let view = RustVariant::view(&nil_variant); + assert_eq!(view.get_type(), VariantType::NIL); + assert_eq!(view.get_value::(), None); + assert_eq!(view.get_value::(), None); + + let int_variant = Variant::from(42i64); + let view = RustVariant::view(&int_variant); + assert_eq!(view.get_value::(), None); + assert_eq!(view.get_value::(), None); + + let bool_variant = Variant::from(true); + let view = RustVariant::view(&bool_variant); + assert_eq!(view.get_value::(), None); + assert_eq!(view.get_value::(), None); +} + +#[itest] +fn rust_variant_special_floats() { + // NaN/infinity round-trip through both the public API and the direct RustVariant view. + for value in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + let variant = value.to_variant(); + let via_view = RustVariant::view(&variant).get_value::().unwrap(); + let via_public = f64::from_variant(&variant); + + if value.is_nan() { + assert!(via_view.is_nan()); + assert!(via_public.is_nan()); + } else { + assert_eq!(via_view, value); + assert_eq!(via_public, value); + } + } +} + +#[itest] +fn rust_variant_setters() { + // Start with nil, set to int, float, bool. Test multiple types in one test. + let mut variant = Variant::nil(); + let view = RustVariant::view_mut(&mut variant); + + assert!(view.set_value(123i64).is_ok()); + assert_eq!(view.get_value::(), Some(123)); + assert_eq!(view.get_type(), VariantType::INT); + + // Change int to float. + assert!(view.set_value(2.72f64).is_ok()); + assert_eq!(view.get_value::(), Some(2.72)); + assert_eq!(view.get_type(), VariantType::FLOAT); + + // Change float to bool. + assert!(view.set_value(true).is_ok()); + assert_eq!(view.get_value::(), Some(true)); + assert_eq!(view.get_type(), VariantType::BOOL); + + // Verify the underlying Variant was actually modified via FFI. + let extracted: bool = variant.to(); + assert!(extracted); +} + +#[itest] +fn rust_variant_setters_reject_complex_types() { + // String is a complex type that needs destruction. + let mut string_variant = Variant::from(GString::from("hello")); + let view = RustVariant::view_mut(&mut string_variant); + + // Should fail because String needs destruction. + let SetError { current_type } = view.set_value(42i64).unwrap_err(); + assert_eq!(current_type, VariantType::STRING); + + // Array is also a complex type. + let mut array_variant = Variant::from(varray![1, 2, 3]); + let view = RustVariant::view_mut(&mut array_variant); + assert!(view.set_value(true).is_err()); +} + +#[itest] +fn rust_variant_clone_independence() { + // Test that cloning creates independent copies via RustMarshal. + let original = Vector2i::new(10, 20); + let modified = Vector2i::new(99, 88); + + let mut variant1 = Variant::nil(); + RustVariant::view_mut(&mut variant1) + .set_value(original) + .unwrap(); + + let variant2 = variant1.clone(); + + // Change variant1 to a different value. + RustVariant::view_mut(&mut variant1) + .set_value(modified) + .unwrap(); + + // variant2 should still have original value. + assert_eq!( + RustVariant::view(&variant2).get_value::(), + Some(original) + ); + assert_eq!( + RustVariant::view(&variant1).get_value::(), + Some(modified) + ); +} diff --git a/itest/rust/src/builtin_tests/containers/variant_test.rs b/itest/rust/src/builtin_tests/containers/variant_test.rs index 44c0af33f..569538e88 100644 --- a/itest/rust/src/builtin_tests/containers/variant_test.rs +++ b/itest/rust/src/builtin_tests/containers/variant_test.rs @@ -883,3 +883,5 @@ fn gstr(s: &str) -> GString { fn sname(s: &str) -> StringName { StringName::from(s) } + +// RustVariant tests in rust_variant_test.rs. diff --git a/itest/rust/src/builtin_tests/mod.rs b/itest/rust/src/builtin_tests/mod.rs index 76ae17877..d54e1f1d0 100644 --- a/itest/rust/src/builtin_tests/mod.rs +++ b/itest/rust/src/builtin_tests/mod.rs @@ -31,6 +31,7 @@ mod containers { mod dictionary_test; mod packed_array_test; mod rid_test; + mod rust_variant_test; mod signal_disconnect_test; mod signal_test; mod variant_test;