Skip to content

Commit

Permalink
Add sanity check that setup_kittest_for_rendering is called when needed
Browse files Browse the repository at this point in the history
  • Loading branch information
Wumpf committed Jan 9, 2025
1 parent 9d84f7e commit 25206ca
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
5 changes: 5 additions & 0 deletions crates/viewer/re_renderer/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ impl RenderContext {
before_view_builder_encoder: Mutex::new(FrameGlobalCommandEncoder::new(&device)),
frame_index: STARTUP_FRAME_IDX,
top_level_error_scope,
num_view_builders_created: AtomicU64::new(0),
};

// Register shader workarounds for the current device.
Expand Down Expand Up @@ -316,6 +317,7 @@ This means, either a call to RenderContext::before_submit was omitted, or the pr
before_view_builder_encoder: Mutex::new(FrameGlobalCommandEncoder::new(&self.device)),
frame_index: self.active_frame.frame_index.wrapping_add(1),
top_level_error_scope: Some(WgpuErrorScope::start(&self.device)),
num_view_builders_created: AtomicU64::new(0),
};
let frame_index = self.active_frame.frame_index;

Expand Down Expand Up @@ -485,6 +487,9 @@ pub struct ActiveFrameContext {
///
/// The only time this is allowed to be `None` is during shutdown and when closing an old and opening a new scope.
top_level_error_scope: Option<WgpuErrorScope>,

/// Number of view builders created in this frame so far.
pub num_view_builders_created: AtomicU64,
}

fn log_adapter_info(info: &wgpu::AdapterInfo) {
Expand Down
4 changes: 4 additions & 0 deletions crates/viewer/re_renderer/src/view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@ impl ViewBuilder {
frame_uniform_buffer_content,
};

ctx.active_frame
.num_view_builders_created
.fetch_add(1, std::sync::atomic::Ordering::Release);

Self {
setup,
queued_draws: vec![composition_draw.into()],
Expand Down
26 changes: 24 additions & 2 deletions crates/viewer/re_viewer_context/src/test_context.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::sync::atomic::AtomicBool;
use std::sync::Arc;

use ahash::HashMap;
Expand Down Expand Up @@ -43,7 +44,9 @@ pub struct TestContext {

command_sender: CommandSender,
command_receiver: CommandReceiver,

egui_render_state: Mutex<Option<egui_wgpu::RenderState>>,
called_setup_kittest_for_rendering: AtomicBool,
}

impl Default for TestContext {
Expand Down Expand Up @@ -83,6 +86,7 @@ impl Default for TestContext {

// Created lazily since each egui_kittest harness needs a new one.
egui_render_state: Mutex::new(None),
called_setup_kittest_for_rendering: AtomicBool::new(false),
}
}
}
Expand Down Expand Up @@ -228,15 +232,18 @@ fn init_shared_renderer_setup() -> SharedWgpuResources {

impl TestContext {
pub fn setup_kittest_for_rendering(&self) -> egui_kittest::HarnessBuilder<()> {
// Egui kittests insists on having a fresh render state for each test.
let new_render_state = create_egui_renderstate();
let builder = egui_kittest::Harness::builder().renderer(
// Note that render state clone is mostly cloning of inner `Arc`.
// This does _not_ duplicate re_renderer's context.
// This does _not_ duplicate re_renderer's context contained within.
egui_kittest::wgpu::WgpuTestRenderer::from_render_state(new_render_state.clone()),
);
// Egui kittests insists on having a fresh render state for each test.
self.egui_render_state.lock().replace(new_render_state);

self.called_setup_kittest_for_rendering
.store(true, std::sync::atomic::Ordering::Relaxed);

builder
}

Expand Down Expand Up @@ -303,6 +310,21 @@ impl TestContext {

func(&ctx);

// If re_renderer was used, `setup_kittest_for_rendering` should have been called.
// (if not, then we clearly didn't render anything since that requires writing to a texture!).
if render_ctx
.active_frame
.num_view_builders_created
.load(std::sync::atomic::Ordering::Acquire)
!= 0
&& !self
.called_setup_kittest_for_rendering
.load(std::sync::atomic::Ordering::Relaxed)
{
panic!("Rendering with `re_renderer` requires setting up kittest with `TestContext::setup_kittest_for_rendering`
to ensure that kittest & re_renderer use the same graphics device.");
}

render_ctx.before_submit();
}

Expand Down

0 comments on commit 25206ca

Please sign in to comment.