Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

renderer: fix the groupby logic in mesh instancing #1657

Merged
merged 2 commits into from
Mar 21, 2023
Merged

renderer: fix the groupby logic in mesh instancing #1657

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6206945
fix the groupby logic in mesh instancing
teh-cmc Mar 21, 2023
d21431a
allocate the right thing...
teh-cmc Mar 21, 2023
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
22 changes: 16 additions & 6 deletions crates/re_renderer/src/renderer/mesh_renderer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@

use std::sync::Arc;

use itertools::Itertools as _;
use ahash::{HashMap, HashMapExt};
use smallvec::smallvec;

use crate::{
include_file,
mesh::{gpu_data::MaterialUniformBuffer, mesh_vertices, GpuMesh, Mesh},
renderer::OutlineMaskProcessor,
resource_managers::GpuMeshHandle,
resource_managers::{GpuMeshHandle, ResourceHandle},
view_builder::ViewBuilder,
wgpu_resources::{
BindGroupLayoutDesc, BufferDesc, GpuBindGroupLayoutHandle, GpuBuffer,
Expand Down Expand Up @@ -175,6 +175,16 @@ impl MeshDrawData {
},
);

let mut instances_by_mesh: HashMap<_, Vec<_>> = HashMap::with_capacity(instances.len());
teh-cmc marked this conversation as resolved.
Show resolved Hide resolved
for instance in instances {
if !matches!(instance.gpu_mesh, ResourceHandle::Invalid) {
instances_by_mesh
.entry(&instance.gpu_mesh)
.or_default()
.push(instance);
}
}

let mut batches = Vec::new();
{
let mut instance_buffer_staging = ctx
Expand All @@ -188,13 +198,12 @@ impl MeshDrawData {

let mesh_manager = ctx.mesh_manager.read();
let mut num_processed_instances = 0;
// TODO(#1530) This grouping doesn't seem to do its job correctly. We're not actually batching correctly right now in all cases.
for (mesh, instances) in &instances.iter().group_by(|instance| &instance.gpu_mesh) {
for (mesh, mut instances) in instances_by_mesh {
let mut count = 0;
let mut count_with_outlines = 0;

// Put all instances with outlines at the start of the instance buffer range.
let instances = instances.sorted_by(|a, b| {
instances.sort_by(|a, b| {
a.outline_mask_ids
.is_none()
.cmp(&b.outline_mask_ids.is_none())
Expand Down Expand Up @@ -232,7 +241,8 @@ impl MeshDrawData {
}
num_processed_instances += count;

// We resolve the meshes here already, so the actual draw call doesn't need to know about the MeshManager.
// We resolve the meshes here already, so the actual draw call doesn't need to
// know about the MeshManager.
let mesh = mesh_manager.get(mesh)?;
batches.push(MeshBatch {
mesh: mesh.clone(),
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/resource_managers/resource_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use slotmap::{Key, SlotMap};
use crate::wgpu_resources::PoolError;

/// Handle to a resource that is stored in a resource manager.
#[derive(Debug, PartialEq, Eq, Clone)]
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum ResourceHandle<InnerHandle: slotmap::Key> {
/// Resource handle that keeps the resource alive as long as there are handles.
///
Expand Down