diff --git a/src/graphics/shader/shader.cpp b/src/graphics/shader/shader.cpp index ef961b5c5..1f4e49f32 100644 --- a/src/graphics/shader/shader.cpp +++ b/src/graphics/shader/shader.cpp @@ -40,9 +40,15 @@ namespace Libs::Graphics { +// A mapped shader and its vertex metadata, parsed once when it is mapped. +struct ShaderMapData: ShaderMappedData { + ShaderVertexMetadata vertex; + const char* vertex_error = nullptr; +}; + struct ShaderMapEntry { - ShaderMappedData data; - uint64_t hash; + ShaderMapData data; + uint64_t hash; }; static std::unique_ptr> g_shader_map; @@ -64,8 +70,11 @@ void ShaderMapUserData(uint64_t addr, const ShaderMappedData& data) { addr, data.code_size_bytes); } const auto hash = XXH3_64bits(reinterpret_cast(addr), data.code_size_bytes); + ShaderMapData mapped {data}; + mapped.vertex_error = + ShaderReadVertexMetadata(data, HW::UserSgprInfo::SGPRS_MAX, mapped.vertex); std::scoped_lock lock(g_shader_map_mutex); - (*g_shader_map)[addr] = {data, hash}; + (*g_shader_map)[addr] = {mapped, hash}; } static ShaderMapEntry ShaderGetMappedData(uint64_t addr, const char* label) { @@ -261,8 +270,8 @@ static uint32_t ShaderCalcPsSystemInputBase(const HW::ShaderRegisters& regs) { static bool ShaderGetStaticVertexInputInfo(uint64_t shader_addr, const HW::UserSgprInfo& user_sgpr, uint32_t user_sgpr_num, const HW::ShaderRegisters& sh, - const ShaderMappedData& data, - ShaderVertexInputInfo& info) { + const ShaderMapData& data, + ShaderVertexInputInfo& info) { KYTY_PROFILER_FUNCTION(); info = {}; @@ -277,13 +286,12 @@ static bool ShaderGetStaticVertexInputInfo(uint64_t shader_addr, const HW::UserS " user_sgpr_num=%u\n", shader_addr, user_sgpr_num); } - ShaderVertexMetadata metadata; - std::string metadata_error; - if (!ShaderReadVertexMetadata(data, HW::UserSgprInfo::SGPRS_MAX, metadata, &metadata_error)) { + if (data.vertex_error != nullptr) { LOGF("ShaderGetVertexInputInfo(): invalid AGC metadata shader=0x%016" PRIx64 ": %s\n", - shader_addr, metadata_error.c_str()); + shader_addr, data.vertex_error); return false; } + const auto& metadata = data.vertex; if (metadata.vertex_attrib_reg >= 0) { info.fetch_external = false; diff --git a/src/graphics/shader/shader.h b/src/graphics/shader/shader.h index b80c31350..8b50133c1 100644 --- a/src/graphics/shader/shader.h +++ b/src/graphics/shader/shader.h @@ -15,17 +15,6 @@ namespace Libs::Graphics { -namespace ShaderError { - -[[nodiscard]] inline bool Fail(std::string* error, std::string_view message) { - if (error != nullptr) { - error->assign(message.data(), message.size()); - } - return false; -} - -} // namespace ShaderError - namespace HW { struct VertexShaderInfo; struct PixelShaderInfo; diff --git a/src/graphics/shader/shaderVertexMetadata.cpp b/src/graphics/shader/shaderVertexMetadata.cpp index 91544edcd..9d5193f6b 100644 --- a/src/graphics/shader/shaderVertexMetadata.cpp +++ b/src/graphics/shader/shaderVertexMetadata.cpp @@ -5,10 +5,10 @@ namespace Libs::Graphics { -bool ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sgprs, - ShaderVertexMetadata& metadata, std::string* error) { +const char* ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sgprs, + ShaderVertexMetadata& metadata) { if (data.user_data == nullptr) { - return ShaderError::Fail(error, "missing AGC user-data header"); + return "missing AGC user-data header"; } ShaderUserData user_data {}; @@ -17,8 +17,7 @@ bool ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sg constexpr uint32_t DirectResourceCount = static_cast(AgcDirectResourceType::Last) + 1u; if (user_data.direct_resource_count > DirectResourceCount) { - return ShaderError::Fail(error, - "AGC direct-resource count exceeds the known resource domain"); + return "AGC direct-resource count exceeds the known resource domain"; } std::array direct_offsets {}; @@ -26,7 +25,7 @@ bool ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sg static_cast(user_data.direct_resource_count) * sizeof(uint16_t); if (direct_size != 0) { if (user_data.direct_resource_offset == nullptr) { - return ShaderError::Fail(error, "missing AGC direct-resource offsets"); + return "missing AGC direct-resource offsets"; } std::memcpy(direct_offsets.data(), user_data.direct_resource_offset, direct_size); } @@ -47,33 +46,33 @@ bool ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sg } if (next.vertex_attrib_reg >= 0 && next.vertex_buffer_reg < 0) { - return ShaderError::Fail(error, "vertex attribute table requires a vertex buffer table"); + return "vertex attribute table requires a vertex buffer table"; } if (next.vertex_buffer_reg < 0) { metadata = next; - return true; + return nullptr; } if (static_cast(next.vertex_buffer_reg) + 1u >= max_user_sgprs) { - return ShaderError::Fail(error, "vertex table pointer exceeds the user-SGPR domain"); + return "vertex table pointer exceeds the user-SGPR domain"; } if (next.vertex_attrib_reg < 0) { metadata = next; - return true; + return nullptr; } if (static_cast(next.vertex_attrib_reg) + 1u >= max_user_sgprs) { - return ShaderError::Fail(error, "vertex table pointer exceeds the user-SGPR domain"); + return "vertex table pointer exceeds the user-SGPR domain"; } if (data.num_input_semantics == 0 || data.num_input_semantics > ShaderVertexInputInfo::RES_MAX) { - return ShaderError::Fail(error, "vertex semantic count is outside the supported domain"); + return "vertex semantic count is outside the supported domain"; } if (data.input_semantics == nullptr) { - return ShaderError::Fail(error, "missing vertex input semantics"); + return "missing vertex input semantics"; } next.input_semantics = {data.input_semantics, data.num_input_semantics}; metadata = next; - return true; + return nullptr; } } // namespace Libs::Graphics diff --git a/src/graphics/shader/shaderVertexMetadata.h b/src/graphics/shader/shaderVertexMetadata.h index 95eb37922..09e1fbe1e 100644 --- a/src/graphics/shader/shaderVertexMetadata.h +++ b/src/graphics/shader/shaderVertexMetadata.h @@ -4,7 +4,6 @@ #include "graphics/shader/shader.h" #include -#include namespace Libs::Graphics { @@ -14,9 +13,10 @@ struct ShaderVertexMetadata { std::span input_semantics; }; -// Validates the vertex metadata and returns a view into the shader header. -bool ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sgprs, - ShaderVertexMetadata& metadata, std::string* error); +// Reads the vertex metadata as a view into the shader header. Returns nullptr if it is +// valid, otherwise why it was rejected. +const char* ShaderReadVertexMetadata(const ShaderMappedData& data, uint32_t max_user_sgprs, + ShaderVertexMetadata& metadata); } // namespace Libs::Graphics diff --git a/tests/ShaderVertexMetadataTests.cpp b/tests/ShaderVertexMetadataTests.cpp index ed07b9c73..bb56176d6 100644 --- a/tests/ShaderVertexMetadataTests.cpp +++ b/tests/ShaderVertexMetadataTests.cpp @@ -40,9 +40,7 @@ void CheckRejected(const ShaderMappedData& data, const char* text) { output.vertex_buffer_reg = 37; output.vertex_attrib_reg = 41; output.input_semantics = prior_semantics; - std::string error; - Check(!ShaderReadVertexMetadata(data, 64, output, &error), text); - Check(!error.empty(), "metadata rejection omitted its diagnostic"); + Check(ShaderReadVertexMetadata(data, 64, output) != nullptr, text); Check(output.vertex_buffer_reg == 37 && output.vertex_attrib_reg == 41 && output.input_semantics.data() == prior_semantics.data() && output.input_semantics.size() == prior_semantics.size(), @@ -52,8 +50,7 @@ void CheckRejected(const ShaderMappedData& data, const char* text) { void TestValidAndInvalidMetadata() { Fixture fixture; ShaderVertexMetadata output; - std::string error; - Check(ShaderReadVertexMetadata(fixture.mapped, 64, output, &error), + Check(ShaderReadVertexMetadata(fixture.mapped, 64, output) == nullptr, "valid AGC vertex metadata was rejected"); Check(output.vertex_buffer_reg == 2 && output.vertex_attrib_reg == 4 && output.input_semantics.size() == 1, @@ -64,7 +61,7 @@ void TestValidAndInvalidMetadata() { AGC_ILLEGAL_DIRECT_OFFSET; buffer_only.mapped.input_semantics = nullptr; buffer_only.mapped.num_input_semantics = 0; - Check(ShaderReadVertexMetadata(buffer_only.mapped, 64, output, &error), + Check(ShaderReadVertexMetadata(buffer_only.mapped, 64, output) == nullptr, "vertex-buffer-only AGC metadata was rejected"); Check(output.vertex_buffer_reg == 2 && output.vertex_attrib_reg == -1 && output.input_semantics.empty(), diff --git a/tests/shaderCfgTests.cpp b/tests/shaderCfgTests.cpp index 96ef6787c..641e48ba6 100644 --- a/tests/shaderCfgTests.cpp +++ b/tests/shaderCfgTests.cpp @@ -1906,6 +1906,13 @@ void TestVertexBufferGrouping() { input.resources[2].Base48() == input.buffers[0].addr, "a later lower-address attribute or changed table left stale vertex offsets"); } + // The vertex table metadata is parsed when the shader is mapped. + user_data.direct_resource_count = 0; + (void)PrepareProgram(regs, HW::Context{}, HW::UserConfig{}, input); + Check(input.fetch_attrib_reg == 2, "a draw reread the vertex metadata of a mapped shader"); + ShaderMapUserData(regs.es_regs.data_addr, mapped); + (void)PrepareProgram(regs, HW::Context{}, HW::UserConfig{}, input); + Check(!input.fetch_embedded, "remapping a shader kept its previous vertex metadata"); } void TestNggVertexEntryState() {