From 1a5f1cc6ab651cfd4444dd0edf783164a28fcc49 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Mon, 10 Feb 2025 23:50:45 -0800 Subject: [PATCH] matdbg: fix three bugs (#8430) 1. changes weren't refreshed for windows/linux-mesa vulkan. This is due to a reordering of the shaders when the shader source is replaced. We fix this by ensuring the ordering before writing out the edit. Fixes #7286 2. A condition was flipped in the SourceFormatter causing the format feature to not work on linux 3. The material update status wasn't really propagated to the front-end due to not updating the status counter. --- libs/matdbg/src/ApiHandler.cpp | 14 +++++++++++--- libs/matdbg/src/ApiHandler.h | 4 +++- libs/matdbg/src/ShaderReplacer.cpp | 26 +++++++++++++++++++++++++- libs/matdbg/src/SourceFormatter.cpp | 2 +- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/libs/matdbg/src/ApiHandler.cpp b/libs/matdbg/src/ApiHandler.cpp index d13161eeac..80d00e2338 100644 --- a/libs/matdbg/src/ApiHandler.cpp +++ b/libs/matdbg/src/ApiHandler.cpp @@ -222,8 +222,13 @@ bool ApiHandler::handleGetApiShader(struct mg_connection* conn, } void ApiHandler::addMaterial(MaterialRecord const* material) { + updateMaterial(material->key); +} + +void ApiHandler::updateMaterial(uint32_t key) { std::unique_lock const lock(mStatusMutex); - snprintf(statusMaterialId, sizeof(statusMaterialId), "%8.8x", material->key); + mCurrentStatus++; + snprintf(statusMaterialId, sizeof(statusMaterialId), "%8.8x", key); mStatusCondition.notify_all(); } @@ -288,8 +293,11 @@ bool ApiHandler::handlePost(CivetServer* server, struct mg_connection* conn) { sstream >> std::hex >> matid >> std::dec >> api >> shaderIndex; std::string const shader = sstream.str().substr(sstream.tellg()); - mServer->handleEditCommand(matid, backend::Backend(api), shaderIndex, shader.c_str(), - shader.size()); + if (!mServer->handleEditCommand(matid, backend::Backend(api), shaderIndex, shader.c_str(), + shader.size())) { + return error(__LINE__, uri); + } + updateMaterial(matid); mg_printf(conn, "HTTP/1.1 200 OK\r\nConnection: close"); return true; diff --git a/libs/matdbg/src/ApiHandler.h b/libs/matdbg/src/ApiHandler.h index 353326a7dc..99c7c15ee8 100644 --- a/libs/matdbg/src/ApiHandler.h +++ b/libs/matdbg/src/ApiHandler.h @@ -55,6 +55,8 @@ private: bool handleGetStatus(struct mg_connection* conn, struct mg_request_info const* request); MaterialRecord const* getMaterialRecord(struct mg_request_info const* request); + void updateMaterial(uint32_t key); + DebugServer* mServer; std::mutex mStatusMutex; @@ -64,7 +66,7 @@ private: // This variable is to implement a *hanging* effect for /api/status. The call to /api/status // will always block until statusMaterialId is updated again. The client is expected to keep // calling /api/status (a constant "pull" to simulate a push). - std::atomic mCurrentStatus = 0; + uint64_t mCurrentStatus = 0; SourceFormatter mFormatter; }; diff --git a/libs/matdbg/src/ShaderReplacer.cpp b/libs/matdbg/src/ShaderReplacer.cpp index 38e4b58229..5c1e97f9bd 100644 --- a/libs/matdbg/src/ShaderReplacer.cpp +++ b/libs/matdbg/src/ShaderReplacer.cpp @@ -25,6 +25,7 @@ #include +#include #include #include @@ -50,10 +51,30 @@ using namespace glslang; using namespace utils; using std::ostream; -using std::stringstream; using std::streampos; +using std::stringstream; using std::vector; + +namespace { + +// This is to ensure that the edited material package have the same order of shaders as the output +// of the original material. We use the ordering in the front-end to simplify logic. +template +void sortRecords(std::vector& records) { + std::sort(records.begin(), records.end(), [](RecordType const& a, RecordType const& b) -> bool { + if (a.shaderModel != b.shaderModel) { + return a.shaderModel < b.shaderModel; + } + if (a.variant.key != b.variant.key) { + return a.variant.key < b.variant.key; + } + return a.stage < b.stage; + }); +} + +} // anonymous + // Tiny database of shader text that can import / export MaterialTextChunk and DictionaryTextChunk. class ShaderIndex { public: @@ -344,6 +365,7 @@ void ShaderIndex::writeChunks(ostream& stream) { for (const auto& record : mShaderRecords) { lines.addText(record.shader); } + sortRecords(mShaderRecords); filamat::ChunkContainer cc; const auto& dchunk = cc.push(std::move(lines), mDictTag); @@ -406,6 +428,8 @@ void BlobIndex::writeChunks(ostream& stream) { } }; + sortRecords(mShaderRecords); + // Apply SMOL-V compression and write out the results. filamat::ChunkContainer cc; cc.push(std::move(mShaderRecords), ChunkType::MaterialSpirv); diff --git a/libs/matdbg/src/SourceFormatter.cpp b/libs/matdbg/src/SourceFormatter.cpp index fe7c3f88ed..9e9c6148e8 100644 --- a/libs/matdbg/src/SourceFormatter.cpp +++ b/libs/matdbg/src/SourceFormatter.cpp @@ -34,7 +34,7 @@ std::string SourceFormatter::format(char const* source) { while (fgets(output, 1024, fp) != NULL) {} int status = pclose(fp); - if (!fp || !WEXITSTATUS(status)) { + if (!fp || WEXITSTATUS(status)) { std::call_once(mClangWarningFlag, []() { utils::slog.w << "[matdbg] unable to run clang-format to format shader file. " << "Please make sure it's installed." << utils::io::endl;