From 6a3cd867f0564329e5e15224d004c8bb6a416b3b Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 2 Jun 2026 21:16:58 +0200 Subject: [PATCH] Validate OpenGL command pass barriers --- docs/modernization/build-inventory.md | 13 ++-- docs/modernization/roadmap.md | 8 +-- src/renderer_gl/command_plan.cpp | 23 +++++++ tests/renderer_gl/command_plan_tests.cpp | 81 ++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 10 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 9887ea8..2753d6f 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -279,12 +279,13 @@ Known local toolchain state: uploads, mipmap generation, texture transitions, texture copies, texture readbacks, frame captures, passthrough commands, trace commands, unsupported commands, and render-pass ordering errors such as state changes outside a - pass, nested passes, and unclosed passes. It also validates executable - command dependencies, including shader-before-uniform and shader-plus-mesh - before draw within each render pass, and rejects invalid texture/sampler bind - slots in malformed recorded streams. `pano_cli record-render` emits the - OpenGL plan texture/sampler bind counts so automation can assert backend - interpretation without an OpenGL context. + pass, nested passes, texture I/O or blits inside a pass, and unclosed passes. + It also validates executable command dependencies, including + shader-before-uniform and shader-plus-mesh before draw within each render + pass, and rejects invalid texture/sampler bind slots in malformed recorded + streams. `pano_cli record-render` emits the OpenGL plan texture/sampler bind + counts so automation can assert backend interpretation without an OpenGL + context. Desktop VR drawing also consumes backend-owned scissor/depth/blend state, depth clear masks, active texture units, and fallback 2D texture unbind targets while retaining the existing VR SDK/platform bridge shape. diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 0ee7609..3a3732c 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -538,10 +538,10 @@ passes, draws, shader binds, shader uniforms, texture/sampler binds, texture uploads, mipmap generation, texture transitions, texture copies, texture readbacks, frame captures, passthrough commands, trace commands, unsupported commands, and render-pass ordering errors such as state changes outside a pass, -nested passes, and unclosed passes. It also validates executable command -dependencies, including shader-before-uniform and shader-plus-mesh before draw -within each render pass, and rejects invalid texture/sampler bind slots in -malformed recorded streams. +nested passes, texture I/O or blits inside a pass, and unclosed passes. It +also validates executable command dependencies, including shader-before-uniform +and shader-plus-mesh before draw within each render pass, and rejects invalid +texture/sampler bind slots in malformed recorded streams. The existing renderer classes are not yet fully behind the renderer interfaces. diff --git a/src/renderer_gl/command_plan.cpp b/src/renderer_gl/command_plan.cpp index 4e97fa2..1d98c11 100644 --- a/src/renderer_gl/command_plan.cpp +++ b/src/renderer_gl/command_plan.cpp @@ -370,21 +370,39 @@ OpenGlCommandPlan plan_recorded_render_commands( break; case OpenGlPlannedCommandKind::upload_texture: ++plan.upload_command_count; + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } break; case OpenGlPlannedCommandKind::generate_mipmaps: ++plan.mipmap_command_count; + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } break; case OpenGlPlannedCommandKind::transition_texture: ++plan.transition_command_count; + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } break; case OpenGlPlannedCommandKind::copy_texture: ++plan.copy_command_count; + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } break; case OpenGlPlannedCommandKind::read_texture: ++plan.readback_command_count; + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } break; case OpenGlPlannedCommandKind::capture_frame: ++plan.capture_command_count; + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } break; case OpenGlPlannedCommandKind::passthrough: ++plan.passthrough_command_count; @@ -395,6 +413,11 @@ OpenGlCommandPlan plan_recorded_render_commands( case OpenGlPlannedCommandKind::trace: ++plan.trace_command_count; break; + case OpenGlPlannedCommandKind::blit_render_target: + if (in_render_pass) { + record_render_pass_order_error(plan, index); + } + break; default: if (planned.requires_render_pass && !in_render_pass) { record_render_pass_order_error(plan, index); diff --git a/tests/renderer_gl/command_plan_tests.cpp b/tests/renderer_gl/command_plan_tests.cpp index 997f6e0..59771e4 100644 --- a/tests/renderer_gl/command_plan_tests.cpp +++ b/tests/renderer_gl/command_plan_tests.cpp @@ -476,6 +476,86 @@ void counts_texture_and_sampler_bindings_in_streams(pp::tests::Harness& h) PP_EXPECT(h, plan.commands[3].sampler_slot == 2U); } +void expect_inside_pass_order_error( + pp::tests::Harness& h, + pp::renderer::RecordedRenderCommand command, + pp::renderer::gl::OpenGlPlannedCommandKind expected_kind) +{ + const std::vector commands { + begin_render_pass_command(), + command, + command_with_kind(pp::renderer::RecordedRenderCommandKind::end_render_pass), + }; + + const auto plan = pp::renderer::gl::plan_recorded_render_commands(commands); + + PP_EXPECT(h, !plan.supported); + PP_EXPECT(h, plan.unsupported_command_count == 0U); + PP_EXPECT(h, plan.render_pass_order_error_count == 1U); + PP_EXPECT(h, plan.first_render_pass_order_error == 1U); + PP_EXPECT(h, plan.dependency_error_count == 0U); + PP_EXPECT(h, plan.commands[1].kind == expected_kind); +} + +void flags_texture_io_inside_render_pass(pp::tests::Harness& h) +{ + pp::renderer::RecordedRenderCommand upload_command; + upload_command.kind = pp::renderer::RecordedRenderCommandKind::upload_texture; + upload_command.texture_desc.format = pp::renderer::TextureFormat::rgba8; + + pp::renderer::RecordedRenderCommand mipmap_command; + mipmap_command.kind = pp::renderer::RecordedRenderCommandKind::generate_mipmaps; + mipmap_command.texture_desc.format = pp::renderer::TextureFormat::rgba8; + + pp::renderer::RecordedRenderCommand transition_command; + transition_command.kind = pp::renderer::RecordedRenderCommandKind::transition_texture; + transition_command.texture_desc.format = pp::renderer::TextureFormat::rgba8; + transition_command.before_state = pp::renderer::TextureState::upload_destination; + transition_command.after_state = pp::renderer::TextureState::shader_read; + + pp::renderer::RecordedRenderCommand copy_command; + copy_command.kind = pp::renderer::RecordedRenderCommandKind::copy_texture; + copy_command.source_desc.format = pp::renderer::TextureFormat::rgba8; + copy_command.destination_desc.format = pp::renderer::TextureFormat::rgba8; + + pp::renderer::RecordedRenderCommand read_command; + read_command.kind = pp::renderer::RecordedRenderCommandKind::read_texture; + read_command.texture_desc.format = pp::renderer::TextureFormat::rgba8; + + pp::renderer::RecordedRenderCommand capture_command; + capture_command.kind = pp::renderer::RecordedRenderCommandKind::capture_frame; + capture_command.target_desc.format = pp::renderer::TextureFormat::rgba8; + + expect_inside_pass_order_error( + h, + upload_command, + pp::renderer::gl::OpenGlPlannedCommandKind::upload_texture); + expect_inside_pass_order_error( + h, + mipmap_command, + pp::renderer::gl::OpenGlPlannedCommandKind::generate_mipmaps); + expect_inside_pass_order_error( + h, + transition_command, + pp::renderer::gl::OpenGlPlannedCommandKind::transition_texture); + expect_inside_pass_order_error( + h, + copy_command, + pp::renderer::gl::OpenGlPlannedCommandKind::copy_texture); + expect_inside_pass_order_error( + h, + read_command, + pp::renderer::gl::OpenGlPlannedCommandKind::read_texture); + expect_inside_pass_order_error( + h, + capture_command, + pp::renderer::gl::OpenGlPlannedCommandKind::capture_frame); + expect_inside_pass_order_error( + h, + blit_command(pp::renderer::BlitFilter::nearest), + pp::renderer::gl::OpenGlPlannedCommandKind::blit_render_target); +} + void flags_broken_render_pass_command_order(pp::tests::Harness& h) { const std::vector outside_pass { @@ -650,6 +730,7 @@ int main() harness.run("names_planned_command_kinds", names_planned_command_kinds); harness.run("plans_valid_recorded_command_streams", plans_valid_recorded_command_streams); harness.run("counts_texture_and_sampler_bindings_in_streams", counts_texture_and_sampler_bindings_in_streams); + harness.run("flags_texture_io_inside_render_pass", flags_texture_io_inside_render_pass); harness.run("flags_broken_render_pass_command_order", flags_broken_render_pass_command_order); harness.run("flags_missing_render_dependencies", flags_missing_render_dependencies); harness.run("tracks_unsupported_commands_in_streams", tracks_unsupported_commands_in_streams);