Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions godot-codegen/src/generator/central_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 needs_ffi_destruction_method = make_needs_ffi_destruction_method(api);

quote! {
#[cfg(target_pointer_width = "32")]
Expand Down Expand Up @@ -49,6 +50,7 @@ pub fn make_sys_central_code(api: &ExtensionApi) -> TokenStream {
}

#godot_type_name_method
#needs_ffi_destruction_method
}
}
}
Expand Down Expand Up @@ -228,6 +230,53 @@ 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` on drop, for two reasons:
/// 1. Non-trivial destructor in `extension_api.json` (refcounted: `String`, `Array`, `Dictionary`, `Object`, ...).
/// 2. Stored heap-allocated inside `Variant` because it exceeds the inline data segment (`Transform2D`, `Transform3D`, `Basis`, `Aabb`,
/// `Projection`). Godot reports `has_destructor=false` for these (trivially destructible in C++), but `Variant` still owns the allocation.
fn make_needs_ffi_destruction_method(api: &ExtensionApi) -> TokenStream {
use crate::models::domain::BuildConfiguration;

// Detecting case 2 needs the type's byte size, which differs per precision: the inline buffer is 16 bytes single / 32 bytes double, and a
// type's size at most doubles in double precision (real: 4 -> 8 bytes; int/ptr unchanged). Hence `size_single > 16` iff `size_double > 32`,
// so the single-precision (Float32) sizes alone decide it for both builds. The flagged pure-float types are too large 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 `has_destructor` from `extension_api.json` (refcounted types) with builtin size (heap-allocated types such as
/// `Transform2D`/`Basis`/`Aabb`/`Transform3D`/`Projection`).
#[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.
Expand Down
1 change: 1 addition & 0 deletions godot-codegen/src/models/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ pub struct BuiltinVariant {
pub builtin_class: Option<BuiltinClass>,

pub variant_type_ord: i32,
pub has_destructor: bool,
}

impl BuiltinVariant {
Expand Down
4 changes: 4 additions & 0 deletions godot-codegen/src/models/domain_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
Expand All @@ -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,
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions godot-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
82 changes: 64 additions & 18 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,13 +466,7 @@ impl<T: Element> Array<T> {
///
/// 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<T>) {
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);

Expand All @@ -491,6 +485,30 @@ impl<T: Element> Array<T> {
}
}

/// 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<RefCounted> 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<T>) {
self.balanced_ensure_mutable();
Expand Down Expand Up @@ -828,8 +846,11 @@ impl<T: Element> Array<T> {
/// 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 a multi-slot `&mut [Variant]` from this pointer unless the array is uniquely owned (refcount == 1): `Array` is ref-counted,
/// so another handle may alias the backing storage, violating Rust's rules. Otherwise write elements via `ptr::write`. See `Variant::borrow_slice_mut`.
fn ptr_mut(&mut self, index: usize) -> sys::GDExtensionVariantPtr {
let ptr = self.ptr_mut_or_null(index);
assert!(
Expand Down Expand Up @@ -1374,9 +1395,8 @@ impl<T: Element + ToGodot> From<&[T]> for Array<T> {
// 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();
Expand All @@ -1398,13 +1418,39 @@ impl<T: Element + ToGodot> FromIterator<T> for Array<T> {
/// Extends a `Array` with the contents of an iterator.
impl<T: Element> Extend<T> for Array<T> {
fn extend<I: IntoIterator<Item = T>>(&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);

let mut filled = current_len;
for i in current_len..new_len {
// `size_hint().0` is only a lower bound; a correct iterator may yield fewer than reported, so stop instead of panicking.
let Some(item) = iter.next() else { break };

let elem_ptr: *mut Variant = self.ptr_mut(i).cast::<Variant>();

// SAFETY:
// * `i` is in bounds after `resize_inner()`.
// * Assignment via `=` (not `ptr::write`) drops the default that `resize_inner` placed there -> no leak.
// * Single-element place assignment avoids a multi-slot `&mut [Variant]`, unsound here since `Array` is refcounted (see `ptr_mut`).
unsafe { *elem_ptr = item.to_variant() };
filled = i + 1;
}

// Iterator under-delivered relative to its hint; drop the default-filled trailing slots so they don't leak into the array.
if filled < new_len {
self.resize_inner(filled);
}
}

// Push any remaining elements (for inexact-size iterators, or when lower == 0).
for item in iter {
self.push(meta::owned_into_arg(item));
}
}
Expand Down
Loading
Loading