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 all commits
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
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/re_renderer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ glam = { workspace = true, features = ["bytemuck"] }
half = { workspace = true, features = ["bytemuck"] }
itertools = { workspace = true }
macaw.workspace = true
never = '0.1'
Wumpf marked this conversation as resolved.
Show resolved Hide resolved
ordered-float = "3.2"
parking_lot.workspace = true
slotmap = "1.0.6"
Expand Down
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
5 changes: 4 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,10 @@ 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, TextureCreationError, TextureManager2D,
TextureManager2DError,
};

mod resource_manager;
pub use resource_manager::{ResourceHandle, ResourceLifeTime, ResourceManagerError};
87 changes: 64 additions & 23 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,33 @@ impl<'a> Texture2DCreationDesc<'a> {
}
}

// TODO(andreas): Move this to texture pool.
#[derive(thiserror::Error, Debug)]
pub enum TextureCreationError {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea to split it out. we should consider moving this down to the texture pool later :)

#[error("Texture with debug label {0:?} has zero width or height!")]
ZeroSize(DebugLabel),
}

#[derive(thiserror::Error, Debug)]
pub enum TextureManager2DError<DataCreationError> {
/// Something went wrong when creating the GPU texture.
#[error(transparent)]
TextureCreation(#[from] TextureCreationError),

/// Something went wrong in a user-callback.
#[error(transparent)]
DataCreation(DataCreationError),
}

impl From<TextureManager2DError<never::Never>> for TextureCreationError {
fn from(err: TextureManager2DError<never::Never>) -> Self {
match err {
TextureManager2DError::TextureCreation(texture_creation) => texture_creation,
TextureManager2DError::DataCreation(never) => match never {},
}
}
}

/// Texture manager for 2D textures.
///
/// The scope is intentionally limited to particular kinds of textures that currently
Expand Down Expand Up @@ -146,7 +173,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 +204,7 @@ impl TextureManager2D {
&mut self,
texture_pool: &mut GpuTexturePool,
creation_desc: &Texture2DCreationDesc<'_>,
) -> GpuTexture2D {
) -> Result<GpuTexture2D, TextureCreationError> {
// 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,39 +231,47 @@ 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, TextureCreationError> {
self.get_or_create_with(key, texture_pool, || texture_desc)
}

/// Creates a new 2D texture resource and schedules data upload to the GPU if a texture
/// wasn't already created using the same key.
pub fn get_or_create_with<'a, Err>(
pub fn get_or_create_with<'a>(
&mut self,
key: u64,
texture_pool: &mut GpuTexturePool,
create_texture_desc: impl FnOnce() -> Texture2DCreationDesc<'a>,
) -> Result<GpuTexture2D, TextureCreationError> {
self.get_or_try_create_with(key, texture_pool, || -> Result<_, never::Never> {
Ok(create_texture_desc())
})
.map_err(|err| err.into())
}

/// Creates a new 2D texture resource and schedules data upload to the GPU if a texture
/// wasn't already created using the same key.
pub fn get_or_try_create_with<'a, Err: std::fmt::Display>(
&mut self,
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
}
std::collections::hash_map::Entry::Vacant(entry) => {
// Run potentially expensive texture creation code:
let tex_creation_desc = try_create_texture_desc()?;
entry
.insert(Self::create_and_upload_texture(
&self.device,
&self.queue,
texture_pool,
&tex_creation_desc,
))
.clone()
let tex_creation_desc = try_create_texture_desc()
.map_err(|err| TextureManager2DError::DataCreation(err))?;
let texture = Self::create_and_upload_texture(
&self.device,
&self.queue,
texture_pool,
&tex_creation_desc,
)?;
entry.insert(texture).clone()
}
};

Expand Down Expand Up @@ -278,8 +314,13 @@ impl TextureManager2D {
queue: &wgpu::Queue,
texture_pool: &mut GpuTexturePool,
creation_desc: &Texture2DCreationDesc<'_>,
) -> GpuTexture2D {
) -> Result<GpuTexture2D, TextureCreationError> {
crate::profile_function!();

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

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

// TODO(andreas): mipmap generation

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

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

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

Expand Down Expand Up @@ -52,12 +54,12 @@ pub fn viewport_resolution_in_pixels(clip_rect: egui::Rect, pixels_from_point: f
[resolution.x as u32, resolution.y as u32]
}

pub fn try_get_or_create_texture<'a, Err>(
pub fn try_get_or_create_texture<'a, Err: std::fmt::Display>(
render_ctx: &mut RenderContext,
texture_key: u64,
try_create_texture_desc: impl FnOnce() -> Result<Texture2DCreationDesc<'a>, Err>,
) -> Result<GpuTexture2D, Err> {
render_ctx.texture_manager_2d.get_or_create_with(
) -> Result<GpuTexture2D, TextureManager2DError<Err>> {
render_ctx.texture_manager_2d.get_or_try_create_with(
texture_key,
&mut render_ctx.gpu_resources.textures,
try_create_texture_desc,
Expand All @@ -68,17 +70,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, TextureCreationError> {
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 {},
}
create_texture_desc,
)
}

/// Render a `re_render` view using the given clip rectangle.
Expand Down
Loading