From c58b9a3718dda24c14c4ac1bfcf167d4375ba791 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 2 Jun 2026 15:10:44 +0200 Subject: [PATCH] Add renderer readback command contract --- docs/modernization/build-inventory.md | 8 +- docs/modernization/roadmap.md | 18 +-- src/renderer_api/recording_renderer.cpp | 30 +++++ src/renderer_api/recording_renderer.h | 8 ++ src/renderer_api/renderer_api.cpp | 35 ++++++ src/renderer_api/renderer_api.h | 7 ++ tests/CMakeLists.txt | 2 +- tests/renderer_api/renderer_api_tests.cpp | 130 ++++++++++++++++++++++ tools/pano_cli/main.cpp | 28 +++++ 9 files changed, 255 insertions(+), 11 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 940b99a..62263c5 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -285,10 +285,12 @@ Known local toolchain state: source code reintroduces raw `GL_*`/`WGL_*` constants outside the allowed legacy OpenGL implementation files. - `pp_renderer_api` exposes a headless `RecordingRenderDevice` that validates - command order, records render commands, and records trace markers without a - window or GL context. + command order, readback bounds, and destination buffer sizes, records render + and readback commands, and records trace markers without a window or GL + context. - `pano_cli record-render` exposes the recording renderer through JSON - automation and is covered by `pano_cli_record_render_smoke`. + automation, including readback command/byte totals, and is covered by + `pano_cli_record_render_smoke`. - `pano_cli simulate-document-history` exposes `pp_document::DocumentHistory` apply/undo/redo state through JSON automation and is covered by `pano_cli_simulate_document_history_smoke`. diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 8886070..e7159b4 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -416,8 +416,9 @@ adding new backends. Status: started. `pp_renderer_api` exists as a headless renderer-neutral target with texture descriptor, byte-size, viewport, mesh, readback bounds, command context, render device, shader program descriptor, mesh, render target, -readback, trace interface validation, and the canonical PanoPainter shader -catalog now consumed by the legacy OpenGL app initialization path. +readback byte-size helpers, readback command validation, trace interface +validation, and the canonical PanoPainter shader catalog now consumed by the +legacy OpenGL app initialization path. `pp_renderer_gl` now exists as the first OpenGL backend library and owns pure OpenGL capability detection for framebuffer fetch, map-buffer alignment, and float texture support. It also owns the OpenGL texture upload-type mapping used @@ -716,7 +717,9 @@ Results: per-layer frame duration, and PNG-encoded face-payload export to PPI bytes, plus malformed payload rejection at the export boundary. - `pp_renderer_api_tests` passed, including shader descriptor validation, - PanoPainter shader catalog validation, and invalid catalog rejection. + PanoPainter shader catalog validation, readback byte-size and command-order + validation, recording readback command capture, and invalid catalog + rejection. - `pp_paint_renderer_compositor_tests` passed. - `pp_ui_core_color_tests` passed. - `pp_ui_core_layout_value_tests` passed. @@ -810,11 +813,12 @@ Results: reintroduces raw `GL_*`/`WGL_*` constants outside the allowed legacy OpenGL implementation files. - `pp_renderer_api` now includes a headless `RecordingRenderDevice` with strict - command-order validation and command/trace capture, giving automation a - backend-neutral render path that does not require a window or GL context. + command-order/readback validation and command/trace/readback capture, giving + automation a backend-neutral render path that does not require a window or GL + context. - `pano_cli record-render` exercises that headless recording renderer and emits - JSON command counts, target dimensions, backend name, and trace/draw summary - for agent automation. + JSON command counts, target dimensions, backend name, trace/draw summary, and + readback command/byte totals for agent automation. - `pano_cli simulate-document-history` exercises pure document history apply/undo/redo behavior and emits JSON layer/frame/history state for agent automation. diff --git a/src/renderer_api/recording_renderer.cpp b/src/renderer_api/recording_renderer.cpp index 160a403..e7d8a32 100644 --- a/src/renderer_api/recording_renderer.cpp +++ b/src/renderer_api/recording_renderer.cpp @@ -174,6 +174,34 @@ pp::foundation::Status RecordingCommandContext::draw() noexcept return pp::foundation::Status::success(); } +pp::foundation::Status RecordingCommandContext::read_texture( + ITexture2D& texture, + ReadbackRegion region, + IReadbackBuffer& destination) noexcept +{ + if (in_render_pass_) { + return pp::foundation::Status::invalid_argument("readback must be outside a render pass"); + } + + const auto desc = texture.desc(); + const auto bytes = readback_byte_size(desc, region); + if (!bytes) { + return bytes.status(); + } + + if (destination.size_bytes() < bytes.value()) { + return pp::foundation::Status::out_of_range("readback buffer is too small"); + } + + push_command(commands_, RecordedRenderCommand { + .kind = RecordedRenderCommandKind::read_texture, + .texture_desc = desc, + .readback_region = region, + .readback_bytes = bytes.value(), + }); + return pp::foundation::Status::success(); +} + void RecordingCommandContext::end_render_pass() noexcept { if (!in_render_pass_) { @@ -251,6 +279,8 @@ const char* recorded_render_command_kind_name(RecordedRenderCommandKind kind) no return "bind_mesh"; case RecordedRenderCommandKind::draw: return "draw"; + case RecordedRenderCommandKind::read_texture: + return "read_texture"; case RecordedRenderCommandKind::end_render_pass: return "end_render_pass"; case RecordedRenderCommandKind::trace_marker: diff --git a/src/renderer_api/recording_renderer.h b/src/renderer_api/recording_renderer.h index d5cbcf6..ae59636 100644 --- a/src/renderer_api/recording_renderer.h +++ b/src/renderer_api/recording_renderer.h @@ -13,6 +13,7 @@ enum class RecordedRenderCommandKind : std::uint8_t { bind_shader, bind_mesh, draw, + read_texture, end_render_pass, trace_marker, }; @@ -23,6 +24,9 @@ struct RecordedRenderCommand { ClearColor clear_color {}; Viewport viewport {}; MeshDesc mesh_desc {}; + TextureDesc texture_desc {}; + ReadbackRegion readback_region {}; + std::uint64_t readback_bytes = 0; const char* component = ""; const char* name = ""; }; @@ -83,6 +87,10 @@ public: [[nodiscard]] pp::foundation::Status bind_shader(IShaderProgram& shader) noexcept override; [[nodiscard]] pp::foundation::Status bind_mesh(IMesh& mesh) noexcept override; [[nodiscard]] pp::foundation::Status draw() noexcept override; + [[nodiscard]] pp::foundation::Status read_texture( + ITexture2D& texture, + ReadbackRegion region, + IReadbackBuffer& destination) noexcept override; void end_render_pass() noexcept override; [[nodiscard]] bool in_render_pass() const noexcept; diff --git a/src/renderer_api/renderer_api.cpp b/src/renderer_api/renderer_api.cpp index d1c27ec..9353fe9 100644 --- a/src/renderer_api/renderer_api.cpp +++ b/src/renderer_api/renderer_api.cpp @@ -196,6 +196,41 @@ pp::foundation::Status validate_readback_region(TextureDesc desc, ReadbackRegion return pp::foundation::Status::success(); } +pp::foundation::Result readback_byte_size(TextureDesc desc, ReadbackRegion region) noexcept +{ + const auto region_status = validate_readback_region(desc, region); + if (!region_status.ok()) { + return pp::foundation::Result::failure(region_status); + } + + const auto bpp = static_cast(bytes_per_pixel(desc.format)); + if (bpp == 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("texture format is not supported")); + } + + const auto width = static_cast(region.width); + const auto height = static_cast(region.height); + if (width > std::numeric_limits::max() / height) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("readback pixel count overflows uint64")); + } + + const auto pixels = width * height; + if (pixels > std::numeric_limits::max() / bpp) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("readback byte size overflows uint64")); + } + + const auto bytes = pixels * bpp; + if (bytes > max_texture_bytes) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("readback byte size exceeds the configured limit")); + } + + return pp::foundation::Result::success(bytes); +} + const char* texture_format_name(TextureFormat format) noexcept { switch (format) { diff --git a/src/renderer_api/renderer_api.h b/src/renderer_api/renderer_api.h index 29a5186..4f0ce22 100644 --- a/src/renderer_api/renderer_api.h +++ b/src/renderer_api/renderer_api.h @@ -122,6 +122,10 @@ public: [[nodiscard]] virtual pp::foundation::Status bind_shader(IShaderProgram& shader) noexcept = 0; [[nodiscard]] virtual pp::foundation::Status bind_mesh(IMesh& mesh) noexcept = 0; [[nodiscard]] virtual pp::foundation::Status draw() noexcept = 0; + [[nodiscard]] virtual pp::foundation::Status read_texture( + ITexture2D& texture, + ReadbackRegion region, + IReadbackBuffer& destination) noexcept = 0; virtual void end_render_pass() noexcept = 0; }; @@ -139,6 +143,9 @@ public: [[nodiscard]] pp::foundation::Status validate_mesh_desc(MeshDesc desc) noexcept; [[nodiscard]] pp::foundation::Status validate_shader_program_desc(ShaderProgramDesc desc) noexcept; [[nodiscard]] pp::foundation::Result texture_byte_size(TextureDesc desc) noexcept; +[[nodiscard]] pp::foundation::Result readback_byte_size( + TextureDesc desc, + ReadbackRegion region) noexcept; [[nodiscard]] pp::foundation::Status validate_readback_region(TextureDesc desc, ReadbackRegion region) noexcept; [[nodiscard]] const char* texture_format_name(TextureFormat format) noexcept; [[nodiscard]] const char* primitive_topology_name(PrimitiveTopology topology) noexcept; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6039df3..2ad1a19 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -365,7 +365,7 @@ if(TARGET pano_cli) COMMAND pano_cli record-render --width 32 --height 16) set_tests_properties(pano_cli_record_render_smoke PROPERTIES LABELS "renderer;integration;desktop-fast" - PASS_REGULAR_EXPRESSION "\"backend\":\"recording\".*\"width\":32.*\"height\":16.*\"commands\":7.*\"drawCommands\":1") + PASS_REGULAR_EXPRESSION "\"backend\":\"recording\".*\"width\":32.*\"height\":16.*\"commands\":8.*\"drawCommands\":1.*\"readbackCommands\":1.*\"readbackBytes\":2048") add_test(NAME pano_cli_simulate_document_edits_smoke COMMAND pano_cli simulate-document-edits --width 128 --height 64) diff --git a/tests/renderer_api/renderer_api_tests.cpp b/tests/renderer_api/renderer_api_tests.cpp index fa11586..ee28ada 100644 --- a/tests/renderer_api/renderer_api_tests.cpp +++ b/tests/renderer_api/renderer_api_tests.cpp @@ -20,9 +20,11 @@ using pp::renderer::PrimitiveTopology; using pp::renderer::ReadbackRegion; using pp::renderer::RecordedRenderCommandKind; using pp::renderer::RecordingMesh; +using pp::renderer::RecordingReadbackBuffer; using pp::renderer::RecordingRenderDevice; using pp::renderer::RecordingRenderTarget; using pp::renderer::RecordingShaderProgram; +using pp::renderer::RecordingTexture2D; using pp::renderer::ShaderProgramDesc; using pp::renderer::ShaderStageSource; using pp::renderer::TextureDesc; @@ -32,6 +34,7 @@ using pp::renderer::max_shader_source_bytes; using pp::renderer::max_texture_dimension; using pp::renderer::panopainter_shader_catalog; using pp::renderer::primitive_topology_name; +using pp::renderer::readback_byte_size; using pp::renderer::recorded_render_command_kind_name; using pp::renderer::ShaderCatalogEntry; using pp::renderer::texture_byte_size; @@ -73,6 +76,34 @@ public: } }; +class FakeTexture final : public pp::renderer::ITexture2D { +public: + [[nodiscard]] TextureDesc desc() const noexcept override + { + return TextureDesc { + .extent = Extent2D { .width = 64, .height = 32 }, + .format = TextureFormat::rgba8, + .render_target = true, + }; + } +}; + +class FakeReadbackBuffer final : public pp::renderer::IReadbackBuffer { +public: + explicit FakeReadbackBuffer(std::uint64_t size) noexcept + : size_(size) + { + } + + [[nodiscard]] std::uint64_t size_bytes() const noexcept override + { + return size_; + } + +private: + std::uint64_t size_ = 0; +}; + class FakeTrace final : public IRenderTrace { public: void marker(const char* component, const char* name) noexcept override @@ -120,6 +151,22 @@ public: : pp::foundation::Status::invalid_argument("render pass has not begun"); } + [[nodiscard]] pp::foundation::Status read_texture( + pp::renderer::ITexture2D& texture, + ReadbackRegion region, + pp::renderer::IReadbackBuffer& destination) noexcept override + { + const auto bytes = readback_byte_size(texture.desc(), region); + if (!bytes) { + return bytes.status(); + } + if (destination.size_bytes() < bytes.value()) { + return pp::foundation::Status::out_of_range("readback buffer is too small"); + } + last_readback_bytes = bytes.value(); + return pp::foundation::Status::success(); + } + void end_render_pass() noexcept override { in_render_pass = false; @@ -127,6 +174,7 @@ public: bool in_render_pass = false; const char* shader_name = nullptr; + std::uint64_t last_readback_bytes = 0; }; class FakeRenderDevice final : public IRenderDevice { @@ -208,6 +256,31 @@ void validates_readback_bounds(pp::tests::Harness& h) PP_EXPECT(h, overrun.code == StatusCode::out_of_range); } +void computes_readback_byte_sizes(pp::tests::Harness& h) +{ + const TextureDesc rgba_desc { + .extent = Extent2D { .width = 64, .height = 32 }, + .format = TextureFormat::rgba8, + .render_target = true, + }; + const TextureDesc r8_desc { + .extent = Extent2D { .width = 64, .height = 32 }, + .format = TextureFormat::r8, + .render_target = true, + }; + + const auto rgba = readback_byte_size(rgba_desc, ReadbackRegion { .x = 4, .y = 2, .width = 8, .height = 3 }); + const auto r8 = readback_byte_size(r8_desc, ReadbackRegion { .x = 0, .y = 0, .width = 5, .height = 7 }); + const auto overrun = readback_byte_size(rgba_desc, ReadbackRegion { .x = 63, .y = 0, .width = 2, .height = 1 }); + + PP_EXPECT(h, rgba.ok()); + PP_EXPECT(h, rgba.value() == 96U); + PP_EXPECT(h, r8.ok()); + PP_EXPECT(h, r8.value() == 35U); + PP_EXPECT(h, !overrun.ok()); + PP_EXPECT(h, overrun.status().code == StatusCode::out_of_range); +} + void validates_viewports_and_mesh_descriptors(pp::tests::Harness& h) { const Extent2D target { .width = 64, .height = 32 }; @@ -349,6 +422,8 @@ void renderer_interfaces_support_backend_neutral_dispatch(pp::tests::Harness& h) { FakeRenderDevice device; FakeRenderTarget target; + FakeTexture texture; + FakeReadbackBuffer readback_buffer(64U * 32U * 4U); FakeShaderProgram shader; FakeMesh mesh; @@ -368,12 +443,24 @@ void renderer_interfaces_support_backend_neutral_dispatch(pp::tests::Harness& h) const auto draw_after_end = context.draw(); PP_EXPECT(h, !draw_after_end.ok()); PP_EXPECT(h, draw_after_end.code == StatusCode::invalid_argument); + PP_EXPECT(h, context.read_texture( + texture, + ReadbackRegion { .x = 2, .y = 3, .width = 4, .height = 5 }, + readback_buffer) + .ok()); PP_EXPECT(h, device.context.shader_name == std::string_view("fake-shader")); + PP_EXPECT(h, device.context.last_readback_bytes == 80U); } void recording_renderer_records_valid_command_sequences(pp::tests::Harness& h) { RecordingRenderDevice device; + RecordingTexture2D texture(TextureDesc { + .extent = Extent2D { .width = 64, .height = 32 }, + .format = TextureFormat::rgba8, + .render_target = true, + }); + RecordingReadbackBuffer readback_buffer(64U * 32U * 4U); RecordingRenderTarget target(TextureDesc { .extent = Extent2D { .width = 64, .height = 32 }, .format = TextureFormat::rgba8, @@ -411,6 +498,20 @@ void recording_renderer_records_valid_command_sequences(pp::tests::Harness& h) PP_EXPECT(h, commands[6].kind == RecordedRenderCommandKind::end_render_pass); PP_EXPECT(h, recorded_render_command_kind_name(commands[5].kind) == std::string_view("draw")); + PP_EXPECT(h, context.read_texture( + texture, + ReadbackRegion { .x = 4, .y = 5, .width = 8, .height = 3 }, + readback_buffer) + .ok()); + const auto commands_after_readback = device.commands(); + PP_EXPECT(h, commands_after_readback.size() == 8U); + PP_EXPECT(h, commands_after_readback[7].kind == RecordedRenderCommandKind::read_texture); + PP_EXPECT(h, commands_after_readback[7].texture_desc.extent.width == 64U); + PP_EXPECT(h, commands_after_readback[7].readback_region.x == 4U); + PP_EXPECT(h, commands_after_readback[7].readback_region.height == 3U); + PP_EXPECT(h, commands_after_readback[7].readback_bytes == 96U); + PP_EXPECT(h, recorded_render_command_kind_name(commands_after_readback[7].kind) == std::string_view("read_texture")); + device.clear(); PP_EXPECT(h, device.commands().empty()); } @@ -428,6 +529,13 @@ void recording_renderer_rejects_invalid_command_order_and_targets(pp::tests::Har .format = TextureFormat::rgba8, .render_target = false, }); + RecordingTexture2D texture(TextureDesc { + .extent = Extent2D { .width = 32, .height = 16 }, + .format = TextureFormat::rgba8, + .render_target = true, + }); + RecordingReadbackBuffer small_readback_buffer(3U); + RecordingReadbackBuffer full_readback_buffer(32U * 16U * 4U); RecordingShaderProgram shader("strict-shader"); RecordingMesh mesh(MeshDesc { .vertex_count = 3, .topology = PrimitiveTopology::triangles }); RecordingMesh empty_mesh(MeshDesc {}); @@ -443,6 +551,13 @@ void recording_renderer_rejects_invalid_command_order_and_targets(pp::tests::Har PP_EXPECT(h, device.commands().empty()); PP_EXPECT(h, context.begin_render_pass(target, ClearColor {}).ok()); + const auto read_during_render_pass = context.read_texture( + texture, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }, + full_readback_buffer); + PP_EXPECT(h, !read_during_render_pass.ok()); + PP_EXPECT(h, read_during_render_pass.code == StatusCode::invalid_argument); + const auto nested_begin = context.begin_render_pass(target, ClearColor {}); PP_EXPECT(h, !nested_begin.ok()); PP_EXPECT(h, nested_begin.code == StatusCode::invalid_argument); @@ -467,6 +582,20 @@ void recording_renderer_rejects_invalid_command_order_and_targets(pp::tests::Har const auto viewport_after_end = context.set_viewport(Viewport { .x = 0, .y = 0, .width = 1, .height = 1 }); PP_EXPECT(h, !viewport_after_end.ok()); PP_EXPECT(h, viewport_after_end.code == StatusCode::invalid_argument); + + const auto read_outside_bounds = context.read_texture( + texture, + ReadbackRegion { .x = 31, .y = 15, .width = 2, .height = 1 }, + full_readback_buffer); + PP_EXPECT(h, !read_outside_bounds.ok()); + PP_EXPECT(h, read_outside_bounds.code == StatusCode::out_of_range); + + const auto read_into_small_buffer = context.read_texture( + texture, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }, + small_readback_buffer); + PP_EXPECT(h, !read_into_small_buffer.ok()); + PP_EXPECT(h, read_into_small_buffer.code == StatusCode::out_of_range); } } @@ -477,6 +606,7 @@ int main() harness.run("computes_texture_sizes", computes_texture_sizes); harness.run("rejects_invalid_or_excessive_extents", rejects_invalid_or_excessive_extents); harness.run("validates_readback_bounds", validates_readback_bounds); + harness.run("computes_readback_byte_sizes", computes_readback_byte_sizes); harness.run("validates_viewports_and_mesh_descriptors", validates_viewports_and_mesh_descriptors); harness.run("validates_shader_program_descriptors", validates_shader_program_descriptors); harness.run("validates_panopainter_shader_catalog", validates_panopainter_shader_catalog); diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index 1d3d0fc..7e18faa 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -2202,11 +2202,18 @@ int record_render(int argc, char** argv) } pp::renderer::RecordingRenderDevice device; + pp::renderer::RecordingTexture2D texture(pp::renderer::TextureDesc { + .extent = pp::renderer::Extent2D { .width = args.width, .height = args.height }, + .format = pp::renderer::TextureFormat::rgba8, + .render_target = true, + }); pp::renderer::RecordingRenderTarget target(pp::renderer::TextureDesc { .extent = pp::renderer::Extent2D { .width = args.width, .height = args.height }, .format = pp::renderer::TextureFormat::rgba8, .render_target = true, }); + pp::renderer::RecordingReadbackBuffer readback_buffer( + static_cast(args.width) * args.height * 4U); pp::renderer::RecordingShaderProgram shader("pano-cli-record-render"); pp::renderer::RecordingMesh mesh(pp::renderer::MeshDesc { .vertex_count = 3, @@ -2249,12 +2256,31 @@ int record_render(int argc, char** argv) return 2; } + const auto readback_status = context.read_texture( + texture, + pp::renderer::ReadbackRegion { + .x = 0, + .y = 0, + .width = args.width, + .height = args.height, + }, + readback_buffer); + if (!readback_status.ok()) { + print_error("record-render", readback_status.message); + return 2; + } + std::size_t draw_commands = 0; + std::size_t readback_commands = 0; std::size_t trace_markers = 0; + std::uint64_t readback_bytes = 0; const auto commands = device.commands(); for (const auto& command : commands) { if (command.kind == pp::renderer::RecordedRenderCommandKind::draw) { ++draw_commands; + } else if (command.kind == pp::renderer::RecordedRenderCommandKind::read_texture) { + ++readback_commands; + readback_bytes += command.readback_bytes; } else if (command.kind == pp::renderer::RecordedRenderCommandKind::trace_marker) { ++trace_markers; } @@ -2267,6 +2293,8 @@ int record_render(int argc, char** argv) << ",\"format\":\"rgba8\"}" << ",\"commands\":" << commands.size() << ",\"drawCommands\":" << draw_commands + << ",\"readbackCommands\":" << readback_commands + << ",\"readbackBytes\":" << readback_bytes << ",\"traceMarkers\":" << trace_markers << ",\"first\":\"" << pp::renderer::recorded_render_command_kind_name(commands.front().kind)