Skip to content

Commit

Permalink
Rename all instances of frame_maintenance to begin_frame (#1360)
Browse files Browse the repository at this point in the history
Comments already indicated that it should be called at the beginning of a frame!
  • Loading branch information
Wumpf authored Feb 20, 2023
1 parent 548d269 commit afd74ec
Show file tree
Hide file tree
Showing 16 changed files with 41 additions and 46 deletions.
2 changes: 1 addition & 1 deletion crates/re_renderer/examples/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<E: Example + 'static> Application<E> {
self.window.request_redraw();
}
Event::RedrawRequested(_) => {
self.re_ctx.frame_maintenance();
self.re_ctx.begin_frame();

// native debug build
#[cfg(all(not(target_arch = "wasm32"), debug_assertions))]
Expand Down
24 changes: 12 additions & 12 deletions crates/re_renderer/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ impl RenderContext {
/// Call this at the beginning of a new frame.
///
/// Updates internal book-keeping, frame allocators and executes delayed events like shader reloading.
pub fn frame_maintenance(&mut self) {
pub fn begin_frame(&mut self) {
self.frame_index += 1;

// Tick the error tracker so that it knows when to reset!
// Note that we're ticking on frame_maintenance rather than raw frames, which
// Note that we're ticking on begin_frame rather than raw frames, which
// makes a world of difference when we're in a poisoned state.
#[cfg(all(not(target_arch = "wasm32"), debug_assertions))] // native debug build
self.err_tracker.tick();
Expand All @@ -178,8 +178,8 @@ impl RenderContext {
re_log::debug!(?modified_paths, "got some filesystem events");
}

self.mesh_manager.frame_maintenance(self.frame_index);
self.texture_manager_2d.frame_maintenance(self.frame_index);
self.mesh_manager.begin_frame(self.frame_index);
self.texture_manager_2d.begin_frame(self.frame_index);

{
let WgpuResourcePools {
Expand All @@ -195,13 +195,13 @@ impl RenderContext {

// Shader module maintenance must come before render pipelines because render pipeline
// recompilation picks up all shaders that have been recompiled this frame.
shader_modules.frame_maintenance(
shader_modules.begin_frame(
&self.device,
&mut self.resolver,
self.frame_index,
&modified_paths,
);
render_pipelines.frame_maintenance(
render_pipelines.begin_frame(
&self.device,
self.frame_index,
shader_modules,
Expand All @@ -210,14 +210,14 @@ impl RenderContext {

// Bind group maintenance must come before texture/buffer maintenance since it
// registers texture/buffer use
bind_groups.frame_maintenance(self.frame_index, textures, buffers, samplers);
bind_groups.begin_frame(self.frame_index, textures, buffers, samplers);

textures.frame_maintenance(self.frame_index);
buffers.frame_maintenance(self.frame_index);
textures.begin_frame(self.frame_index);
buffers.begin_frame(self.frame_index);

pipeline_layouts.frame_maintenance(self.frame_index);
bind_group_layouts.frame_maintenance(self.frame_index);
samplers.frame_maintenance(self.frame_index);
pipeline_layouts.begin_frame(self.frame_index);
bind_group_layouts.begin_frame(self.frame_index);
samplers.begin_frame(self.frame_index);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/error_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl ErrorTracker {
/// Logs a wgpu error, making sure to deduplicate them as needed.
pub fn handle_error(&self, error: wgpu::Error) {
// The pipeline is in a poisoned state, errors are still coming in: we won't be
// clearing the tracker until it had at least 2 complete frame_maintenance cycles
// clearing the tracker until it had at least 2 complete begin_frame cycles
// without any errors (meaning the swapchain surface is stabilized).
self.clear_countdown.store(3, Ordering::Relaxed);

Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/resource_managers/mesh_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl MeshManager {
self.manager.get(handle)
}

pub(crate) fn frame_maintenance(&mut self, frame_index: u64) {
self.manager.frame_maintenance(frame_index);
pub(crate) fn begin_frame(&mut self, frame_index: u64) {
self.manager.begin_frame(frame_index);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ where
}
}

pub(crate) fn frame_maintenance(&mut self, frame_index: u64) {
pub(crate) fn begin_frame(&mut self, frame_index: u64) {
// Kill single frame resources.
self.single_frame_resources.clear();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl TextureManager2D {
}

#[allow(clippy::unused_self)]
pub(crate) fn frame_maintenance(&mut self, _frame_index: u64) {
pub(crate) fn begin_frame(&mut self, _frame_index: u64) {
// no-op.
// In the future we might add handling of background processing or introduce frame-lived textures.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl GpuBindGroupLayoutPool {
self.pool.get_resource(handle)
}

pub fn frame_maintenance(&mut self, frame_index: u64) {
pub fn begin_frame(&mut self, frame_index: u64) {
self.pool.current_frame_index = frame_index;
}

Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/wgpu_resources/bind_group_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,14 @@ impl GpuBindGroupPool {
}
}

pub fn frame_maintenance(
pub fn begin_frame(
&mut self,
frame_index: u64,
_textures: &mut GpuTexturePool,
_buffers: &mut GpuBufferPool,
_samplers: &mut GpuSamplerPool,
) {
self.pool.frame_maintenance(frame_index, |_res| {});
self.pool.begin_frame(frame_index, |_res| {});
// TODO(andreas): Update usage counter on dependent resources.
}

Expand Down
5 changes: 2 additions & 3 deletions crates/re_renderer/src/wgpu_resources/buffer_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ impl GpuBufferPool {
}

/// Called by `RenderContext` every frame. Updates statistics and may free unused buffers.
pub fn frame_maintenance(&mut self, frame_index: u64) {
self.pool
.frame_maintenance(frame_index, |res| res.destroy());
pub fn begin_frame(&mut self, frame_index: u64) {
self.pool.begin_frame(frame_index, |res| res.destroy());
}

/// Takes strong buffer handle to ensure the user is still holding on to the buffer.
Expand Down
22 changes: 9 additions & 13 deletions crates/re_renderer/src/wgpu_resources/dynamic_resource_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub(super) struct DynamicResourcePool<Handle: Key, Desc, Res> {

/// Handles to all alive resources.
/// We story any ref counted handle we give out in [`DynamicResourcePool::alloc`] here in order to keep it alive.
/// Every [`DynamicResourcePool::frame_maintenance`] we check if the pool is now the only owner of the handle
/// Every [`DynamicResourcePool::begin_frame`] we check if the pool is now the only owner of the handle
/// and if so mark it as deallocated.
/// Being a [`SecondaryMap`] allows us to upgrade "weak" handles to strong handles,
/// something required by [`super::GpuBindGroupPool`]
Expand Down Expand Up @@ -91,11 +91,7 @@ where
})
}

pub fn frame_maintenance(
&mut self,
frame_index: u64,
mut on_destroy_resource: impl FnMut(&Res),
) {
pub fn begin_frame(&mut self, frame_index: u64, mut on_destroy_resource: impl FnMut(&Res)) {
self.current_frame_index = frame_index;

// Throw out any resources that we haven't reclaimed last frame.
Expand Down Expand Up @@ -206,7 +202,7 @@ mod tests {
{
let drop_counter_before = drop_counter.load(Ordering::Acquire);
let mut called_destroy = false;
pool.frame_maintenance(1, |_| called_destroy = true);
pool.begin_frame(1, |_| called_destroy = true);

assert!(!called_destroy);
assert_eq!(drop_counter_before, drop_counter.load(Ordering::Acquire),);
Expand All @@ -221,9 +217,9 @@ mod tests {
{
let drop_counter_before = drop_counter.load(Ordering::Acquire);
let mut called_destroy = false;
pool.frame_maintenance(2, |_| called_destroy = true);
pool.begin_frame(2, |_| called_destroy = true);
assert!(!called_destroy);
pool.frame_maintenance(3, |_| called_destroy = true);
pool.begin_frame(3, |_| called_destroy = true);
assert!(called_destroy);
let drop_counter_now = drop_counter.load(Ordering::Acquire);
assert_eq!(
Expand All @@ -248,10 +244,10 @@ mod tests {
drop(handle1);

let mut called_destroy = false;
pool.frame_maintenance(4, |_| called_destroy = true);
pool.begin_frame(4, |_| called_destroy = true);
assert!(!called_destroy);
assert_eq!(drop_counter_before, drop_counter.load(Ordering::Acquire),);
pool.frame_maintenance(5, |_| called_destroy = true);
pool.begin_frame(5, |_| called_destroy = true);
assert!(called_destroy);
assert_eq!(
drop_counter_before + 1,
Expand Down Expand Up @@ -328,8 +324,8 @@ mod tests {
// Query with invalid handle
let inner_handle = *handle;
drop(handle);
pool.frame_maintenance(0, |_| {});
pool.frame_maintenance(1, |_| {});
pool.begin_frame(0, |_| {});
pool.begin_frame(1, |_| {});
assert!(matches!(
pool.get_resource(inner_handle),
Err(PoolError::ResourceNotAvailable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl GpuPipelineLayoutPool {
self.pool.get_resource(handle)
}

pub fn frame_maintenance(&mut self, frame_index: u64) {
pub fn begin_frame(&mut self, frame_index: u64) {
self.pool.current_frame_index = frame_index;
}

Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/wgpu_resources/render_pipeline_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl GpuRenderPipelinePool {
})
}

pub fn frame_maintenance(
pub fn begin_frame(
&mut self,
device: &wgpu::Device,
frame_index: u64,
Expand All @@ -179,7 +179,7 @@ impl GpuRenderPipelinePool {
// The frame counter just got bumped by one. So any shader that has `frame_created`,
// equal the current frame now, must have been recompiled since the user didn't have a
// chance yet to add new shaders for this frame!
// (note that this assumes that shader `frame_maintenance` happens before pipeline `frame_maintenance`)
// (note that this assumes that shader `begin_frame` happens before pipeline `begin_frame`)
if frame_created < frame_index {
return None;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/wgpu_resources/sampler_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl GpuSamplerPool {
self.pool.get_resource(handle)
}

pub fn frame_maintenance(&mut self, frame_index: u64) {
pub fn begin_frame(&mut self, frame_index: u64) {
self.pool.current_frame_index = frame_index;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl GpuShaderModulePool {
.get_or_create(desc, |desc| desc.create_shader_module(device, resolver))
}

pub fn frame_maintenance<Fs: FileSystem>(
pub fn begin_frame<Fs: FileSystem>(
&mut self,
device: &wgpu::Device,
resolver: &mut FileResolver<Fs>,
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/wgpu_resources/texture_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ impl GpuTexturePool {
}

/// Called by `RenderContext` every frame. Updates statistics and may free unused textures.
pub fn frame_maintenance(&mut self, frame_index: u64) {
pub fn begin_frame(&mut self, frame_index: u64) {
self.pool
.frame_maintenance(frame_index, |res| res.texture.destroy());
.begin_frame(frame_index, |res| res.texture.destroy());
}

/// Takes strong texture handle to ensure the user is still holding on to the texture.
Expand Down
4 changes: 2 additions & 2 deletions crates/re_viewer/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ impl eframe::App for App {
.paint_callback_resources
.get::<re_renderer::RenderContext>()
.unwrap();
// Query statistics before frame_maintenance as this might be more accurate if there's resources that we recreate every frame.
// Query statistics before begin_frame as this might be more accurate if there's resources that we recreate every frame.
render_ctx.gpu_resources.statistics()
};

Expand Down Expand Up @@ -504,7 +504,7 @@ impl eframe::App for App {
.paint_callback_resources
.get_mut::<re_renderer::RenderContext>()
.unwrap();
render_ctx.frame_maintenance();
render_ctx.begin_frame();

if log_db.is_empty() {
wait_screen_ui(ui, &self.rx);
Expand Down

1 comment on commit afd74ec

@github-actions
Copy link

Choose a reason for hiding this comment

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

Rust Benchmark

Benchmark suite Current: afd74ec Previous: 548d269 Ratio
datastore/insert/batch/rects/insert 543855 ns/iter (± 5831) 562227 ns/iter (± 5000) 0.97
datastore/latest_at/batch/rects/query 1800 ns/iter (± 7) 1835 ns/iter (± 11) 0.98
datastore/latest_at/missing_components/primary 355 ns/iter (± 1) 356 ns/iter (± 0) 1.00
datastore/latest_at/missing_components/secondaries 420 ns/iter (± 2) 433 ns/iter (± 2) 0.97
datastore/range/batch/rects/query 149177 ns/iter (± 779) 153144 ns/iter (± 1033) 0.97
mono_points_arrow/generate_message_bundles 46337485 ns/iter (± 463167) 45934909 ns/iter (± 954488) 1.01
mono_points_arrow/generate_messages 124442581 ns/iter (± 1114912) 124859394 ns/iter (± 1122795) 1.00
mono_points_arrow/encode_log_msg 150822654 ns/iter (± 974862) 152120205 ns/iter (± 832273) 0.99
mono_points_arrow/encode_total 324804552 ns/iter (± 2445228) 322750928 ns/iter (± 1610749) 1.01
mono_points_arrow/decode_log_msg 176947517 ns/iter (± 838515) 175393518 ns/iter (± 840170) 1.01
mono_points_arrow/decode_message_bundles 65099027 ns/iter (± 741626) 64581240 ns/iter (± 884473) 1.01
mono_points_arrow/decode_total 237573783 ns/iter (± 2239220) 237345731 ns/iter (± 2848358) 1.00
batch_points_arrow/generate_message_bundles 329864 ns/iter (± 2332) 331144 ns/iter (± 1088) 1.00
batch_points_arrow/generate_messages 6292 ns/iter (± 29) 6299 ns/iter (± 18) 1.00
batch_points_arrow/encode_log_msg 374339 ns/iter (± 775) 371276 ns/iter (± 1319) 1.01
batch_points_arrow/encode_total 729442 ns/iter (± 2011) 716954 ns/iter (± 2274) 1.02
batch_points_arrow/decode_log_msg 345266 ns/iter (± 989) 348583 ns/iter (± 913) 0.99
batch_points_arrow/decode_message_bundles 2058 ns/iter (± 12) 2058 ns/iter (± 15) 1
batch_points_arrow/decode_total 351915 ns/iter (± 568) 346709 ns/iter (± 1881) 1.02
arrow_mono_points/insert 6001053877 ns/iter (± 10507801) 6021744068 ns/iter (± 15125163) 1.00
arrow_mono_points/query 1723117 ns/iter (± 5184) 1714409 ns/iter (± 8578) 1.01
arrow_batch_points/insert 2624028 ns/iter (± 7661) 2618441 ns/iter (± 10492) 1.00
arrow_batch_points/query 16896 ns/iter (± 16) 16864 ns/iter (± 56) 1.00
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.