Skip to content

Commit

Permalink
Make viewer contexts's render context reference non-mutable (#4430)
Browse files Browse the repository at this point in the history
### What

* Prerequisite for #1325
* Follow-up of #4422

Makes `ViewBuilder`'s draw method no longer require a mutable render
context, thus no longer requiring a mutable frame state on the render
context (which previously held the mutable viewbuilder!), thus finally
lifting the need to store mutable references to the render context from
the viewer context!

Having a non-mut draw method on ViewBuilder is _mostly_ straight forward
at this point. The main hurdle are render phases that so far consumed
themselves on drawing because of readback textures (which are still
self-consuming for good reasons).

### Checklist
* [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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4430/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4430/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4430)
- [Docs
preview](https://rerun.io/preview/965d71c3723e74947e2d063372d51d9ae89cc9dd/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/965d71c3723e74947e2d063372d51d9ae89cc9dd/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)


---
Part of series towards more multithreading in the viewer!
* #4387
* #4404
* #4389
* #4421
* #4422 
* You are here ➡️ #4430
  • Loading branch information
Wumpf authored Dec 5, 2023
1 parent 90429c6 commit d78d06c
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 d78d06c

Please sign in to comment.