Skip to content

Commit

Permalink
comment fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Wumpf committed Dec 5, 2023
1 parent 549cbd3 commit 8e4b0d8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 27 deletions.
2 changes: 1 addition & 1 deletion crates/re_renderer/src/view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ impl ViewBuilder {
// Renderers can't be added anyways at this point (RendererData add their Renderer on creation),
// so no point in taking the lock repeatedly.
//
// Note that this is a limitation that will be lifted in future versions of wgpu.
// TODO(gfx-rs/wgpu#1453): Note that this is a limitation that will be lifted in future versions of wgpu.
// However, having our locking concentrated for the duration of a view draw
// is also beneficial since it enforces the model of prepare->draw which avoids a lot of repeated
// locking and unlocking.
Expand Down
37 changes: 16 additions & 21 deletions crates/re_renderer/src/wgpu_resources/static_resource_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,30 +49,24 @@ where
Desc: std::fmt::Debug + Clone + Eq + Hash,
{
pub fn get_or_create<F: FnOnce(&Desc) -> Res>(&self, desc: &Desc, creation_func: F) -> Handle {
{
// Ensure the lock isn't held in the creation case.
if let Some(handle) = self.lookup.read().get(desc) {
return *handle;
}
// Ensure the lock isn't held in the creation case.
if let Some(handle) = self.lookup.read().get(desc) {
return *handle;
}
{
re_tracing::profile_scope!(
"Creating new static resource",
std::any::type_name::<Res>()
);

let resource = creation_func(desc);
let handle = self.resources.write().insert(StoredResource {
resource,
statistics: ResourceStatistics {
frame_created: self.current_frame_index,
last_frame_used: self.current_frame_index.into(),
},
});
self.lookup.write().insert(desc.clone(), handle);
re_tracing::profile_scope!("Creating new static resource", std::any::type_name::<Res>());

handle
}
let resource = creation_func(desc);
let handle = self.resources.write().insert(StoredResource {
resource,
statistics: ResourceStatistics {
frame_created: self.current_frame_index,
last_frame_used: self.current_frame_index.into(),
},
});
self.lookup.write().insert(desc.clone(), handle);

handle
}

pub fn recreate_resources<F: FnMut(&Desc) -> Option<Res>>(&mut self, mut recreation_func: F) {
Expand Down Expand Up @@ -137,6 +131,7 @@ fn to_pool_error<T>(get_result: Option<T>, handle: impl Key) -> Result<T, PoolEr
})
}

/// Accessor to the resource pool, either by taking a read lock or by moving out the resources.
pub trait StaticResourcePoolAccessor<Handle: Key, Res> {
fn get(&self, handle: Handle) -> Result<&Res, PoolError>;
}
Expand Down
11 changes: 6 additions & 5 deletions crates/re_viewer_context/src/gpu_bridge/re_renderer_callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback {
return Vec::new();
};

let command_buffer = match view_builder.draw(ctx, self.clear_color) {
match view_builder.draw(ctx, self.clear_color) {
Ok(command_buffer) => {
// If drawing worked, put the view_builder back in so we can use it during paint.
ctx.active_frame
Expand All @@ -90,9 +90,7 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback {
// TODO(andreas): It would be nice to paint an error message instead.
Vec::new()
}
};

command_buffer
}
}

fn finish_prepare(
Expand All @@ -114,7 +112,7 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback {
// are longer lived than the pass itself.
// This is a bit of a conundrum since we can't store a lock guard in the callback resources.
// So instead, we work around this by moving the render pipelines out of their lock!
// Future wgpu versions will lift this restriction and will allow us to remove this workaround.
// TODO(gfx-rs/wgpu#1453): Future wgpu versions will lift this restriction and will allow us to remove this workaround.
if ctx.active_frame.pinned_render_pipelines.is_none() {
let render_pipelines = ctx.gpu_resources.render_pipelines.take_resources();
ctx.active_frame.pinned_render_pipelines = Some(render_pipelines);
Expand All @@ -130,12 +128,14 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback {
paint_callback_resources: &'a egui_wgpu::CallbackResources,
) {
let Some(ctx) = paint_callback_resources.get::<re_renderer::RenderContext>() else {
// TODO(#4433): Shouldn't show up like this.
re_log::error_once!(
"Failed to execute egui draw callback. No render context available."
);
return;
};
let Some(render_pipelines) = ctx.active_frame.pinned_render_pipelines.as_ref() else {
// TODO(#4433): Shouldn't show up like this.
re_log::error_once!(
"Failed to execute egui draw callback. Render pipelines weren't transferred out of the pool first."
);
Expand All @@ -148,6 +148,7 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback {
.get::<ViewBuilderMap>()
.and_then(|view_builder_map| view_builder_map.get(self.view_builder))
else {
// TODO(#4433): Shouldn't show up like this.
re_log::error_once!(
"Failed to execute egui draw callback. View builder with handle {:?} not found.",
self.view_builder
Expand Down

0 comments on commit 8e4b0d8

Please sign in to comment.