Skip to content

Add support for RenderDoc API to offload test suite to capture for Vulkan on Windows #651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
spall wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add support for RenderDoc API to offload test suite to capture for Vulkan on Windows #651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ configure_file(
${CMAKE_CURRENT_BINARY_DIR}/include/Config.h)

set(DIRECTX_HEADERS_PATH ${CMAKE_CURRENT_SOURCE_DIR}/third-party/DirectX-Headers)
set(RENDERDOC_HEADERS_PATH ${CMAKE_CURRENT_SOURCE_DIR}/third-party/Renderdoc)

if (WIN32)
message(STATUS "Including vendored zlib")
Expand Down
2 changes: 1 addition & 1 deletion include/API/Device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Device {
virtual const Capabilities &getCapabilities() = 0;
virtual llvm::StringRef getAPIName() const = 0;
virtual GPUAPI getAPI() const = 0;
virtual llvm::Error executeProgram(Pipeline &P) = 0;
virtual llvm::Error executeProgram(Pipeline &P, bool Capture = false) = 0;
virtual void printExtra(llvm::raw_ostream &OS) {}

virtual ~Device() = 0;
Expand Down
1 change: 1 addition & 0 deletions lib/API/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
list(APPEND api_headers PRIVATE ${RENDERDOC_HEADERS_PATH})
if (OFFLOADTEST_ENABLE_VULKAN)
list(APPEND api_sources VK/Device.cpp)
list(APPEND api_libraries ${Vulkan_LIBRARIES})
Expand Down
2 changes: 1 addition & 1 deletion lib/API/DX/Device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ class DXDevice : public offloadtest::Device {
return llvm::Error::success();
}

llvm::Error executeProgram(Pipeline &P) override {
llvm::Error executeProgram(Pipeline &P, bool Capture = false) override {
llvm::sys::AddSignalHandler(
[](void *Cookie) {
ID3D12Device *Device = (ID3D12Device *)Cookie;
Expand Down
52 changes: 51 additions & 1 deletion lib/API/VK/Device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
#include "Support/Pipeline.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/Support/Error.h"
#include "renderdoc_app.h"

#ifdef _WIN32
#include <windows.h>
#endif

#include <memory>
#include <numeric>
Expand Down Expand Up @@ -1951,10 +1956,36 @@ class VKDevice : public offloadtest::Device {
return llvm::Error::success();
}

llvm::Error executeProgram(Pipeline &P) override {
llvm::Error executeProgram(Pipeline &P, bool Capture = false) override {
RENDERDOC_API_1_4_1 *RDocAPI = NULL;
if (Capture) {
// Initialize RenderDoc on Windows
#if defined(_WIN32)
if (HMODULE Mod = GetModuleHandleA("renderdoc.dll")) {
Copy link
Collaborator

@damyanp damyanp Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leaks the module. I don't know if we care about that though.

pRENDERDOC_GetAPI RENDERDOC_GetAPI =
(pRENDERDOC_GetAPI)GetProcAddress(Mod, "RENDERDOC_GetAPI");
int Ret = RENDERDOC_GetAPI(eRENDERDOC_API_Version_1_4_1, (void **)&RDocAPI);
assert(Ret == 1);
llvm::outs() << "RenderDoc API initialized.\n";
Copy link
Collaborator

@damyanp damyanp Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really check the return code from GetAPI - so if the user has, say, an older version of renderdoc.dll then it'll happily say "RenderDoc API initialized" and then not take a capture.

Related to this, I don't think we want to handle runtime errors like this with an assert.

} else {
DWORD error = GetLastError();
llvm::outs() << "Failed to get renderdoc dll: " << error << "\n";
Copy link
Collaborator

@damyanp damyanp Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llvm::mapLastWindowsError() might be interesting to look at here. Looks like it should convert to strings as well.

Kinda hard to find the right balance for polish for a feature like this.

}
#endif
}

InvocationState State;
if (auto Err = createDevice(State))
return Err;

// Start capture
if (RDocAPI) {
RDocAPI->StartFrameCapture(NULL, NULL);
llvm::outs() << "RenderDoc frame capture started.\n";
} else if(Capture)
llvm::outs() << "RenderDoc frame capture NOT started because we failed to \
initialize RenderDoc API. It is only supported on Windows.";
Comment on lines +1985 to +1987
Copy link
Collaborator

@damyanp damyanp Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if(Capture)
llvm::outs() << "RenderDoc frame capture NOT started because we failed to \
initialize RenderDoc API. It is only supported on Windows.";
} else if(Capture) {
llvm::outs() << "RenderDoc frame capture NOT started because we failed to \
initialize RenderDoc API. It is only supported on Windows.";
}

I don't know why the diffs on suggestions is so broken these days - anyway suggestion is to add braces around the else if.

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements:

Inconsistent bracing within if/else if/else chains (if one block requires braces, all must)


llvm::outs() << "Physical device created.\n";
if (auto Err = createShaderModules(P, State))
return Err;
Expand Down Expand Up @@ -1998,6 +2029,25 @@ class VKDevice : public offloadtest::Device {
return Err;
llvm::outs() << "Compute pipeline created.\n";

// Stop frame capture
if (RDocAPI) {
uint32_t Ret = RDocAPI->EndFrameCapture(NULL, NULL);
if (Ret) {
llvm::outs() << "RenderDoc frame capture ended.\n";
Copy link
Collaborator

@damyanp damyanp Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paging the tab police!!

uint32_t CaptureID = RDocAPI->GetNumCaptures() - 1;
uint32_t PathLength;
// get PathLength
if (RDocAPI->GetCapture(CaptureID, NULL, &PathLength, NULL)) {
std::string CapturePath(PathLength, ' ');
// populate the path
RDocAPI->GetCapture(CaptureID, CapturePath.data(), NULL, NULL);
llvm::outs() << "The RenderDoc frame capture has been saved to " << CapturePath << "\n";
} else
llvm::outs() << "No capture generated.\n";
} else
llvm::outs() << "Capture failed.\n";
}

if (auto Err = cleanup(State))
return Err;
llvm::outs() << "Cleanup complete.\n";
Expand Down
Loading
Loading