Skip to content

Commit

Permalink
Fix error when logging segmentation image (#6449)
Browse files Browse the repository at this point in the history
* Fixes #6443
* the problem was that we use row ids for hashing texture manager
entries. Instead, we need to use a combination of texture meaning + row
id, because the same row may give rise to several textures!
* Fixes this also for any other combination of textures on the same row
(e.g. depth + regular)
* Fixes bad error message display

* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6449?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6449?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6449)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
Wumpf authored and abey79 committed May 29, 2024
1 parent bdbd6e2 commit 4ca07b7
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 76 deletions.
58 changes: 34 additions & 24 deletions crates/re_data_ui/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use re_renderer::renderer::ColormappedTexture;
use re_types::components::{ClassId, DepthMeter};
use re_types::datatypes::{TensorBuffer, TensorData, TensorDimension};
use re_types::tensor_data::{DecodedTensor, TensorDataMeaning, TensorElement};
use re_ui::ReUi;
use re_viewer_context::{
gpu_bridge, Annotations, TensorDecodeCache, TensorStats, TensorStatsCache, UiLayout,
ViewerContext,
Expand Down Expand Up @@ -146,26 +145,27 @@ pub fn tensor_ui(
|ui| {
ui.set_min_size(preview_size);

show_image_preview(
match show_image_preview(
ctx.render_ctx,
ctx.re_ui,
ui,
texture.clone(),
&debug_name,
preview_size,
)
.on_hover_ui(|ui| {
// Show larger image on hover.
let preview_size = Vec2::splat(400.0);
show_image_preview(
ctx.render_ctx,
ctx.re_ui,
ui,
texture.clone(),
&debug_name,
preview_size,
);
});
) {
Ok(response) => response.on_hover_ui(|ui| {
// Show larger image on hover.
let preview_size = Vec2::splat(400.0);
show_image_preview(
ctx.render_ctx,
ui,
texture.clone(),
&debug_name,
preview_size,
)
.ok();
}),
Err((response, err)) => response.on_hover_text(err.to_string()),
}
},
);
}
Expand Down Expand Up @@ -215,14 +215,16 @@ pub fn tensor_ui(
.available_size()
.min(texture_size(texture))
.min(egui::vec2(150.0, 300.0));
let response = show_image_preview(
let response = match show_image_preview(
ctx.render_ctx,
ctx.re_ui,
ui,
texture.clone(),
&debug_name,
preview_size,
);
) {
Ok(response) => response,
Err((response, err)) => response.on_hover_text(err.to_string()),
};

if let Some(pointer_pos) = ui.ctx().pointer_latest_pos() {
let image_rect = response.rect;
Expand Down Expand Up @@ -279,14 +281,15 @@ fn texture_size(colormapped_texture: &ColormappedTexture) -> Vec2 {
///
/// Extremely small images will be stretched on their thin axis to make them visible.
/// This does not preserve aspect ratio, but we only stretch it to a very thin size, so it is fine.
///
/// Returns error if the image could not be rendered.
fn show_image_preview(
render_ctx: &re_renderer::RenderContext,
re_ui: &ReUi,
ui: &mut egui::Ui,
colormapped_texture: ColormappedTexture,
debug_name: &str,
desired_size: egui::Vec2,
) -> egui::Response {
) -> Result<egui::Response, (egui::Response, anyhow::Error)> {
const MIN_SIZE: f32 = 2.0;

let texture_size = texture_size(&colormapped_texture);
Expand All @@ -309,10 +312,17 @@ fn show_image_preview(
egui::TextureOptions::LINEAR,
debug_name,
) {
let label_response = ui.label(re_ui.error_text(err.to_string()));
response.union(label_response)
let color = ui.visuals().error_fg_color;
painter.text(
response.rect.left_top(),
egui::Align2::LEFT_TOP,
"🚫",
egui::FontId::default(),
color,
);
Err((response, err))
} else {
response
Ok(response)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/renderer/rectangles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ pub enum RectangleError {
#[error("Texture format not supported: {0:?} - use float or integer textures instead.")]
TextureFormatNotSupported(wgpu::TextureFormat),

#[error("Color mapping is being applied to a four-component RGBA texture")]
#[error("Color mapping cannot be applied to a four-component RGBA image, but only to a single-component image.")]
ColormappingRgbaTexture,

#[error("Only 1 and 4 component textures are supported, got {0} components")]
Expand Down
4 changes: 3 additions & 1 deletion crates/re_space_view_spatial/src/visualizers/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,12 +560,14 @@ impl ImageVisualizer {
let tensor_stats = ctx
.cache
.entry(|c: &mut TensorStatsCache| c.entry(tensor_data_row_id, tensor));
let depth_texture = re_viewer_context::gpu_bridge::depth_tensor_to_gpu(
let depth_texture = re_viewer_context::gpu_bridge::tensor_to_gpu(
ctx.render_ctx,
&debug_name,
tensor_data_row_id,
tensor,
TensorDataMeaning::Depth,
&tensor_stats,
&ent_context.annotations,
)?;

let depth_from_world_scale = *properties.depth_from_world_scale;
Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/src/tensor_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ impl DecodedTensor {

// Backwards comparabillity shim
// TODO(jleibs): fully express this in terms of indicator components
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum TensorDataMeaning {
/// Default behavior: guess based on shape
Unknown,
Expand Down
5 changes: 1 addition & 4 deletions crates/re_viewer_context/src/gpu_bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ mod tensor_to_gpu;

pub use colormap::colormap_dropdown_button_ui;
pub use re_renderer_callback::new_renderer_callback;
pub use tensor_to_gpu::{
class_id_tensor_to_gpu, color_tensor_to_gpu, depth_tensor_to_gpu, tensor_to_gpu,
texture_height_width_channels,
};
pub use tensor_to_gpu::{tensor_to_gpu, texture_height_width_channels};

use crate::TensorStats;

Expand Down
100 changes: 55 additions & 45 deletions crates/re_viewer_context/src/gpu_bridge/tensor_to_gpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ use super::{get_or_create_texture, try_get_or_create_texture};

// ----------------------------------------------------------------------------

#[derive(Copy, Clone, Hash)]
enum TextureKeyUsage {
AnnotationContextColormap,
TensorData(TensorDataMeaning),
}

/// Returns a texture key for a given row id & usage.
///
/// Several textures may be created from the same row.
/// This makes sure that they all get different keys!
fn generate_texture_key(row_id: RowId, usage: TextureKeyUsage) -> u64 {
hash((row_id, usage))
}

/// Set up tensor for rendering on the GPU.
///
/// This will only upload the tensor if it isn't on the GPU already.
Expand All @@ -49,43 +63,37 @@ pub fn tensor_to_gpu(
tensor.shape()
));

let texture_key =
generate_texture_key(tensor_data_row_id, TextureKeyUsage::TensorData(meaning));

match meaning {
TensorDataMeaning::Unknown => color_tensor_to_gpu(
render_ctx,
debug_name,
tensor_data_row_id,
tensor,
tensor_stats,
),
TensorDataMeaning::Unknown => {
color_tensor_to_gpu(render_ctx, debug_name, texture_key, tensor, tensor_stats)
}
TensorDataMeaning::ClassId => class_id_tensor_to_gpu(
render_ctx,
debug_name,
tensor_data_row_id,
texture_key,
tensor,
tensor_stats,
annotations,
),
TensorDataMeaning::Depth => depth_tensor_to_gpu(
render_ctx,
debug_name,
tensor_data_row_id,
tensor,
tensor_stats,
),
TensorDataMeaning::Depth => {
depth_tensor_to_gpu(render_ctx, debug_name, texture_key, tensor, tensor_stats)
}
}
}

// ----------------------------------------------------------------------------
// Color textures:

pub fn color_tensor_to_gpu(
fn color_tensor_to_gpu(
render_ctx: &RenderContext,
debug_name: &str,
tensor_data_row_id: RowId,
texture_key: u64,
tensor: &DecodedTensor,
tensor_stats: &TensorStats,
) -> anyhow::Result<ColormappedTexture> {
let texture_key = hash(tensor_data_row_id);
let [height, width, depth] = texture_height_width_channels(tensor)?;

let texture_handle = try_get_or_create_texture(render_ctx, texture_key, || {
Expand Down Expand Up @@ -197,16 +205,20 @@ pub fn color_tensor_to_gpu(
// ----------------------------------------------------------------------------
// Textures with class_id annotations:

pub fn class_id_tensor_to_gpu(
fn class_id_tensor_to_gpu(
render_ctx: &RenderContext,
debug_name: &str,
tensor_data_row_id: RowId,
texture_key: u64,
tensor: &DecodedTensor,
tensor_stats: &TensorStats,
annotations: &Annotations,
) -> anyhow::Result<ColormappedTexture> {
re_tracing::profile_function!();
let texture_key = hash(tensor_data_row_id);

let colormap_key = generate_texture_key(
annotations.row_id(),
TextureKeyUsage::AnnotationContextColormap,
);

let [_height, _width, depth] = texture_height_width_channels(tensor)?;
anyhow::ensure!(
Expand All @@ -229,27 +241,26 @@ pub fn class_id_tensor_to_gpu(
let colormap_width = 256;
let colormap_height = (num_colors + colormap_width - 1) / colormap_width;

let colormap_texture_handle =
get_or_create_texture(render_ctx, hash(annotations.row_id()), || {
let data: Vec<u8> = (0..(colormap_width * colormap_height))
.flat_map(|id| {
let color = annotations
.resolved_class_description(Some(ClassId::from(id as u16)))
.annotation_info()
.color(None, DefaultColor::TransparentBlack);
color.to_array() // premultiplied!
})
.collect();

Texture2DCreationDesc {
label: "class_id_colormap".into(),
data: data.into(),
format: TextureFormat::Rgba8UnormSrgb,
width: colormap_width as u32,
height: colormap_height as u32,
}
})
.context("Failed to create class_id_colormap.")?;
let colormap_texture_handle = get_or_create_texture(render_ctx, colormap_key, || {
let data: Vec<u8> = (0..(colormap_width * colormap_height))
.flat_map(|id| {
let color = annotations
.resolved_class_description(Some(ClassId::from(id as u16)))
.annotation_info()
.color(None, DefaultColor::TransparentBlack);
color.to_array() // premultiplied!
})
.collect();

Texture2DCreationDesc {
label: "class_id_colormap".into(),
data: data.into(),
format: TextureFormat::Rgba8UnormSrgb,
width: colormap_width as u32,
height: colormap_height as u32,
}
})
.context("Failed to create class_id_colormap.")?;

let main_texture_handle = try_get_or_create_texture(render_ctx, texture_key, || {
general_texture_creation_desc_from_tensor(debug_name, tensor)
Expand All @@ -270,15 +281,14 @@ pub fn class_id_tensor_to_gpu(
// ----------------------------------------------------------------------------
// Depth textures:

pub fn depth_tensor_to_gpu(
fn depth_tensor_to_gpu(
render_ctx: &RenderContext,
debug_name: &str,
tensor_data_row_id: RowId,
texture_key: u64,
tensor: &DecodedTensor,
tensor_stats: &TensorStats,
) -> anyhow::Result<ColormappedTexture> {
re_tracing::profile_function!();
let texture_key = hash(tensor_data_row_id);

let [_height, _width, depth] = texture_height_width_channels(tensor)?;
anyhow::ensure!(
Expand Down

0 comments on commit 4ca07b7

Please sign in to comment.