Skip to content

Commit

Permalink
Require FromApp to be implemented for all glob types (#317)
Browse files Browse the repository at this point in the history
* Remove `Glob::new`
* Implement `FromApp` for `Glob<T: FromApp>`
* Remove all debugging labels
  • Loading branch information
Nicolas-Ferre authored Jul 21, 2024
1 parent 844d678 commit c419122
Showing 57 changed files with 419 additions and 458 deletions.
21 changes: 14 additions & 7 deletions crates/modor/src/globals.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(clippy::non_canonical_partial_ord_impl)] // warnings caused by Derivative

use crate::{App, Node, RootNode, RootNodeHandle, Visit};
use crate::{App, FromApp, Node, RootNode, RootNodeHandle, Visit};
use derivative::Derivative;
use log::error;
use std::iter::Flatten;
@@ -18,7 +18,8 @@ use std::sync::{Arc, Mutex};
/// # use modor::*;
/// #
/// fn create_glob(app: &mut App) -> Glob<&'static str> {
/// let glob = Glob::new(app, "shared value");
/// let glob = Glob::from_app(app);
/// *glob.get_mut(app) = "shared value";
/// assert_eq!(glob.get(app), &"shared value");
/// glob
/// }
@@ -37,12 +38,12 @@ pub struct Glob<T> {
phantom: PhantomData<fn(T)>,
}

impl<T> Glob<T>
impl<T> FromApp for Glob<T>
where
T: 'static,
T: FromApp,
{
/// Creates a new shared `value`.
pub fn new(app: &mut App, value: T) -> Self {
fn from_app(app: &mut App) -> Self {
let value = T::from_app(app);
let globals = app.handle::<Globals<T>>();
Self {
ref_: GlobRef {
@@ -53,7 +54,12 @@ where
phantom: PhantomData,
}
}
}

impl<T> Glob<T>
where
T: 'static,
{
/// Returns the unique index of the shared value.
///
/// Note that in case the [`Glob<T>`] and all associated [`GlobRef<T>`]s are dropped, this index
@@ -93,7 +99,8 @@ impl<T> AsRef<GlobRef<T>> for Glob<T> {
/// # use modor::*;
/// #
/// fn create_glob_ref(app: &mut App) -> GlobRef<&'static str> {
/// let glob = Glob::new(app, "shared value");
/// let glob = Glob::from_app(app);
/// *glob.get_mut(app) = "shared_value";
/// let ref_ = glob.as_ref().clone();
/// assert_eq!(ref_.get(app), &"shared value");
/// ref_
61 changes: 35 additions & 26 deletions crates/modor/tests/integration/globals.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use log::Level;
use modor::{App, Glob, Globals};
use modor::{App, FromApp, Glob, Globals};
use modor_derive::{Node, RootNode, Visit};

#[modor::test]
fn create_glob() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::from_app(&mut app);
let glob2 = Glob::from_app(&mut app);
*glob1.get_mut(&mut app) = "a";
*glob2.get_mut(&mut app) = "b";
assert_eq!(glob1.index(), 0);
assert_eq!(glob2.index(), 1);
assert_eq!(glob1.get(&app), &"a");
@@ -19,8 +21,10 @@ fn create_glob() {
#[modor::test]
fn create_glob_ref() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::from_app(&mut app);
let glob2 = Glob::from_app(&mut app);
*glob1.get_mut(&mut app) = "a";
*glob2.get_mut(&mut app) = "b";
let glob1_ref = glob1.as_ref().clone();
let glob2_ref = glob2.as_ref().clone();
assert_eq!(glob1_ref.index(), 0);
@@ -35,62 +39,65 @@ fn create_glob_ref() {
#[modor::test]
fn recreate_glob_without_ref_before_update() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let _glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::from_app(&mut app);
let _glob2 = Glob::<&str>::from_app(&mut app);
*glob1.get_mut(&mut app) = "a";
drop(glob1);
let glob3 = Glob::new(&mut app, "c");
let glob3 = Glob::<&str>::from_app(&mut app);
assert_eq!(glob3.index(), 2);
let glob4 = Glob::new(&mut app, "d");
let glob4 = Glob::<&str>::from_app(&mut app);
assert_eq!(glob4.index(), 3);
}

#[modor::test]
fn recreate_glob_without_ref_after_update() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let _glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::<&str>::from_app(&mut app);
let _glob2 = Glob::<&str>::from_app(&mut app);
drop(glob1);
app.update();
let glob3 = Glob::new(&mut app, "c");
let glob3 = Glob::<&str>::from_app(&mut app);
assert_eq!(glob3.index(), 2);
app.update();
assert_eq!(Glob::new(&mut app, "d").index(), 0);
assert_eq!(Glob::new(&mut app, "e").index(), 3);
assert_eq!(Glob::<&str>::from_app(&mut app).index(), 0);
assert_eq!(Glob::<&str>::from_app(&mut app).index(), 3);
}

#[modor::test]
fn recreate_glob_with_not_dropped_ref_after_update() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let _glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::<&str>::from_app(&mut app);
let _glob2 = Glob::<&str>::from_app(&mut app);
let _glob1_ref = glob1.as_ref().clone();
drop(glob1);
app.update();
let glob3 = Glob::new(&mut app, "c");
let glob3 = Glob::<&str>::from_app(&mut app);
assert_eq!(glob3.index(), 2);
}

#[modor::test]
fn recreate_glob_with_dropped_ref_after_update() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let _glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::<&str>::from_app(&mut app);
let _glob2 = Glob::<&str>::from_app(&mut app);
let glob1_ref = glob1.as_ref().clone();
drop(glob1);
drop(glob1_ref);
app.update();
let glob3 = Glob::new(&mut app, "c");
let glob3 = Glob::<&str>::from_app(&mut app);
assert_eq!(glob3.index(), 2);
app.update();
assert_eq!(Glob::new(&mut app, "d").index(), 0);
assert_eq!(Glob::new(&mut app, "e").index(), 3);
assert_eq!(Glob::<&str>::from_app(&mut app).index(), 0);
assert_eq!(Glob::<&str>::from_app(&mut app).index(), 3);
}

#[modor::test]
fn access_all_globals() {
let mut app = App::new::<Root>(Level::Info);
let _glob1 = Glob::new(&mut app, "a");
let _glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::from_app(&mut app);
let glob2 = Glob::from_app(&mut app);
*glob1.get_mut(&mut app) = "a";
*glob2.get_mut(&mut app) = "b";
let globals = app.get_mut::<Globals<&str>>();
assert!(globals.deleted_items().is_empty());
assert_eq!(globals.get(0), Some(&"a"));
@@ -105,8 +112,10 @@ fn access_all_globals() {
#[modor::test]
fn access_all_globals_after_value_dropped() {
let mut app = App::new::<Root>(Level::Info);
let glob1 = Glob::new(&mut app, "a");
let _glob2 = Glob::new(&mut app, "b");
let glob1 = Glob::from_app(&mut app);
let glob2 = Glob::from_app(&mut app);
*glob1.get_mut(&mut app) = "a";
*glob2.get_mut(&mut app) = "b";
drop(glob1);
app.update();
let globals = app.get_mut::<Globals<&str>>();
4 changes: 2 additions & 2 deletions crates/modor_graphics/src/animation.rs
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ use std::time::Duration;
///
/// impl AnimatedSprite {
/// fn new(app: &mut App) -> Self {
/// let texture = Texture::new(app, "spritesheet").load_from_path(app, "spritesheet.png");
/// let texture = Texture::new(app).load_from_path(app, "spritesheet.png");
/// let animation_parts = vec![
/// TexturePart::new(0, 0),
/// TexturePart::new(1, 0),
@@ -45,7 +45,7 @@ use std::time::Duration;
/// TexturePart::new(2, 1),
/// ];
/// Self {
/// sprite: Sprite2D::new(app, "sprite")
/// sprite: Sprite2D::new(app)
/// .with_material(|m| m.texture = texture.glob().clone()),
/// animation: TextureAnimation::new(3, 2)
/// .with_fps(5)
23 changes: 18 additions & 5 deletions crates/modor_graphics/src/buffer.rs
Original file line number Diff line number Diff line change
@@ -26,17 +26,21 @@ where
usages: BufferUsages,
label: impl Into<String>,
) -> Self {
let data = bytemuck::cast_slice(data);
let cast_data = Self::cast_data(data);
let label = label.into();
Self {
inner: Self::create_buffer(gpu, data, usages, &label),
inner: Self::create_buffer(gpu, cast_data, usages, &label),
len: data.len(),
usages,
label,
phantom: PhantomData,
}
}

pub(crate) fn len(&self) -> usize {
self.len
}

pub(crate) fn resource(&self) -> BindingResource<'_> {
self.inner.as_entire_binding()
}
@@ -46,12 +50,12 @@ where
}

pub(crate) fn update(&mut self, gpu: &Gpu, data: &[T]) {
let data = bytemuck::cast_slice(data);
let cast_data = Self::cast_data(data);
if self.len < data.len() {
self.inner = Self::create_buffer(gpu, data, self.usages, &self.label);
self.inner = Self::create_buffer(gpu, cast_data, self.usages, &self.label);
self.len = data.len();
} else {
gpu.queue.write_buffer(&self.inner, 0, data);
gpu.queue.write_buffer(&self.inner, 0, cast_data);
}
}

@@ -62,6 +66,15 @@ where
usage: usages,
})
}

#[allow(clippy::cast_possible_truncation)]
fn cast_data(data: &[T]) -> &[u8] {
if data.is_empty() {
&[0; wgpu::COPY_BUFFER_ALIGNMENT as usize]
} else {
bytemuck::cast_slice(data)
}
}
}

#[derive(Debug)]
27 changes: 11 additions & 16 deletions crates/modor_graphics/src/camera.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ use crate::buffer::{Buffer, BufferBindGroup};
use crate::gpu::{Gpu, GpuManager};
use crate::{Size, TargetGlob};
use fxhash::FxHashMap;
use modor::{App, Builder, Glob, GlobRef, Node, Visit};
use modor::{App, Builder, FromApp, Glob, GlobRef, Node, Visit};
use modor_physics::modor_math::{Mat4, Quat, Vec2, Vec3};
use std::collections::hash_map::Entry;
use wgpu::{BindGroup, BufferUsages};
@@ -30,7 +30,7 @@ use wgpu::{BindGroup, BufferUsages};
/// fn new(app: &mut App) -> Self {
/// let camera = app.get_mut::<MovingCamera>().camera.glob().clone();
/// Self {
/// sprite: Sprite2D::new(app, "object")
/// sprite: Sprite2D::new(app)
/// .with_model(|m| m.size = Vec2::ONE * 0.2)
/// .with_model(|m| m.camera = camera)
/// }
@@ -52,7 +52,7 @@ use wgpu::{BindGroup, BufferUsages};
/// fn on_create(app: &mut App) -> Self {
/// let target = app.get_mut::<Window>().target.glob().clone();
/// Self {
/// camera: Camera2D::new(app, "moving", vec![target])
/// camera: Camera2D::new(app, vec![target])
/// .with_size(Vec2::ONE * 0.5) // zoom x2
/// }
/// }
@@ -76,7 +76,6 @@ pub struct Camera2D {
#[builder(form(closure))]
pub targets: Vec<GlobRef<TargetGlob>>,
glob: Glob<Camera2DGlob>,
label: String,
}

impl Node for Camera2D {
@@ -90,23 +89,20 @@ impl Node for Camera2D {
glob.register_targets(&self.targets);
for (target_index, target_size) in target_sizes {
let transform = self.gpu_transform(target_size.into());
glob.update_target(&gpu, target_index, transform, &self.label);
glob.update_target(&gpu, target_index, transform);
}
}
}

impl Camera2D {
/// Creates a new camera.
///
/// The `label` is used to identity the camera in logs.
pub fn new(app: &mut App, label: impl Into<String>, targets: Vec<GlobRef<TargetGlob>>) -> Self {
pub fn new(app: &mut App, targets: Vec<GlobRef<TargetGlob>>) -> Self {
Self {
position: Vec2::ZERO,
size: Vec2::ONE,
rotation: 0.,
targets,
glob: Glob::new(app, Camera2DGlob::default()),
label: label.into(),
glob: Glob::from_app(app),
}
}

@@ -183,11 +179,11 @@ impl Camera2DGlob {
self.targets = targets.into();
}

fn update_target(&mut self, gpu: &Gpu, target_index: usize, transform: Mat4, label: &str) {
fn update_target(&mut self, gpu: &Gpu, target_index: usize, transform: Mat4) {
match self.target_uniforms.entry(target_index) {
Entry::Occupied(mut entry) => entry.get_mut().update(gpu, transform),
Entry::Vacant(entry) => {
entry.insert(CameraUniform::new(gpu, transform, label));
entry.insert(CameraUniform::new(gpu, transform));
}
}
}
@@ -212,21 +208,20 @@ struct CameraUniform {
impl CameraUniform {
const BINDING: u32 = 0;

fn new(gpu: &Gpu, transform: Mat4, label: &str) -> Self {
let label = format!("camera_2d:{label}");
fn new(gpu: &Gpu, transform: Mat4) -> Self {
let buffer = Buffer::new(
gpu,
&[transform.to_array()],
BufferUsages::UNIFORM | BufferUsages::COPY_DST,
&label,
"camera_2d",
);
Self {
bind_group: BufferBindGroup::uniform(
gpu,
&buffer,
Self::BINDING,
&gpu.camera_bind_group_layout,
&label,
"camera_2d",
),
buffer,
transform,
Loading

0 comments on commit c419122

Please sign in to comment.