Skip to content

Commit

Permalink
Add Root::push() and Index::push() to assist with glTF construction.
Browse files Browse the repository at this point in the history
These methods reduce the amount of boilerplate and possible mistakes
involved in constructing a glTF asset from scratch.

Most use cases are served by `Root::push()`; `Index::push()` is useful
for cases like animation samplers where the vector of objects is not in
the `Root`, or in algorithmically complex situations such as pushing
both within and outside closures, which would produce borrow conflicts
if `&mut Root` was required for everything.

This requires some expansion of the `Get` trait to support writing.
By keeping the old `Get::get()`, this is not a breaking change unless
some dependent *implements* `Get`, but if we choose to make general
breaking changes, it would probably make sense to rename the trait,
and perhaps even declare it an implementation detail, seal it, and hide
its methods.
  • Loading branch information
kpreid committed Dec 18, 2023
1 parent c17732e commit bb53b13
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 25 deletions.
49 changes: 29 additions & 20 deletions gltf-json/src/extensions/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,21 @@ pub struct KhrLightsPunctual {

#[cfg(feature = "KHR_lights_punctual")]
impl crate::root::Get<crate::extensions::scene::khr_lights_punctual::Light> for crate::Root {
fn get(
&self,
id: crate::Index<crate::extensions::scene::khr_lights_punctual::Light>,
) -> Option<&crate::extensions::scene::khr_lights_punctual::Light> {
if let Some(extensions) = self.extensions.as_ref() {
if let Some(khr_lights_punctual) = extensions.khr_lights_punctual.as_ref() {
khr_lights_punctual.lights.get(id.value())
} else {
None
}
} else {
None
}
fn read_by_type(&self) -> &[crate::extensions::scene::khr_lights_punctual::Light] {
self.extensions
.as_ref()
.and_then(|extensions| extensions.khr_lights_punctual.as_ref())
.map(|khr_lights_punctual| khr_lights_punctual.lights.as_slice())
.unwrap_or(&[])
}

fn write_by_type(&mut self) -> &mut Vec<crate::extensions::scene::khr_lights_punctual::Light> {
&mut self
.extensions
.get_or_insert_with(Default::default)
.khr_lights_punctual
.get_or_insert_with(Default::default)
.lights
}
}

Expand All @@ -60,15 +62,22 @@ pub struct KhrMaterialsVariants {

#[cfg(feature = "KHR_materials_variants")]
impl crate::root::Get<crate::extensions::scene::khr_materials_variants::Variant> for crate::Root {
fn get(
&self,
id: crate::Index<crate::extensions::scene::khr_materials_variants::Variant>,
) -> Option<&crate::extensions::scene::khr_materials_variants::Variant> {
fn read_by_type(&self) -> &[crate::extensions::scene::khr_materials_variants::Variant] {
self.extensions
.as_ref()?
.as_ref()
.and_then(|extensions| extensions.khr_materials_variants.as_ref())
.map(|khr_materials_variants| khr_materials_variants.variants.as_slice())
.unwrap_or(&[])
}

fn write_by_type(
&mut self,
) -> &mut Vec<crate::extensions::scene::khr_materials_variants::Variant> {
&mut self
.extensions
.get_or_insert_with(Default::default)
.khr_materials_variants
.as_ref()?
.get_or_insert_with(Default::default)
.variants
.get(id.value())
}
}
97 changes: 92 additions & 5 deletions gltf-json/src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,53 @@ use crate::{
use validation::Validate;

/// Helper trait for retrieving top-level objects by a universal identifier.
///
/// It is not necessary to use this trait directly; see the methods on [`Root`].
pub trait Get<T> {
fn read_by_type(&self) -> &[T];
fn write_by_type(&mut self) -> &mut Vec<T>;

/// Retrieves a single value at the given index.
fn get(&self, id: Index<T>) -> Option<&T>;
#[deprecated = "use `Root::get()` instead"]
fn get(&self, index: Index<T>) -> Option<&T> {
self.read_by_type().get(index.value())
}
}

/// Represents an offset into an array of type `T` owned by the root glTF object.
/// Represents an offset into a vector of type `T` owned by the root glTF object.
///
/// This type may be used with the following functions:
///
/// * [`Root::get()`] to retrieve objects from [`Root`].
/// * [`Root::push()`] to add new objects to [`Root`].
pub struct Index<T>(u32, marker::PhantomData<fn() -> T>);

impl<T> Index<T> {
/// Given a vector of glTF objects, call [`Vec::push()`] to insert it into the vector,
/// then return an [`Index`] for it.
///
/// This allows you to easily obtain [`Index`] values with the correct index and type when
/// creating a glTF asset. Note that for [`Root`], you can call [`Root::push()`] without
/// needing to retrieve the correct vector first.
///
/// # Panics
///
/// Panics if the vector has [`u32::MAX`] or more elements, in which case an `Index` cannot be
/// created.
pub fn push(vec: &mut Vec<T>, value: T) -> Index<T> {
let len = vec.len();
let Ok(index): Result<u32, _> = len.try_into() else {
panic!(
"glTF vector of {ty} has {len} elements, which exceeds the Index limit",
ty = std::any::type_name::<T>(),
);
};

vec.push(value);
Index::new(index)
}
}

/// The root object of a glTF 2.0 asset.
#[derive(Clone, Debug, Default, Deserialize, Serialize, Validate)]
pub struct Root {
Expand Down Expand Up @@ -124,7 +163,27 @@ impl Root {
where
Self: Get<T>,
{
(self as &dyn Get<T>).get(index)
self.read_by_type().get(index.value())
}

/// Insert the given value into this (as via [`Vec::push()`]), then return the [`Index`] to it.
///
/// This allows you to easily obtain [`Index`] values with the correct index and type when
/// creating a glTF asset.
///
/// If you have a mutable borrow conflict when using this method, consider using the more
/// explicit [`Index::push()`] method, passing it only the necessary vector.
///
/// # Panics
///
/// Panics if there are already [`u32::MAX`] or more elements of this type,
/// in which case an `Index` cannot be created.
#[track_caller]
pub fn push<T>(&mut self, value: T) -> Index<T>
where
Self: Get<T>,
{
Index::push(self.write_by_type(), value)
}

/// Deserialize from a JSON string slice.
Expand Down Expand Up @@ -295,8 +354,11 @@ where
macro_rules! impl_get {
($ty:ty, $field:ident) => {
impl<'a> Get<$ty> for Root {
fn get(&self, index: Index<$ty>) -> Option<&$ty> {
self.$field.get(index.value())
fn read_by_type(&self) -> &[$ty] {
&self.$field
}
fn write_by_type(&mut self) -> &mut Vec<$ty> {
&mut self.$field
}
}
};
Expand Down Expand Up @@ -345,4 +407,29 @@ mod tests {
Index<Material>: Send + Sync,
{
}

#[test]
fn index_push() {
let some_object = "hello";

let mut vec = Vec::new();
assert_eq!(Index::push(&mut vec, some_object), Index::new(0));
assert_eq!(Index::push(&mut vec, some_object), Index::new(1));
}

#[test]
fn root_push() {
let some_object = Buffer {
byte_length: validation::USize64(1),
#[cfg(feature = "names")]
name: None,
uri: None,
extensions: None,
extras: Default::default(),
};

let mut root = Root::default();
assert_eq!(root.push(some_object.clone()), Index::new(0));
assert_eq!(root.push(some_object), Index::new(1));
}
}

0 comments on commit bb53b13

Please sign in to comment.