Fix Issues with Instructions Retaining Objects (#553)

* Add duplicate object test

* Fix duplicate object issues

rend3-test/tests/object.rs

@@ -1,11 +1,69 @@
 use anyhow::Context;
-use glam::{Mat4, Vec3, Vec4};
+use glam::{Mat4, Quat, Vec3, Vec4};
 use rend3::{
-    types::{Camera, Handedness},
+    types::{Camera, Handedness, ObjectChange},
     util::freelist::FreelistDerivedBuffer,
 };
 use rend3_test::{no_gpu_return, test_attr, FrameRenderSettings, TestRunner, Threshold};
 
+/// Ensure that duplicate_object doesn't retain the object for an extra frame.
+#[test_attr]
+pub async fn deuplicate_object_retain() -> anyhow::Result<()> {
+    let iad = no_gpu_return!(rend3::create_iad(None, None, None, None).await)
+        .context("InstanceAdapterDevice creation failed")?;
+
+    let Ok(runner) = TestRunner::builder()
+        .iad(iad.clone())
+        .handedness(Handedness::Left)
+        .build()
+        .await
+    else {
+        return Ok(());
+    };
+
+    runner.set_camera_data(Camera {
+        projection: rend3::types::CameraProjection::Raw(Mat4::IDENTITY),
+        view: Mat4::IDENTITY,
+    });
+
+    let material = runner.add_unlit_material(Vec4::ONE);
+    let object1 = runner.plane(
+        material,
+        Mat4::from_scale_rotation_translation(Vec3::new(-0.25, 0.25, 0.25), Quat::IDENTITY, Vec3::new(-0.5, 0.0, 0.0)),
+    );
+
+    runner
+        .render_and_compare(
+            FrameRenderSettings::new(),
+            "tests/results/object/duplicate-object-retain-left.png",
+            Threshold::Mean(0.0),
+        )
+        .await?;
+
+    let _object2 = runner.duplicate_object(
+        &object1,
+        ObjectChange {
+            transform: Some(Mat4::from_scale_rotation_translation(
+                Vec3::new(-0.25, 0.25, 0.25),
+                Quat::IDENTITY,
+                Vec3::new(0.5, 0.0, 0.0),
+            )),
+            ..Default::default()
+        },
+    );
+    drop(object1);
+
+    runner
+        .render_and_compare(
+            FrameRenderSettings::new(),
+            "tests/results/object/duplicate-object-retain-right.png",
+            Threshold::Mean(0.0),
+        )
+        .await?;
+
+    Ok(())
+}
+
 /// There was a bug in the culling implementation where the per-material buffer
 /// was never resized to fit the number of objects in the scene once it was initially
 /// created. This manifested as objects above the initial frame count would get all-zero

rend3-types/src/lib.rs

@@ -962,7 +962,7 @@ pub struct Texture {
 #[derive(Debug, Clone)]
 pub struct TextureFromTexture {
     pub label: Option<String>,
-    pub src: Texture2DHandle,
+    pub src: RawTexture2DHandle,
     pub start_mip: u32,
     pub mip_count: Option<NonZeroU32>,
 }

rend3/src/instruction.rs

@@ -3,73 +3,73 @@ use std::{mem, panic::Location};
 use glam::Mat4;
 use parking_lot::Mutex;
 use rend3_types::{
-    trait_supertrait_alias, MaterialHandle, ObjectChange, ObjectHandle, PointLight, PointLightChange, PointLightHandle,
-    RawDirectionalLightHandle, RawGraphDataHandleUntyped, RawMaterialHandle, RawMeshHandle, RawPointLightHandle,
-    RawSkeletonHandle, RawTexture2DHandle, RawTextureCubeHandle, SkeletonHandle, Texture2DHandle, TextureCubeHandle,
-    TextureFromTexture, WasmNotSend, WasmNotSync,
+    trait_supertrait_alias, ObjectChange, PointLight, PointLightChange, RawDirectionalLightHandle,
+    RawGraphDataHandleUntyped, RawMaterialHandle, RawMeshHandle, RawPointLightHandle, RawSkeletonHandle,
+    RawTexture2DHandle, RawTextureCubeHandle, TextureFromTexture, WasmNotSend, WasmNotSync,
 };
 use wgpu::{CommandBuffer, Device};
 
 use crate::{
     managers::{GraphStorage, InternalSkeleton, InternalTexture, MaterialManager, TextureManager},
-    types::{Camera, DirectionalLight, DirectionalLightChange, DirectionalLightHandle, Object, RawObjectHandle},
+    types::{Camera, DirectionalLight, DirectionalLightChange, Object, RawObjectHandle},
     RendererProfile,
 };
 
-trait_supertrait_alias!(pub AddMaterialFillInvoke: FnOnce(&mut MaterialManager, &Device, RendererProfile, &mut TextureManager<crate::types::Texture2DTag>, &MaterialHandle) + WasmNotSend + WasmNotSync);
-trait_supertrait_alias!(pub ChangeMaterialChangeInvoke: FnOnce(&mut MaterialManager, &Device, &TextureManager<crate::types::Texture2DTag>, &MaterialHandle) + WasmNotSend + WasmNotSync);
+trait_supertrait_alias!(pub AddMaterialFillInvoke: FnOnce(&mut MaterialManager, &Device, RendererProfile, &mut TextureManager<crate::types::Texture2DTag>, RawMaterialHandle) + WasmNotSend + WasmNotSync);
+trait_supertrait_alias!(pub ChangeMaterialChangeInvoke: FnOnce(&mut MaterialManager, &Device, &TextureManager<crate::types::Texture2DTag>, RawMaterialHandle) + WasmNotSend + WasmNotSync);
 trait_supertrait_alias!(pub AddGraphDataAddInvoke: FnOnce(&mut GraphStorage) + WasmNotSend);
 
 pub struct Instruction {
     pub kind: InstructionKind,
     pub location: Location<'static>,
 }
 
-#[allow(clippy::type_complexity)]
+// None of these need strong handles to the resources, as any
+// resource deletions will also be instructions, added after the given instruction is added.
 pub enum InstructionKind {
     AddMesh {
         cmd_buf: CommandBuffer,
     },
     AddSkeleton {
-        handle: SkeletonHandle,
+        handle: RawSkeletonHandle,
         // Boxed for size
         skeleton: Box<InternalSkeleton>,
     },
     AddTexture2D {
-        handle: Texture2DHandle,
+        handle: RawTexture2DHandle,
         internal_texture: InternalTexture,
         cmd_buf: Option<CommandBuffer>,
     },
     AddTexture2DFromTexture {
-        handle: Texture2DHandle,
+        handle: RawTexture2DHandle,
         texture: TextureFromTexture,
     },
     AddTextureCube {
-        handle: TextureCubeHandle,
+        handle: RawTextureCubeHandle,
         internal_texture: InternalTexture,
         cmd_buf: Option<CommandBuffer>,
     },
     AddMaterial {
-        handle: MaterialHandle,
+        handle: RawMaterialHandle,
         fill_invoke: Box<dyn AddMaterialFillInvoke>,
     },
     AddObject {
-        handle: ObjectHandle,
+        handle: RawObjectHandle,
         object: Object,
     },
     AddDirectionalLight {
-        handle: DirectionalLightHandle,
+        handle: RawDirectionalLightHandle,
         light: DirectionalLight,
     },
     AddPointLight {
-        handle: PointLightHandle,
+        handle: RawPointLightHandle,
         light: PointLight,
     },
     AddGraphData {
         add_invoke: Box<dyn AddGraphDataAddInvoke>,
     },
     ChangeMaterial {
-        handle: MaterialHandle,
+        handle: RawMaterialHandle,
         change_invoke: Box<dyn ChangeMaterialChangeInvoke>,
     },
     ChangeDirectionalLight {
@@ -122,8 +122,8 @@ pub enum InstructionKind {
         data: Camera,
     },
     DuplicateObject {
-        src_handle: ObjectHandle,
-        dst_handle: ObjectHandle,
+        src_handle: RawObjectHandle,
+        dst_handle: RawObjectHandle,
         change: ObjectChange,
     },
 }

rend3/src/managers/directional.rs

@@ -8,7 +8,7 @@ use wgpu::{
 
 use crate::{
     managers::CameraState,
-    types::{DirectionalLight, DirectionalLightHandle},
+    types::DirectionalLight,
     util::{
         bind_merge::{BindGroupBuilder, BindGroupLayoutBuilder},
         buffer::WrappedPotBuffer,
@@ -81,7 +81,7 @@ impl DirectionalLightManager {
         }
     }
 
-    pub fn add(&mut self, handle: &DirectionalLightHandle, light: DirectionalLight) {
+    pub fn add(&mut self, handle: RawDirectionalLightHandle, light: DirectionalLight) {
         if handle.idx >= self.data.len() {
             self.data.resize_with(handle.idx + 1, || None);
         }

rend3/src/managers/material.rs

@@ -11,7 +11,6 @@ use wgpu::{BindGroup, BindGroupLayout, BindingType, Buffer, BufferBindingType, C
 use crate::{
     managers::{object_add_callback, ObjectAddCallbackArgs, TextureManager},
     profile::ProfileData,
-    types::MaterialHandle,
     util::{
         bind_merge::BindGroupLayoutBuilder, freelist::FreelistDerivedBuffer, math::round_up, scatter_copy::ScatterCopy,
         typedefs::FastHashMap,
@@ -138,7 +137,7 @@ impl MaterialManager {
         device: &Device,
         profile: RendererProfile,
         texture_manager_2d: &mut TextureManager<crate::types::Texture2DTag>,
-        handle: &MaterialHandle,
+        handle: RawMaterialHandle,
         material: M,
     ) {
         let bind_group_index = if profile == RendererProfile::CpuDriven {
@@ -169,17 +168,17 @@ impl MaterialManager {
         });
         drop(data_vec);
 
-        self.handle_to_typeid.insert(**handle, TypeId::of::<M>());
+        self.handle_to_typeid.insert(handle, TypeId::of::<M>());
     }
 
     pub fn update<M: Material>(
         &mut self,
         device: &Device,
         texture_manager_2d: &TextureManager<crate::types::Texture2DTag>,
-        handle: &MaterialHandle,
+        handle: RawMaterialHandle,
         material: M,
     ) {
-        let type_id = self.handle_to_typeid[&**handle];
+        let type_id = self.handle_to_typeid[&handle];
 
         assert_eq!(type_id, TypeId::of::<M>());
 

rend3/src/managers/object.rs

@@ -12,7 +12,7 @@ use wgpu::{Buffer, CommandEncoder, Device};
 use super::SkeletonManager;
 use crate::{
     managers::{InternalMesh, MaterialManager, MeshManager},
-    types::{Object, ObjectHandle},
+    types::Object,
     util::{
         freelist::FreelistDerivedBuffer, frustum::BoundingSphere, iter::ExactSizerIterator, scatter_copy::ScatterCopy,
         typedefs::FastHashMap,
@@ -125,7 +125,7 @@ impl ObjectManager {
     pub fn add(
         &mut self,
         device: &Device,
-        handle: &ObjectHandle,
+        handle: RawObjectHandle,
         object: Object,
         mesh_manager: &MeshManager,
         skeleton_manager: &SkeletonManager,
@@ -151,7 +151,7 @@ impl ObjectManager {
                 manager: self,
                 internal_mesh,
                 skeleton_ranges,
-                handle: **handle,
+                handle,
                 object,
             },
         );
@@ -211,22 +211,22 @@ impl ObjectManager {
     pub fn duplicate_object(
         &mut self,
         device: &Device,
-        src_handle: ObjectHandle,
-        dst_handle: ObjectHandle,
+        src_handle: RawObjectHandle,
+        dst_handle: RawObjectHandle,
         change: ObjectChange,
         mesh_manager: &MeshManager,
         skeleton_manager: &SkeletonManager,
         material_manager: &mut MaterialManager,
     ) {
-        let type_id = self.handle_to_typeid[&*src_handle];
+        let type_id = self.handle_to_typeid[&src_handle];
 
         let archetype = self.archetype.get_mut(&type_id).unwrap();
 
         let dst_obj = (archetype.duplicate_object)(&mut archetype.data_vec, src_handle.idx, change);
 
         self.add(
             device,
-            &dst_handle,
+            dst_handle,
             dst_obj,
             mesh_manager,
             skeleton_manager,

rend3/src/managers/point.rs

@@ -1,6 +1,6 @@
 use encase::{ArrayLength, ShaderType};
 use glam::{Vec3, Vec4};
-use rend3_types::{PointLight, PointLightChange, PointLightHandle, RawPointLightHandle};
+use rend3_types::{PointLight, PointLightChange, RawPointLightHandle};
 use wgpu::{BufferUsages, Device, ShaderStages};
 
 use crate::{
@@ -39,7 +39,7 @@ impl PointLightManager {
         }
     }
 
-    pub fn add(&mut self, handle: &PointLightHandle, light: PointLight) {
+    pub fn add(&mut self, handle: RawPointLightHandle, light: PointLight) {
         if handle.idx >= self.data.len() {
             self.data.resize(handle.idx + 1, None);
         }

rend3/src/managers/skeleton.rs

@@ -3,7 +3,7 @@ use std::ops::Range;
 use arrayvec::ArrayVec;
 use glam::Mat4;
 use rend3_types::{
-    MeshHandle, RawSkeletonHandle, Skeleton, SkeletonHandle, VertexAttributeId, VERTEX_ATTRIBUTE_JOINT_INDICES,
+    MeshHandle, RawSkeletonHandle, Skeleton, VertexAttributeId, VERTEX_ATTRIBUTE_JOINT_INDICES,
     VERTEX_ATTRIBUTE_JOINT_WEIGHTS, VERTEX_ATTRIBUTE_NORMAL, VERTEX_ATTRIBUTE_POSITION, VERTEX_ATTRIBUTE_TANGENT,
 };
 use thiserror::Error;
@@ -139,7 +139,7 @@ impl SkeletonManager {
         })
     }
 
-    pub fn add(&mut self, handle: &SkeletonHandle, internal: InternalSkeleton) {
+    pub fn add(&mut self, handle: RawSkeletonHandle, internal: InternalSkeleton) {
         self.global_joint_count += internal.joint_matrices.len();
 
         if handle.idx >= self.data.len() {

rend3/src/renderer/eval.rs

@@ -37,7 +37,7 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
                         .try_lock()
                         .unwrap()
                         .begin_scope("Add Skeleton", &mut encoder, &renderer.device);
-                    data_core.skeleton_manager.add(&handle, *skeleton);
+                    data_core.skeleton_manager.add(handle, *skeleton);
                     let _ = data_core.profiler.try_lock().unwrap().end_scope(&mut encoder);
                 }
                 InstructionKind::AddTexture2D {
@@ -46,18 +46,18 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
                     cmd_buf,
                 } => {
                     cmd_bufs.extend(cmd_buf);
-                    data_core.d2_texture_manager.fill(*handle, internal_texture);
+                    data_core.d2_texture_manager.fill(handle, internal_texture);
                 }
                 InstructionKind::AddTexture2DFromTexture { handle, texture } => data_core
                     .d2_texture_manager
-                    .fill_from_texture(&renderer.device, &mut encoder, *handle, texture),
+                    .fill_from_texture(&renderer.device, &mut encoder, handle, texture),
                 InstructionKind::AddTextureCube {
                     handle,
                     internal_texture,
                     cmd_buf,
                 } => {
                     cmd_bufs.extend(cmd_buf);
-                    data_core.d2c_texture_manager.fill(*handle, internal_texture);
+                    data_core.d2c_texture_manager.fill(handle, internal_texture);
                 }
                 InstructionKind::AddMaterial { handle, fill_invoke } => {
                     profiling::scope!("Add Material");
@@ -66,7 +66,7 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
                         &renderer.device,
                         renderer.profile,
                         &mut data_core.d2_texture_manager,
-                        &handle,
+                        handle,
                     );
                 }
                 InstructionKind::AddGraphData { add_invoke } => {
@@ -79,13 +79,13 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
                         &mut data_core.material_manager,
                         &renderer.device,
                         &mut data_core.d2_texture_manager,
-                        &handle,
+                        handle,
                     );
                 }
                 InstructionKind::AddObject { handle, object } => {
                     data_core.object_manager.add(
                         &renderer.device,
-                        &handle,
+                        handle,
                         object,
                         &renderer.mesh_manager,
                         &data_core.skeleton_manager,
@@ -99,13 +99,13 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
                     data_core.skeleton_manager.set_joint_matrices(handle, joint_matrices);
                 }
                 InstructionKind::AddDirectionalLight { handle, light } => {
-                    data_core.directional_light_manager.add(&handle, light);
+                    data_core.directional_light_manager.add(handle, light);
                 }
                 InstructionKind::ChangeDirectionalLight { handle, change } => {
                     data_core.directional_light_manager.update(handle, change);
                 }
                 InstructionKind::AddPointLight { handle, light } => {
-                    data_core.point_light_manager.add(&handle, light);
+                    data_core.point_light_manager.add(handle, light);
                 }
                 InstructionKind::ChangePointLight { handle, change } => {
                     data_core.point_light_manager.update(handle, change);