Skip to content

Commit

Permalink
matdbg: fix three bugs (#8430)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
poweifeng authored Feb 11, 2025
1 parent 9f5001d commit 1a5f1cc
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 6 deletions.
14 changes: 11 additions & 3 deletions libs/matdbg/src/ApiHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion libs/matdbg/src/ApiHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class ApiHandler : public CivetHandler {
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;
Expand All @@ -64,7 +66,7 @@ class ApiHandler : public CivetHandler {
// 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<uint64_t> mCurrentStatus = 0;
uint64_t mCurrentStatus = 0;

SourceFormatter mFormatter;
};
Expand Down
26 changes: 25 additions & 1 deletion libs/matdbg/src/ShaderReplacer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include <utils/Log.h>

#include <algorithm>
#include <sstream>

#include <GlslangToSpv.h>
Expand All @@ -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<typename RecordType>
void sortRecords(std::vector<RecordType>& 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:
Expand Down Expand Up @@ -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<DictionaryTextChunk>(std::move(lines), mDictTag);
Expand Down Expand Up @@ -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<MaterialBinaryChunk>(std::move(mShaderRecords), ChunkType::MaterialSpirv);
Expand Down
2 changes: 1 addition & 1 deletion libs/matdbg/src/SourceFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 1a5f1cc

Please sign in to comment.