Enable re_renderer for all tests using the TestContext if graphics adapter is present
#8606
Conversation
Wumpf
added
🔨 testing
|
@rerun-bot full-check |
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/12656489804 |
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/12660223373 |
|
note to self: expecting this to fail since the PR as is requires graphics adapter creation to be always successful and we don't do anything towards that. But I want to see how it fails exactly and then remedy it (essentially solving #8245) |
|
decided to take this more step by step: for the moment this does not enforce the wgpu adapter and by extension the re_renderer context to be present |
Wumpf
changed the title
re_renderer for all tests using the TestContextre_renderer for all tests using the TestContext if graphics adapter is present
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/12670527402 |
| { name = "pulldown-cmark" }, # Build-dependency via `ply-rs` (!). TODO(emilk): use a better crate for .ply parsing | ||
| { name = "redox_syscall" }, # Plenty of versions in the wild | ||
| { name = "pollster" }, # rfd is still on 0.3 | ||
| { name = "accesskit_consumer" }, # Duplicate as when used as dev dependency - eframe & egui_kittest are on different versions. |
There was a problem hiding this comment.
this line is new, nothing else changed
There was a problem hiding this comment.
Random thought: currently we silently opt-out when ViewerContext::render_ctx is None.
Maybe we should panic instead in debug builds?
That way we can run tests that don't require the render_ctx by setting it to None, but will not accidentally forget to set it for tests that do need it (like the colormap). i.e., forgetting to call setup_kittest_for_rendering will be a hard error.
There was a problem hiding this comment.
we're soooo close to having this be a thing of the past 😄
let's hold on for a bit
|
|
||
| let compatible_surface = None; | ||
| let depth_format = None; | ||
| let msaa_samples = 1; |
There was a problem hiding this comment.
re_renderer's individual views (managed each by a ViewBuilder) have, but egui's final target doesn't - re_Renderer resolves and copies into egui in ViewBuilder::composite
leaving a comment about this since this is quite the mental gymnastic!
| } | ||
| } | ||
| } | ||
|
|
||
| fn create_egui_renderstate() -> Option<egui_wgpu::RenderState> { |
There was a problem hiding this comment.
it would be helpful with a docstring explaining why this could return None
There was a problem hiding this comment.
on its way out, but will do anyways just in case I can't finish my thread of work
good idea, I look into it! |
The viewer test harness now tries to create a wgpu adapter (only once via a
Lazy) in order to re-use this in an re_renderer context. As a consequence screenshot tests can now use egui elements that bottom out inre_rendererdrawing (i.e. inViewBuilder).For the moment the adapter creation is allowed to fail, meaning that if we're on a ci machine without any graphics adapter at all, we can still continue as before with render context ==
None.Performance:
Timing reports for
cargo test -p re_component_ui(as reported by test runner) on Windows:Next steps:
render_ctxnon option againWgpuSetup,egui_kittestnow prefers software rasterizers for testing emilk/egui#5506