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.

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

Fix crash for missing class ids causing zero sized texture #1947

Merged
merged 8 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
23 changes: 13 additions & 10 deletions crates/re_renderer/examples/2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,19 @@ impl framework::Example for Render2D {
);
}

let rerun_logo_texture = re_ctx.texture_manager_2d.create(
&mut re_ctx.gpu_resources.textures,
&Texture2DCreationDesc {
label: "rerun logo".into(),
data: image_data.into(),
format: wgpu::TextureFormat::Rgba8UnormSrgb,
width: rerun_logo.width(),
height: rerun_logo.height(),
},
);
let rerun_logo_texture = re_ctx
.texture_manager_2d
.create(
&mut re_ctx.gpu_resources.textures,
&Texture2DCreationDesc {
label: "rerun logo".into(),
data: image_data.into(),
format: wgpu::TextureFormat::Rgba8UnormSrgb,
width: rerun_logo.width(),
height: rerun_logo.height(),
},
)
.expect("Failed to create texture for rerun logo");
emilk marked this conversation as resolved.
Show resolved Hide resolved
Render2D {
rerun_logo_texture,

Expand Down
50 changes: 28 additions & 22 deletions crates/re_renderer/examples/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,17 +408,20 @@ impl DepthTexture {
});

let label = format!("depth texture spiral {dimensions}");
let texture = re_ctx.texture_manager_2d.get_or_create(
hash(&label),
&mut re_ctx.gpu_resources.textures,
Texture2DCreationDesc {
label: label.into(),
data: bytemuck::cast_slice(&data).into(),
format: wgpu::TextureFormat::R32Float,
width: dimensions.x,
height: dimensions.y,
},
);
let texture = re_ctx
.texture_manager_2d
.get_or_create(
hash(&label),
&mut re_ctx.gpu_resources.textures,
Texture2DCreationDesc {
label: label.into(),
data: bytemuck::cast_slice(&data).into(),
format: wgpu::TextureFormat::R32Float,
width: dimensions.x,
height: dimensions.y,
},
)
.expect("Failed to create depth texture.");

Self {
dimensions,
Expand Down Expand Up @@ -448,17 +451,20 @@ impl AlbedoTexture {
});

let label = format!("albedo texture spiral {dimensions}");
let texture = re_ctx.texture_manager_2d.get_or_create(
hash(&label),
&mut re_ctx.gpu_resources.textures,
Texture2DCreationDesc {
label: label.into(),
data: bytemuck::cast_slice(&rgba8).into(),
format: wgpu::TextureFormat::Rgba8UnormSrgb,
width: dimensions.x,
height: dimensions.y,
},
);
let texture = re_ctx
.texture_manager_2d
.get_or_create(
hash(&label),
&mut re_ctx.gpu_resources.textures,
Texture2DCreationDesc {
label: label.into(),
data: bytemuck::cast_slice(&rgba8).into(),
format: wgpu::TextureFormat::Rgba8UnormSrgb,
width: dimensions.x,
height: dimensions.y,
},
)
.expect("Failed to create albedo texture.");

Self {
dimensions,
Expand Down
12 changes: 10 additions & 2 deletions crates/re_renderer/src/importer/gltf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,16 @@ pub fn load_gltf_from_buffer(
};

images_as_textures.push(
ctx.texture_manager_2d
.create(&mut ctx.gpu_resources.textures, &texture),
match ctx
.texture_manager_2d
.create(&mut ctx.gpu_resources.textures, &texture)
{
Ok(texture) => texture,
Err(err) => {
re_log::error!("Failed to create texture: {err:?}");
ctx.texture_manager_2d.white_texture_unorm_handle().clone()
}
},
);
}

Expand Down
4 changes: 3 additions & 1 deletion crates/re_renderer/src/resource_managers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ mod mesh_manager;
pub use mesh_manager::{GpuMeshHandle, MeshManager};

mod texture_manager;
pub use texture_manager::{GpuTexture2D, Texture2DCreationDesc, TextureManager2D};
pub use texture_manager::{
GpuTexture2D, Texture2DCreationDesc, TextureManager2D, TextureManager2DError,
};

mod resource_manager;
pub use resource_manager::{ResourceHandle, ResourceLifeTime, ResourceManagerError};
41 changes: 25 additions & 16 deletions crates/re_renderer/src/resource_managers/texture_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ impl<'a> Texture2DCreationDesc<'a> {
}
}

#[derive(thiserror::Error, Debug)]
pub enum TextureManager2DError<DataCreationError> {
#[error("Texture with debug label {0:?} has zero width or height!")]
ZeroSize(DebugLabel),

#[error(transparent)]
DataCreationError(#[from] DataCreationError),
}

/// Texture manager for 2D textures.
///
/// The scope is intentionally limited to particular kinds of textures that currently
Expand Down Expand Up @@ -135,7 +144,7 @@ impl TextureManager2D {
) -> Self {
crate::profile_function!();

let white_texture_unorm = Self::create_and_upload_texture(
let white_texture_unorm = Self::create_and_upload_texture::<()>(
&device,
&queue,
texture_pool,
Expand All @@ -146,7 +155,8 @@ impl TextureManager2D {
width: 1,
height: 1,
},
);
)
.expect("Failed to create white pixel texture!");

let zeroed_texture_float =
create_zero_texture(texture_pool, &device, wgpu::TextureFormat::Rgba8Unorm);
Expand Down Expand Up @@ -176,7 +186,7 @@ impl TextureManager2D {
&mut self,
texture_pool: &mut GpuTexturePool,
creation_desc: &Texture2DCreationDesc<'_>,
) -> GpuTexture2D {
) -> Result<GpuTexture2D, TextureManager2DError<()>> {
// TODO(andreas): Disabled the warning as we're moving towards using this texture manager for user-logged images.
// However, it's still very much a concern especially once we add mipmapping. Something we need to keep in mind.
//
Expand All @@ -203,14 +213,8 @@ impl TextureManager2D {
key: u64,
texture_pool: &mut GpuTexturePool,
texture_desc: Texture2DCreationDesc<'_>,
) -> GpuTexture2D {
enum Never {}
match self.get_or_create_with(key, texture_pool, || -> Result<_, Never> {
Ok(texture_desc)
}) {
Ok(tex_handle) => tex_handle,
Err(never) => match never {},
}
) -> Result<GpuTexture2D, TextureManager2DError<()>> {
self.get_or_create_with(key, texture_pool, || -> Result<_, ()> { Ok(texture_desc) })
}

/// Creates a new 2D texture resource and schedules data upload to the GPU if a texture
Expand All @@ -220,7 +224,7 @@ impl TextureManager2D {
key: u64,
texture_pool: &mut GpuTexturePool,
try_create_texture_desc: impl FnOnce() -> Result<Texture2DCreationDesc<'a>, Err>,
) -> Result<GpuTexture2D, Err> {
) -> Result<GpuTexture2D, TextureManager2DError<Err>> {
let texture_handle = match self.texture_cache.entry(key) {
std::collections::hash_map::Entry::Occupied(texture_handle) => {
texture_handle.get().clone() // already inserted
Expand All @@ -234,7 +238,7 @@ impl TextureManager2D {
&self.queue,
texture_pool,
&tex_creation_desc,
))
)?)
.clone()
}
};
Expand Down Expand Up @@ -273,13 +277,18 @@ impl TextureManager2D {
&self.zeroed_texture_uint.0
}

fn create_and_upload_texture(
fn create_and_upload_texture<Err>(
device: &wgpu::Device,
queue: &wgpu::Queue,
texture_pool: &mut GpuTexturePool,
creation_desc: &Texture2DCreationDesc<'_>,
) -> GpuTexture2D {
) -> Result<GpuTexture2D, TextureManager2DError<Err>> {
crate::profile_function!();

if creation_desc.width == 0 || creation_desc.height == 0 {
return Err(TextureManager2DError::ZeroSize(creation_desc.label.clone()));
}

let size = wgpu::Extent3d {
width: creation_desc.width,
height: creation_desc.height,
Expand Down Expand Up @@ -329,7 +338,7 @@ impl TextureManager2D {

// TODO(andreas): mipmap generation

GpuTexture2D(texture)
Ok(GpuTexture2D(texture))
}

pub(crate) fn begin_frame(&mut self, _frame_index: u64) {
Expand Down
15 changes: 5 additions & 10 deletions crates/re_viewer/src/gpu_bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use egui::mutex::Mutex;

use re_renderer::{
renderer::{ColormappedTexture, RectangleOptions},
resource_managers::{GpuTexture2D, Texture2DCreationDesc},
resource_managers::{GpuTexture2D, Texture2DCreationDesc, TextureManager2DError},
RenderContext, ViewBuilder,
};

Expand Down Expand Up @@ -56,7 +56,7 @@ pub fn try_get_or_create_texture<'a, Err>(
render_ctx: &mut RenderContext,
texture_key: u64,
try_create_texture_desc: impl FnOnce() -> Result<Texture2DCreationDesc<'a>, Err>,
) -> Result<GpuTexture2D, Err> {
) -> Result<GpuTexture2D, TextureManager2DError<Err>> {
render_ctx.texture_manager_2d.get_or_create_with(
texture_key,
&mut render_ctx.gpu_resources.textures,
Expand All @@ -68,17 +68,12 @@ pub fn get_or_create_texture<'a>(
render_ctx: &mut RenderContext,
texture_key: u64,
create_texture_desc: impl FnOnce() -> Texture2DCreationDesc<'a>,
) -> GpuTexture2D {
enum Never {}
let result: Result<GpuTexture2D, Never> = render_ctx.texture_manager_2d.get_or_create_with(
) -> Result<GpuTexture2D, TextureManager2DError<()>> {
render_ctx.texture_manager_2d.get_or_create_with(
texture_key,
&mut render_ctx.gpu_resources.textures,
|| Ok(create_texture_desc()),
);
match result {
Ok(handle) => handle,
Err(never) => match never {},
}
)
}

/// Render a `re_render` view using the given clip rectangle.
Expand Down
18 changes: 12 additions & 6 deletions crates/re_viewer/src/gpu_bridge/tensor_to_gpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::borrow::Cow;

use bytemuck::{allocation::pod_collect_to_vec, cast_slice, Pod};
use egui::util::hash;
use egui::{util::hash, NumExt};
use wgpu::TextureFormat;

use re_log_types::component_types::{DecodedTensor, Tensor, TensorData};
Expand Down Expand Up @@ -94,7 +94,8 @@ fn color_tensor_to_gpu(
width,
height,
})
})?;
})
.map_err(|e| anyhow::anyhow!("Failed to create texture for color tensor: {e:?}"))?;
emilk marked this conversation as resolved.
Show resolved Hide resolved

let texture_format = texture_handle.format();

Expand Down Expand Up @@ -157,8 +158,10 @@ fn class_id_tensor_to_gpu(

// We pack the colormap into a 2D texture so we don't go over the max texture size.
// We only support u8 and u16 class ids, so 256^2 is the biggest texture we need.
// Note that if max is 0, we still need to generate a row to accomodate the 0 class id
// (also zero sized textures are not allowed)
let colormap_width = 256;
let colormap_height = (max as usize + colormap_width - 1) / colormap_width;
let colormap_height = (max.at_least(1.0) as usize + colormap_width - 1) / colormap_width;
emilk marked this conversation as resolved.
Show resolved Hide resolved

let colormap_texture_handle =
get_or_create_texture(render_ctx, hash(annotations.row_id), || {
Expand All @@ -179,11 +182,13 @@ fn class_id_tensor_to_gpu(
width: colormap_width as u32,
height: colormap_height as u32,
}
});
})
.map_err(|e| anyhow::anyhow!("Failed to create class_id_colormap: {e:?}"))?;
emilk marked this conversation as resolved.
Show resolved Hide resolved

let main_texture_handle = try_get_or_create_texture(render_ctx, hash(tensor.id()), || {
general_texture_creation_desc_from_tensor(debug_name, tensor)
})?;
})
.map_err(|e| anyhow::anyhow!("Failed to create texture for class id tensor: {e:?}"))?;
emilk marked this conversation as resolved.
Show resolved Hide resolved

Ok(ColormappedTexture {
texture: main_texture_handle,
Expand Down Expand Up @@ -212,7 +217,8 @@ fn depth_tensor_to_gpu(

let texture = try_get_or_create_texture(render_ctx, hash(tensor.id()), || {
general_texture_creation_desc_from_tensor(debug_name, tensor)
})?;
})
.map_err(|e| anyhow::anyhow!("Failed to create depth tensor texture: {e:?}"))?;
emilk marked this conversation as resolved.
Show resolved Hide resolved

Ok(ColormappedTexture {
texture,
Expand Down
61 changes: 32 additions & 29 deletions crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,35 +290,38 @@ impl ImagesPart {
// So to not couple this, we use a different key here
let texture_key = egui::util::hash((tensor.id(), "depth_cloud"));
let mut data_f32 = Vec::new();
ctx.render_ctx.texture_manager_2d.get_or_create_with(
texture_key,
&mut ctx.render_ctx.gpu_resources.textures,
|| {
// TODO(andreas/cmc): Ideally we'd upload the u16 data as-is.
// However, R16Unorm is behind a feature flag and Depth16Unorm doesn't work on WebGL (and is awkward as this is a depth buffer format!).
let data = match &tensor.data {
TensorData::U16(data) => {
data_f32.extend(data.as_slice().iter().map(|d| *d as f32));
bytemuck::cast_slice(&data_f32).into()
}
TensorData::F32(data) => bytemuck::cast_slice(data).into(),
_ => {
return Err(format!(
"Tensor datatype {} is not supported for back-projection",
tensor.dtype()
));
}
};

Ok(Texture2DCreationDesc {
label: format!("Depth cloud for {ent_path:?}").into(),
data,
format: wgpu::TextureFormat::R32Float,
width: width as _,
height: height as _,
})
},
)?
ctx.render_ctx
.texture_manager_2d
.get_or_create_with(
texture_key,
&mut ctx.render_ctx.gpu_resources.textures,
|| {
// TODO(andreas/cmc): Ideally we'd upload the u16 data as-is.
// However, R16Unorm is behind a feature flag and Depth16Unorm doesn't work on WebGL (and is awkward as this is a depth buffer format!).
let data = match &tensor.data {
TensorData::U16(data) => {
data_f32.extend(data.as_slice().iter().map(|d| *d as f32));
bytemuck::cast_slice(&data_f32).into()
}
TensorData::F32(data) => bytemuck::cast_slice(data).into(),
_ => {
return Err(format!(
"Tensor datatype {} is not supported for back-projection",
tensor.dtype()
));
}
};

Ok(Texture2DCreationDesc {
label: format!("Depth cloud for {ent_path:?}").into(),
data,
format: wgpu::TextureFormat::R32Float,
width: width as _,
height: height as _,
})
},
)
.map_err(|e| format!("Failed to create depth cloud texture: {e:?}"))?
emilk marked this conversation as resolved.
Show resolved Hide resolved
};

let depth_from_world_scale = *properties.depth_from_world_scale.get();
Expand Down
Loading