From 6a47e9b7a0395b28fbe9ac17a58fffd2c57e5082 Mon Sep 17 00:00:00 2001 From: Lili Zoey Date: Sat, 11 Mar 2023 17:59:40 +0100 Subject: [PATCH] Implement support for RID Add `RenderingServer` to minimal classes, to be able to test the RID impl against an actual server Add `Engine` to minimal classes, to be able to disable error printing in itests --- godot-codegen/src/lib.rs | 2 + godot-core/src/builtin/mod.rs | 2 + godot-core/src/builtin/others.rs | 1 - godot-core/src/builtin/rid.rs | 95 ++++++++++++++++++ godot-core/src/builtin/variant/impls.rs | 2 +- itest/rust/src/lib.rs | 12 ++- itest/rust/src/rid_test.rs | 127 ++++++++++++++++++++++++ 7 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 godot-core/src/builtin/rid.rs create mode 100644 itest/rust/src/rid_test.rs diff --git a/godot-codegen/src/lib.rs b/godot-codegen/src/lib.rs index 02f110e66..da4bb857c 100644 --- a/godot-codegen/src/lib.rs +++ b/godot-codegen/src/lib.rs @@ -268,6 +268,7 @@ const SELECTED_CLASSES: &[&str] = &[ "CollisionObject2D", "CollisionShape2D", "Control", + "Engine", "FileAccess", "HTTPRequest", "Image", @@ -286,6 +287,7 @@ const SELECTED_CLASSES: &[&str] = &[ "PathFollow2D", "PhysicsBody2D", "RefCounted", + "RenderingServer", "Resource", "ResourceLoader", "RigidBody2D", diff --git a/godot-core/src/builtin/mod.rs b/godot-core/src/builtin/mod.rs index 85180b151..db80ceda9 100644 --- a/godot-core/src/builtin/mod.rs +++ b/godot-core/src/builtin/mod.rs @@ -45,6 +45,7 @@ pub use others::*; pub use packed_array::*; pub use projection::*; pub use quaternion::*; +pub use rid::*; pub use string::*; pub use string_name::*; pub use transform2d::*; @@ -92,6 +93,7 @@ mod others; mod packed_array; mod projection; mod quaternion; +mod rid; mod string; mod string_name; mod transform2d; diff --git a/godot-core/src/builtin/others.rs b/godot-core/src/builtin/others.rs index 8deae9c11..8606dc91e 100644 --- a/godot-core/src/builtin/others.rs +++ b/godot-core/src/builtin/others.rs @@ -17,7 +17,6 @@ impl_builtin_stub!(Rect2, OpaqueRect2); impl_builtin_stub!(Rect2i, OpaqueRect2i); impl_builtin_stub!(Plane, OpaquePlane); impl_builtin_stub!(Aabb, OpaqueAabb); -impl_builtin_stub!(Rid, OpaqueRid); impl_builtin_stub!(Callable, OpaqueCallable); impl_builtin_stub!(Signal, OpaqueSignal); diff --git a/godot-core/src/builtin/rid.rs b/godot-core/src/builtin/rid.rs new file mode 100644 index 000000000..f4ac2d130 --- /dev/null +++ b/godot-core/src/builtin/rid.rs @@ -0,0 +1,95 @@ +/* + * 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::num::NonZeroU64; + +use godot_ffi as sys; +use sys::{ffi_methods, GodotFfi}; + +/// A RID ("resource ID") is an opaque handle that refers to a Godot `Resource`. +/// +/// RIDs do not grant access to the resource itself. Instead, they can be used in lower-level resource APIs +/// such as the [servers]. See also [Godot API docs for `RID`][docs]. +/// +/// RIDs should be largely safe to work with. Certain calls to servers may fail, however doing so will +/// trigger an error from Godot, and will not cause any UB. +/// +/// # Safety Caveat: +/// +/// In Godot 3, RID was not as safe as described here. We believe that this is fixed in Godot 4, but this has +/// not been extensively tested as of yet. Some confirmed UB from Godot 3 does not occur anymore, but if you +/// find anything suspicious or outright UB please open an issue. +/// +/// [servers]: https://docs.godotengine.org/en/stable/tutorials/optimization/using_servers.html +/// [docs]: https://docs.godotengine.org/en/stable/classes/class_rid.html + +// Using normal rust repr to take advantage advantage of the nullable pointer optimization. As this enum is +// eligible for it, it is also guaranteed to have it. Meaning the layout of this type is identical to `u64`. +// See: https://doc.rust-lang.org/nomicon/ffi.html#the-nullable-pointer-optimization +// Cannot use `#[repr(C)]` as it does not use the nullable pointer optimization. +#[derive(Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd, Debug)] +pub enum Rid { + /// A valid RID may refer to some resource, but is not guaranteed to do so. + Valid(NonZeroU64), + /// An invalid RID will never refer to a resource. Internally it is represented as a 0. + Invalid, +} + +impl Rid { + /// Create a new RID. + #[inline] + pub const fn new(id: u64) -> Self { + match NonZeroU64::new(id) { + Some(id) => Self::Valid(id), + None => Self::Invalid, + } + } + + /// Convert this RID into a [`u64`]. Returns 0 if it is invalid. + /// + /// _Godot equivalent: `Rid.get_id()`_ + #[inline] + pub const fn to_u64(self) -> u64 { + match self { + Rid::Valid(id) => id.get(), + Rid::Invalid => 0, + } + } + + /// Convert this RID into a [`u64`] if it is valid. Otherwise return None. + #[inline] + pub const fn to_valid_u64(self) -> Option { + match self { + Rid::Valid(id) => Some(id.get()), + Rid::Invalid => None, + } + } + + /// Convert this RID into a [`NonZeroU64`]. + #[inline] + pub const fn to_non_zero_u64(self) -> Option { + match self { + Rid::Valid(id) => Some(id), + Rid::Invalid => None, + } + } + + /// Returns `true` if this is a valid RID. + #[inline] + pub const fn is_valid(&self) -> bool { + matches!(self, Rid::Valid(_)) + } + + /// Returns `true` if this is an invalid RID. + #[inline] + pub const fn is_invalid(&self) -> bool { + matches!(self, Rid::Invalid) + } +} + +impl GodotFfi for Rid { + ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } +} diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index bd36ff601..177098d21 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -159,7 +159,6 @@ mod impls { impl_variant_metadata!(Rect2i, /* rect2i_to_variant, rect2i_from_variant, */ Rect2i); impl_variant_metadata!(Plane, /* plane_to_variant, plane_from_variant, */ Plane); impl_variant_metadata!(Aabb, /* aabb_to_variant, aabb_from_variant, */ Aabb); - impl_variant_metadata!(Rid, /* rid_to_variant, rid_from_variant, */ Rid); impl_variant_metadata!(Callable, /* callable_to_variant, callable_from_variant, */ Callable); impl_variant_metadata!(Signal, /* signal_to_variant, signal_from_variant, */ Signal); impl_variant_traits!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant, PackedByteArray); @@ -172,6 +171,7 @@ mod impls { impl_variant_traits!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant, PackedVector3Array); impl_variant_traits!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant, PackedColorArray); impl_variant_traits!(Projection, projection_to_variant, projection_from_variant, Projection); + impl_variant_traits!(Rid, rid_to_variant, rid_from_variant, Rid); impl_variant_traits!(Transform2D, transform_2d_to_variant, transform_2d_from_variant, Transform2D); impl_variant_traits!(Transform3D, transform_3d_to_variant, transform_3d_from_variant, Transform3D); impl_variant_traits!(Dictionary, dictionary_to_variant, dictionary_from_variant, Dictionary); diff --git a/itest/rust/src/lib.rs b/itest/rust/src/lib.rs index 9ba473312..d6c47e9f8 100644 --- a/itest/rust/src/lib.rs +++ b/itest/rust/src/lib.rs @@ -4,7 +4,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use godot::engine::Node; +use godot::engine::{Engine, Node}; use godot::init::{gdextension, ExtensionLibrary}; use godot::obj::Gd; use godot::sys; @@ -23,6 +23,7 @@ mod node_test; mod object_test; mod packed_array_test; mod quaternion_test; +mod rid_test; mod singleton_test; mod string_test; mod transform2d_test; @@ -55,6 +56,15 @@ pub(crate) fn expect_panic(context: &str, code: impl FnOnce() + std::panic::Unwi ); } +/// Disable printing errors from Godot. Ideally we should catch and handle errors, ensuring they happen when +/// expected. But that isn't possible, so for now we can just disable printing the error to avoid spamming +/// the terminal when tests should error. +pub(crate) fn suppress_godot_print(mut f: impl FnMut()) { + Engine::singleton().set_print_error_messages(false); + f(); + Engine::singleton().set_print_error_messages(true); +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Entry point + #[itest] test registration diff --git a/itest/rust/src/rid_test.rs b/itest/rust/src/rid_test.rs new file mode 100644 index 000000000..99104f160 --- /dev/null +++ b/itest/rust/src/rid_test.rs @@ -0,0 +1,127 @@ +/* + * 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::{collections::HashSet, thread}; + +use godot::{ + engine::RenderingServer, + prelude::{inner::InnerRid, Color, Rid, Vector2}, +}; + +use crate::{itest, suppress_godot_print}; + +#[itest] +fn rid_equiv() { + let invalid: Rid = Rid::Invalid; + let valid: Rid = Rid::new((10 << 32) | 20); + assert!(!InnerRid::from_outer(&invalid).is_valid()); + assert!(InnerRid::from_outer(&valid).is_valid()); + + assert_eq!(InnerRid::from_outer(&invalid).get_id(), 0); + assert_eq!(InnerRid::from_outer(&valid).get_id(), (10 << 32) | 20); +} + +#[itest] +fn canvas_set_parent() { + // This originally caused UB, but still testing it here in case it breaks. + let mut server = RenderingServer::singleton(); + let canvas = server.canvas_create(); + let viewport = server.viewport_create(); + + suppress_godot_print(|| server.canvas_item_set_parent(viewport, canvas)); + suppress_godot_print(|| server.canvas_item_set_parent(viewport, viewport)); + + server.free_rid(canvas); + server.free_rid(viewport); +} + +#[itest] +fn multi_thread_test() { + let threads = (0..10) + .map(|_| { + thread::spawn(|| { + let mut server = RenderingServer::singleton(); + (0..1000).map(|_| server.canvas_item_create()).collect() + }) + }) + .collect::>(); + + let mut rids: Vec = vec![]; + + for thread in threads.into_iter() { + rids.append(&mut thread.join().unwrap()); + } + + let set = rids.iter().cloned().collect::>(); + assert_eq!(set.len(), rids.len()); + + let mut server = RenderingServer::singleton(); + + for rid in rids.iter() { + server.canvas_item_add_circle(*rid, Vector2::ZERO, 1.0, Color::from_rgb(1.0, 0.0, 0.0)); + } + + for rid in rids.iter() { + server.free_rid(*rid); + } +} + +/// Check that godot does not crash upon receiving various RIDs that may be edge cases. As it could do in Godot 3. +#[itest] +fn strange_rids() { + let mut server = RenderingServer::singleton(); + let mut rids: Vec = vec![ + // Invalid RID. + 0, + // Normal RID, should work without issue. + 1, + 10, + // Testing the boundaries of various ints. + u8::MAX as u64, + u16::MAX as u64, + u32::MAX as u64, + u64::MAX, + i8::MIN as u64, + i8::MAX as u64, + i16::MIN as u64, + i16::MAX as u64, + i32::MIN as u64, + i32::MAX as u64, + i64::MIN as u64, + i64::MAX as u64, + // Biggest RIDs possible in Godot (ignoring local indices). + 0xFFFFFFFF << 32, + 0x7FFFFFFF << 32, + // Godot's servers treats RIDs as two u32s, so testing what happens round the region where + // one u32 overflows into the next. + u32::MAX as u64 + 1, + u32::MAX as u64 + 2, + u32::MAX as u64 - 1, + u32::MAX as u64 - 2, + // A couple random RIDs. + 1234567891011121314, + 14930753991246632225, + 8079365198791785081, + 10737267678893224303, + 12442588258967011829, + 4275912429544145425, + ]; + // Checking every number with exactly 2 bits = 1. + // An approximation of exhaustively checking every number. + for i in 0..64 { + for j in 0..63 { + if j >= i { + rids.push((1 << i) | (1 << (j + 1))) + } else { + rids.push((1 << i) | (1 << j)) + } + } + } + + for id in rids.iter() { + suppress_godot_print(|| server.canvas_item_clear(Rid::new(*id))) + } +}