From 831e5deeaefffb4e8d65d64dc797c8407b864166 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 2 Jun 2026 18:03:42 +0200 Subject: [PATCH] Validate renderer readback descriptors --- docs/modernization/build-inventory.md | 2 +- docs/modernization/roadmap.md | 2 +- src/renderer_api/renderer_api.cpp | 6 ++-- tests/renderer_api/renderer_api_tests.cpp | 43 +++++++++++++++++++++++ 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 0a03c15..912184a 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -303,7 +303,7 @@ Known local toolchain state: blend state, texture-slot binding, sampler-state binding, texture-upload byte counts, texture mip-level counts, texture/mesh/shader resource debug labels, mipmap-generation commands, texture-state transitions, shader-uniform writes, explicit draw descriptor ranges, texture-copy regions, - readback bounds, frame-capture sources, destination buffer sizes, and + readback descriptor/bounds, frame-capture sources, destination buffer sizes, and render-target blit regions, records render-pass-clear/scissor/depth/blend/shader-uniform/texture-bind/ sampler-bind/draw/upload/mipmap-generation/texture-transition/texture-copy/readback/ diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index c122817..5810215 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -428,7 +428,7 @@ descriptor, mesh, render target, readback byte-size helpers, texture mip-level validation, resource debug-label validation, texture-upload/readback command validation, mipmap-generation command validation, texture-state transition validation, frame-capture byte-size helpers, -frame-capture command validation, render-target blit validation, texture-slot +readback/copy descriptor validation, frame-capture command validation, render-target blit validation, texture-slot binding validation, blend-state validation, scissor-state validation, depth-state validation, trace marker/scope validation, sampler-state validation, texture/mesh/shader resource-label validation, diff --git a/src/renderer_api/renderer_api.cpp b/src/renderer_api/renderer_api.cpp index 3b63213..2b1831b 100644 --- a/src/renderer_api/renderer_api.cpp +++ b/src/renderer_api/renderer_api.cpp @@ -567,9 +567,9 @@ pp::foundation::Status validate_trace_label(const char* component, const char* n pp::foundation::Status validate_readback_region(TextureDesc desc, ReadbackRegion region) noexcept { - const auto extent_status = validate_extent(desc.extent); - if (!extent_status.ok()) { - return extent_status; + const auto desc_status = validate_texture_desc(desc); + if (!desc_status.ok()) { + return desc_status; } if (region.width == 0 || region.height == 0) { diff --git a/tests/renderer_api/renderer_api_tests.cpp b/tests/renderer_api/renderer_api_tests.cpp index d15de6f..01e52ad 100644 --- a/tests/renderer_api/renderer_api_tests.cpp +++ b/tests/renderer_api/renderer_api_tests.cpp @@ -1059,6 +1059,11 @@ void validates_readback_bounds(pp::tests::Harness& h) .format = TextureFormat::rgba8, .usage = TextureUsage::render_target | TextureUsage::sampled | TextureUsage::upload_destination | TextureUsage::readback_source | TextureUsage::copy_source | TextureUsage::copy_destination, }; + const TextureDesc unsupported_usage_desc { + .extent = Extent2D { .width = 64, .height = 32 }, + .format = TextureFormat::rgba8, + .usage = TextureUsage::readback_source | static_cast(1U << 31U), + }; PP_EXPECT(h, validate_readback_region(desc, ReadbackRegion { .x = 0, .y = 0, .width = 64, .height = 32 }).ok()); PP_EXPECT(h, validate_readback_region(desc, ReadbackRegion { .x = 63, .y = 31, .width = 1, .height = 1 }).ok()); @@ -1066,6 +1071,9 @@ void validates_readback_bounds(pp::tests::Harness& h) const auto empty = validate_readback_region(desc, ReadbackRegion { .x = 0, .y = 0, .width = 0, .height = 1 }); const auto origin_outside = validate_readback_region(desc, ReadbackRegion { .x = 65, .y = 0, .width = 1, .height = 1 }); const auto overrun = validate_readback_region(desc, ReadbackRegion { .x = 63, .y = 31, .width = 2, .height = 1 }); + const auto unsupported_usage = validate_readback_region( + unsupported_usage_desc, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }); PP_EXPECT(h, !empty.ok()); PP_EXPECT(h, empty.code == StatusCode::invalid_argument); @@ -1073,6 +1081,8 @@ void validates_readback_bounds(pp::tests::Harness& h) PP_EXPECT(h, origin_outside.code == StatusCode::out_of_range); PP_EXPECT(h, !overrun.ok()); PP_EXPECT(h, overrun.code == StatusCode::out_of_range); + PP_EXPECT(h, !unsupported_usage.ok()); + PP_EXPECT(h, unsupported_usage.code == StatusCode::invalid_argument); } void computes_readback_byte_sizes(pp::tests::Harness& h) @@ -1087,10 +1097,18 @@ void computes_readback_byte_sizes(pp::tests::Harness& h) .format = TextureFormat::r8, .usage = TextureUsage::render_target | TextureUsage::sampled | TextureUsage::upload_destination | TextureUsage::readback_source | TextureUsage::copy_source | TextureUsage::copy_destination, }; + const TextureDesc unknown_format_desc { + .extent = Extent2D { .width = 64, .height = 32 }, + .format = static_cast(255), + .usage = TextureUsage::render_target | TextureUsage::sampled | TextureUsage::upload_destination | TextureUsage::readback_source | TextureUsage::copy_source | TextureUsage::copy_destination, + }; 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 }); + const auto unknown_format = readback_byte_size( + unknown_format_desc, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }); PP_EXPECT(h, rgba.ok()); PP_EXPECT(h, rgba.value() == 96U); @@ -1098,6 +1116,8 @@ void computes_readback_byte_sizes(pp::tests::Harness& h) PP_EXPECT(h, r8.value() == 35U); PP_EXPECT(h, !overrun.ok()); PP_EXPECT(h, overrun.status().code == StatusCode::out_of_range); + PP_EXPECT(h, !unknown_format.ok()); + PP_EXPECT(h, unknown_format.status().code == StatusCode::invalid_argument); } void computes_frame_capture_byte_sizes(pp::tests::Harness& h) @@ -1168,6 +1188,15 @@ void validates_texture_copy_contract(pp::tests::Harness& h) .extent = Extent2D { .width = 16, .height = 8 }, .format = TextureFormat::r8, }; + const TextureDesc unknown_format_desc { + .extent = Extent2D { .width = 16, .height = 8 }, + .format = static_cast(255), + }; + const TextureDesc unsupported_usage_desc { + .extent = Extent2D { .width = 16, .height = 8 }, + .format = TextureFormat::rgba8, + .usage = TextureUsage::copy_source | static_cast(1U << 31U), + }; PP_EXPECT(h, validate_texture_copy_descs( rgba_desc, @@ -1191,6 +1220,16 @@ void validates_texture_copy_contract(pp::tests::Harness& h) ReadbackRegion { .x = 15, .y = 0, .width = 2, .height = 1 }, rgba_desc, ReadbackRegion { .x = 0, .y = 0, .width = 2, .height = 1 }); + const auto invalid_format_endpoints = validate_texture_copy_descs( + unknown_format_desc, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }, + unknown_format_desc, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }); + const auto unsupported_source_usage = validate_texture_copy_descs( + unsupported_usage_desc, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }, + rgba_desc, + ReadbackRegion { .x = 0, .y = 0, .width = 1, .height = 1 }); PP_EXPECT(h, !mismatched_format.ok()); PP_EXPECT(h, mismatched_format.code == StatusCode::invalid_argument); @@ -1198,6 +1237,10 @@ void validates_texture_copy_contract(pp::tests::Harness& h) PP_EXPECT(h, mismatched_region.code == StatusCode::invalid_argument); PP_EXPECT(h, !outside_source.ok()); PP_EXPECT(h, outside_source.code == StatusCode::out_of_range); + PP_EXPECT(h, !invalid_format_endpoints.ok()); + PP_EXPECT(h, invalid_format_endpoints.code == StatusCode::invalid_argument); + PP_EXPECT(h, !unsupported_source_usage.ok()); + PP_EXPECT(h, unsupported_source_usage.code == StatusCode::invalid_argument); } void validates_blend_contract(pp::tests::Harness& h)