From 1a047c739dab7ef6929f06495c574784fc7edcaf Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Thu, 21 May 2026 20:58:51 +0200 Subject: [PATCH 1/5] Add `VariantType::needs_ffi_destruction()` via codegen Distinguishes variant types that own resources (require Godot-side destruction) from plain-data types. Used by rust-side marshalling to detect when FFI fast paths are safe. --- godot-codegen/src/generator/central_files.rs | 64 ++++++++++++++++++++ godot-codegen/src/models/domain.rs | 1 + godot-codegen/src/models/domain_mapping.rs | 4 ++ 3 files changed, 69 insertions(+) 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, } } } From 0b170f0f40e152c2368994812317f1849c810b32 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Thu, 21 May 2026 20:58:54 +0200 Subject: [PATCH 2/5] Validate return type for `#[bench(manual)]` functions --- godot-macros/src/bench.rs | 7 +++++++ 1 file changed, 7 insertions(+) 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! { From fc55c8c1e6c11ae291cd84dfa67d8b0710ed0d51 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Thu, 21 May 2026 20:59:02 +0200 Subject: [PATCH 3/5] Add `Array::resize_default` and accelerate `Extend` `Extend` now pre-allocates based on the iterator's size hint, removing per-element grow overhead for size-known iterators. `resize_default` complements `resize()` for default-constructible types where no explicit fill value is needed. --- godot-core/src/builtin/collections/array.rs | 77 ++++++++++++++++----- 1 file changed, 59 insertions(+), 18 deletions(-) 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)); } } From c39688a718ac771315349ce695e7ea0b5e690a25 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Thu, 21 May 2026 20:59:06 +0200 Subject: [PATCH 4/5] Bypass FFI for plain-data Variant conversions POD builtin types (numbers, vectors, color, RID) marshal directly through Variant's inline memory layout, skipping Godot's variant FFI for both value conversion and Variant lifecycle (clone, drop, default). A `variant-ffi-marshal` feature toggles back to the original FFI path, enabling A/B benchmarking. --- godot-core/Cargo.toml | 2 + godot-core/src/builtin/variant/impls.rs | 165 ++++++++--- godot-core/src/builtin/variant/mod.rs | 43 ++- .../src/builtin/variant/rust_variant.rs | 279 ++++++++++++++++++ godot-core/src/meta/mod.rs | 6 +- godot-core/src/meta/traits.rs | 7 + godot/Cargo.toml | 1 + godot/src/lib.rs | 2 + 8 files changed, 451 insertions(+), 54 deletions(-) create mode 100644 godot-core/src/builtin/variant/rust_variant.rs 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/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/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, From c9bb01f1382754c125462eb3f8f856cb98563cd1 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Thu, 21 May 2026 20:59:11 +0200 Subject: [PATCH 5/5] Add integration tests and benchmarks for variant marshalling Roundtrip tests cover all `RustMarshal` types via both the public conversion API and the direct memory view. Benches measure `Array::extend` and packed-to-typed conversion paths. --- itest/rust/src/benchmarks/array.rs | 32 ++ itest/rust/src/benchmarks/mod.rs | 30 +- itest/rust/src/benchmarks/variant.rs | 162 ++++++++ .../containers/rust_variant_test.rs | 365 ++++++++++++++++++ .../builtin_tests/containers/variant_test.rs | 2 + itest/rust/src/builtin_tests/mod.rs | 1 + 6 files changed, 590 insertions(+), 2 deletions(-) create mode 100644 itest/rust/src/benchmarks/array.rs create mode 100644 itest/rust/src/benchmarks/variant.rs create mode 100644 itest/rust/src/builtin_tests/containers/rust_variant_test.rs 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;