Skip to content

Commit

Permalink
make view builder draw take a non mutable reference to self to resolv…
Browse files Browse the repository at this point in the history
…e anther egui callback conundrum. Trickle out resulting mut removal all the way to viewer_context
  • Loading branch information
Wumpf committed Dec 5, 2023
1 parent eab8244 commit 965d71c
Show file tree
Hide file tree
Showing 16 changed files with 46 additions and 110 deletions.
10 changes: 5 additions & 5 deletions crates/re_data_ui/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl EntityDataUi for re_types::components::TensorData {

#[allow(clippy::too_many_arguments)]
fn tensor_ui(
ctx: &mut ViewerContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
verbosity: UiVerbosity,
entity_path: &re_data_store::EntityPath,
Expand Down Expand Up @@ -253,7 +253,7 @@ 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.
fn show_image_preview(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
re_ui: &ReUi,
ui: &mut egui::Ui,
colormapped_texture: ColormappedTexture,
Expand Down Expand Up @@ -433,7 +433,7 @@ pub fn tensor_summary_ui(

#[allow(clippy::too_many_arguments)]
fn show_zoomed_image_region_tooltip(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
parent_ui: &egui::Ui,
response: egui::Response,
tensor_data_row_id: RowId,
Expand Down Expand Up @@ -523,7 +523,7 @@ pub fn show_zoomed_image_region_area_outline(
/// `meter`: iff this is a depth map, how long is one meter?
#[allow(clippy::too_many_arguments)]
pub fn show_zoomed_image_region(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
ui: &mut egui::Ui,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
Expand Down Expand Up @@ -553,7 +553,7 @@ pub fn show_zoomed_image_region(
/// `meter`: iff this is a depth map, how long is one meter?
#[allow(clippy::too_many_arguments)]
fn try_show_zoomed_image_region(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
ui: &mut egui::Ui,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/allocator/gpu_readback_belt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl GpuReadbackBuffer {
/// Does 2D-only entirely for convenience as it greatly simplifies the input parameters.
/// Additionally, we assume as tightly as possible packed data as this is by far the most common use.
pub fn read_texture2d(
self,
&mut self,
encoder: &mut wgpu::CommandEncoder,
source: wgpu::ImageCopyTexture<'_>,
copy_extents: glam::UVec2,
Expand All @@ -58,7 +58,7 @@ impl GpuReadbackBuffer {
/// This method will add the required padding between the texture copies if necessary.
/// Panics if the buffer is too small.
pub fn read_multiple_texture2d(
mut self,
&mut self,
encoder: &mut wgpu::CommandEncoder,
sources_and_extents: &[(wgpu::ImageCopyTexture<'_>, glam::UVec2)],
) -> Result<(), GpuReadbackError> {
Expand Down
5 changes: 0 additions & 5 deletions crates/re_renderer/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ impl RenderContext {

let active_frame = ActiveFrameContext {
before_view_builder_encoder: Mutex::new(FrameGlobalCommandEncoder::new(&device)),
per_frame_data_helper: TypeMap::new(),
pinned_render_pipelines: None,
frame_index: 0,
};
Expand Down Expand Up @@ -312,7 +311,6 @@ impl RenderContext {
before_view_builder_encoder: Mutex::new(FrameGlobalCommandEncoder::new(&self.device)),
frame_index: self.active_frame.frame_index + 1,
pinned_render_pipelines: None,
per_frame_data_helper: TypeMap::new(),
};
let frame_index = self.active_frame.frame_index;

Expand Down Expand Up @@ -462,9 +460,6 @@ pub struct ActiveFrameContext {
/// (i.e. typically in [`crate::renderer::DrawData`] creation!)
pub before_view_builder_encoder: Mutex<FrameGlobalCommandEncoder>,

/// Utility type map that will be cleared every frame.
pub per_frame_data_helper: TypeMap,

/// Render pipelines that were moved out of the resource pool.
///
/// Will be moved back to the resource pool at the start of the frame.
Expand Down
6 changes: 3 additions & 3 deletions crates/re_renderer/src/draw_phases/outlines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ impl OutlineMaskProcessor {
}

pub fn compute_outlines(
self,
&self,
pipelines: &GpuRenderPipelinePoolAccessor<'_>,
encoder: &mut wgpu::CommandEncoder,
) -> Result<(), PoolError> {
Expand Down Expand Up @@ -402,7 +402,7 @@ impl OutlineMaskProcessor {

// Perform jump flooding.
let render_pipeline_step = pipelines.get(self.render_pipeline_jumpflooding_step)?;
for (i, bind_group) in self.bind_group_jumpflooding_steps.into_iter().enumerate() {
for (i, bind_group) in self.bind_group_jumpflooding_steps.iter().enumerate() {
let mut jumpflooding_step = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: DebugLabel::from(format!("{} - jumpflooding_step {i}", self.label)).get(),
color_attachments: &[Some(wgpu::RenderPassColorAttachment {
Expand All @@ -417,7 +417,7 @@ impl OutlineMaskProcessor {
});

jumpflooding_step.set_pipeline(render_pipeline_step);
jumpflooding_step.set_bind_group(0, &bind_group, &[]);
jumpflooding_step.set_bind_group(0, bind_group, &[]);
jumpflooding_step.draw(0..3, 0..1);
}

Expand Down
11 changes: 6 additions & 5 deletions crates/re_renderer/src/draw_phases/picking_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::{
DebugLabel, GpuReadbackBuffer, GpuReadbackIdentifier, RectInt, RenderContext,
};

use parking_lot::Mutex;
use smallvec::smallvec;

/// GPU retrieved & processed picking data result.
Expand Down Expand Up @@ -141,7 +142,7 @@ pub enum PickingLayerError {
pub struct PickingLayerProcessor {
pub picking_target: GpuTexture,
picking_depth_target: GpuTexture,
readback_buffer: GpuReadbackBuffer,
readback_buffer: Mutex<GpuReadbackBuffer>,
bind_group_0: GpuBindGroup,

depth_readback_workaround: Option<DepthReadbackWorkaround>,
Expand Down Expand Up @@ -285,7 +286,7 @@ impl PickingLayerProcessor {
);
let buffer_size = row_info_id.buffer_size_padded + row_info_depth.buffer_size_padded;

let readback_buffer = ctx.gpu_readback_belt.lock().allocate(
let readback_buffer = Mutex::new(ctx.gpu_readback_belt.lock().allocate(
&ctx.device,
&ctx.gpu_resources.buffers,
buffer_size,
Expand All @@ -296,7 +297,7 @@ impl PickingLayerProcessor {
world_from_cropped_projection: cropped_projection_from_world.inverse(),
depth_readback_workaround_in_use: depth_readback_workaround.is_some(),
}),
);
));

PickingLayerProcessor {
bind_group_0,
Expand Down Expand Up @@ -342,7 +343,7 @@ impl PickingLayerProcessor {
}

pub fn end_render_pass(
self,
&self,
encoder: &mut wgpu::CommandEncoder,
render_pipelines: &GpuRenderPipelinePoolAccessor<'_>,
) -> Result<(), PickingLayerError> {
Expand All @@ -362,7 +363,7 @@ impl PickingLayerProcessor {
&self.picking_depth_target
};

self.readback_buffer.read_multiple_texture2d(
self.readback_buffer.lock().read_multiple_texture2d(
encoder,
&[
(
Expand Down
12 changes: 7 additions & 5 deletions crates/re_renderer/src/draw_phases/screenshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//! We could render the same image with subpixel moved camera in order to get super-sampling without hitting texture size limitations.
//! Or alternatively try to render the images in several tiles 🤔. In any case this would greatly improve quality!
use parking_lot::Mutex;

use crate::{
allocator::GpuReadbackError,
texture_info::Texture2DBufferInfo,
Expand All @@ -25,7 +27,7 @@ struct ReadbackBeltMetadata<T: 'static + Send + Sync> {

pub struct ScreenshotProcessor {
screenshot_texture: GpuTexture,
screenshot_readback_buffer: GpuReadbackBuffer,
screenshot_readback_buffer: Mutex<GpuReadbackBuffer>,
}

impl ScreenshotProcessor {
Expand All @@ -40,7 +42,7 @@ impl ScreenshotProcessor {
readback_user_data: T,
) -> Self {
let buffer_info = Texture2DBufferInfo::new(Self::SCREENSHOT_COLOR_FORMAT, resolution);
let screenshot_readback_buffer = ctx.gpu_readback_belt.lock().allocate(
let screenshot_readback_buffer = Mutex::new(ctx.gpu_readback_belt.lock().allocate(
&ctx.device,
&ctx.gpu_resources.buffers,
buffer_info.buffer_size_padded,
Expand All @@ -49,7 +51,7 @@ impl ScreenshotProcessor {
extent: resolution,
user_data: readback_user_data,
}),
);
));

let screenshot_texture = ctx.gpu_resources.textures.alloc(
&ctx.device,
Expand Down Expand Up @@ -100,10 +102,10 @@ impl ScreenshotProcessor {
}

pub fn end_render_pass(
self,
&self,
encoder: &mut wgpu::CommandEncoder,
) -> Result<(), GpuReadbackError> {
self.screenshot_readback_buffer.read_texture2d(
self.screenshot_readback_buffer.lock().read_texture2d(
encoder,
wgpu::ImageCopyTexture {
texture: &self.screenshot_texture.texture,
Expand Down
8 changes: 4 additions & 4 deletions crates/re_renderer/src/view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ impl ViewBuilder {

/// Draws the frame as instructed to a temporary HDR target.
pub fn draw(
&mut self,
&self,
ctx: &RenderContext,
clear_color: Rgba,
) -> Result<wgpu::CommandBuffer, PoolError> {
Expand Down Expand Up @@ -608,7 +608,7 @@ impl ViewBuilder {
}
}

if let Some(picking_processor) = self.picking_processor.take() {
if let Some(picking_processor) = &self.picking_processor {
{
let mut pass = picking_processor.begin_render_pass(&setup.name, &mut encoder);
// PickingProcessor has as custom frame uniform buffer.
Expand All @@ -635,7 +635,7 @@ impl ViewBuilder {
}
}

if let Some(outline_mask_processor) = self.outline_mask_processor.take() {
if let Some(outline_mask_processor) = &self.outline_mask_processor {
re_tracing::profile_scope!("outlines");
{
re_tracing::profile_scope!("outline mask pass");
Expand All @@ -646,7 +646,7 @@ impl ViewBuilder {
outline_mask_processor.compute_outlines(&pipelines, &mut encoder)?;
}

if let Some(screenshot_processor) = self.screenshot_processor.take() {
if let Some(screenshot_processor) = &self.screenshot_processor {
{
let mut pass = screenshot_processor.begin_render_pass(&setup.name, &mut encoder);
pass.set_bind_group(0, &setup.bind_group_0, &[]);
Expand Down
1 change: 0 additions & 1 deletion crates/re_space_view_spatial/src/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ pub fn view_2d(
// Draw a re_renderer driven view.
// Camera & projection are configured to ingest space coordinates directly.
painter.add(gpu_bridge::new_renderer_callback(
ctx.render_ctx,
view_builder,
painter.clip_rect(),
ui.visuals().extreme_bg_color.into(),
Expand Down
1 change: 0 additions & 1 deletion crates/re_space_view_spatial/src/ui_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,6 @@ pub fn view_3d(
ctx.render_ctx,
));
ui.painter().add(gpu_bridge::new_renderer_callback(
ctx.render_ctx,
view_builder,
rect,
re_renderer::Rgba::TRANSPARENT,
Expand Down
10 changes: 5 additions & 5 deletions crates/re_space_view_tensor/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl PerTensorState {
&self.color_mapping
}

pub fn ui(&mut self, ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) {
pub fn ui(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) {
let Some((tensor_data_row_id, tensor)) = &self.tensor else {
ui.label("No Tensor shown in this Space View.");
return;
Expand Down Expand Up @@ -234,7 +234,7 @@ impl SpaceViewClass for TensorSpaceView {
}

fn view_tensor(
ctx: &mut ViewerContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
state: &mut PerTensorState,
tensor_data_row_id: RowId,
Expand Down Expand Up @@ -292,7 +292,7 @@ fn view_tensor(
}

fn tensor_slice_ui(
ctx: &mut ViewerContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
state: &PerTensorState,
tensor_data_row_id: RowId,
Expand All @@ -311,7 +311,7 @@ fn tensor_slice_ui(
}

fn paint_tensor_slice(
ctx: &mut ViewerContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
state: &PerTensorState,
tensor_data_row_id: RowId,
Expand Down Expand Up @@ -384,7 +384,7 @@ impl Default for ColorMapping {
impl ColorMapping {
fn ui(
&mut self,
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
re_ui: &re_ui::ReUi,
ui: &mut egui::Ui,
) {
Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl AppState {
&mut self,
app_blueprint: &AppBlueprint<'_>,
ui: &mut egui::Ui,
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
store_db: &StoreDb,
store_context: &StoreContext<'_>,
re_ui: &re_ui::ReUi,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ fn space_view_top_level_properties(

fn container_top_level_properties(
ui: &mut egui::Ui,
_ctx: &mut ViewerContext<'_>,
_ctx: &ViewerContext<'_>,
viewport: &mut ViewportBlueprint<'_>,
tile_id: &egui_tiles::TileId,
) {
Expand Down Expand Up @@ -739,7 +739,7 @@ fn entity_props_ui(
}

fn colormap_props_ui(
ctx: &mut ViewerContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
entity_props: &mut EntityProperties,
) {
Expand Down
4 changes: 2 additions & 2 deletions crates/re_viewer_context/src/gpu_bridge/colormap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::gpu_bridge::{get_or_create_texture, render_image};

/// Show the given colormap as a horizontal bar.
fn colormap_preview_ui(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
ui: &mut egui::Ui,
colormap: re_renderer::Colormap,
) -> anyhow::Result<egui::Response> {
Expand Down Expand Up @@ -60,7 +60,7 @@ fn colormap_preview_ui(
}

pub fn colormap_dropdown_button_ui(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
ui: &mut egui::Ui,
map: &mut re_renderer::Colormap,
) {
Expand Down
3 changes: 1 addition & 2 deletions crates/re_viewer_context/src/gpu_bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub fn get_or_create_texture<'a>(

/// Render the given image, respecting the clip rectangle of the given painter.
pub fn render_image(
render_ctx: &mut re_renderer::RenderContext,
render_ctx: &re_renderer::RenderContext,
egui_painter: &egui::Painter,
image_rect_on_screen: egui::Rect,
colormapped_texture: ColormappedTexture,
Expand Down Expand Up @@ -196,7 +196,6 @@ pub fn render_image(
)?);

egui_painter.add(new_renderer_callback(
render_ctx,
view_builder,
viewport,
re_renderer::Rgba::TRANSPARENT,
Expand Down
Loading

0 comments on commit 965d71c

Please sign in to comment.