Skip to content
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.

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

Allow adding debug annotations to OpenGL objects, as well as defining GPU "zones" to achieve functionality very similar to Tracy, but on the GPU. #1845

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 rts/Lua/LuaConstPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ bool LuaConstPlatform::PushEntries(lua_State* L)
LuaPushNamedString(L, "hwConfig", Platform::GetHardwareStr());
LuaPushNamedNumber(L, "cpuLogicalCores", Threading::GetLogicalCpuCores());
LuaPushNamedNumber(L, "cpuPhysicalCores", Threading::GetPhysicalCpuCores());
LuaPushNamedString(L, "cpuBrand", Threading::GetCPUBrand());

LuaPushNamedString(L, "sysInfoHash", Platform::GetSysInfoHash());
LuaPushNamedString(L, "macAddrHash", Platform::GetMacAddrHash());
Expand Down
69 changes: 69 additions & 0 deletions rts/Lua/LuaOpenGL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,11 @@ bool LuaOpenGL::PushEntries(lua_State* L)
REGISTER_LUA_CFUNC(GetWaterRendering);
REGISTER_LUA_CFUNC(GetMapRendering);


REGISTER_LUA_CFUNC(ObjectLabel);
REGISTER_LUA_CFUNC(PushDebugGroup);
REGISTER_LUA_CFUNC(PopDebugGroup);

if (canUseShaders)
LuaShaders::PushEntries(L);

Expand Down Expand Up @@ -5661,5 +5666,69 @@ int LuaOpenGL::GetMapRendering(lua_State* L)
return 0;
}


/**
* @function gl.ObjectLabel labels an object for use with debugging tools
* @param objectTypeIdentifier GLenum Specifies the type of object being labeled.
* @param objectID GLuint Specifies the name or ID of the object to label.
* @param label string A null-terminated string containing the label to be assigned to the object.
* @treturn nil
*/
int LuaOpenGL::ObjectLabel(lua_State* L) {
GLenum identifier = (GLenum)luaL_checkinteger(L, 1);
if (identifier != GL_BUFFER && identifier != GL_SHADER && identifier != GL_PROGRAM &&
identifier != GL_VERTEX_ARRAY && identifier != GL_QUERY && identifier != GL_PROGRAM_PIPELINE &&
identifier != GL_TRANSFORM_FEEDBACK && identifier != GL_TEXTURE && identifier != GL_RENDERBUFFER &&
identifier != GL_FRAMEBUFFER) {
return 0;
}
Comment on lines +5678 to +5684
Copy link
Collaborator

Choose a reason for hiding this comment

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

I found this snippet in the gameside PR:

local GL_PROGRAM = 0x82E2
gl.ObjectLabel(GL_PROGRAM, gl_program_id, self.shaderName)

Sounds bad if the game needs to know those magic 0x82E2 constants to use the function. Would be good if there were either appropriate named constants in LuaConstGL.cpp (GL.PROGRAM etc) or if this accepted a string ("program" etc).

GLuint objectID = (GLuint)luaL_checkinteger(L, 2);
const char* label = luaL_checkstring(L, 3);

// Ensure that identifier is a valid GLenum
// The length should be passed as -1 because the strings are null terminated
// Check that the GLuint name refers to a valid openGL object
// Check that the label is a valid string
Comment on lines +5690 to +5691
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these "check this, check that" comments mean that the code is WIP and those checks should be added?

glObjectLabel(identifier, objectID, -1, label);

return 0;
}


// https://registry.khronos.org/OpenGL-Refpages/gl4/html/glPushDebugGroup.xhtml
/**
* @function gl.PushDebugGroup pushes a debug marker for nVidia nSight 2024.04, does not seem to work when FBO's are raw bound
* @param id GLuint A numeric identifier for the group.
* @param message string A human-readable string describing the debug group.
* @param source boolean true for GL_DEBUG_SOURCE_APPLICATION, false for GL_DEBUG_SOURCE_THIRD_PARTY. default false
* @treturn nil
*/
int LuaOpenGL::PushDebugGroup(lua_State* L) {
GLuint id = (GLuint)luaL_checkinteger(L, 1);
const char* message = luaL_checkstring(L, 2);
bool source = luaL_optboolean(L, 3, false);

GLint maxLength;
glGetIntegerv(GL_MAX_DEBUG_MESSAGE_LENGTH, &maxLength);

if (strlen(message) >= maxLength) {
luaL_error(L, "Message length exceeds GL_MAX_DEBUG_MESSAGE_LENGTH");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps just trim the message to the appropriate length? Otherwise a portable way of preventing this error (GL.MAX_DEBUG_MESSAGE_LENGTH lua constant?) would be nice.

return 0;
}

glPushDebugGroup((source ? GL_DEBUG_SOURCE_APPLICATION: GL_DEBUG_SOURCE_THIRD_PARTY) , id, -1, message);

return 0;
}

/**
* @function gl.PopDebugGroup
* @treturn nil
*/
int LuaOpenGL::PopDebugGroup(lua_State* L) {
glPopDebugGroup();
return 0;
}

/******************************************************************************/
/******************************************************************************/
4 changes: 4 additions & 0 deletions rts/Lua/LuaOpenGL.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,10 @@ class LuaOpenGL {
static int GetSun(lua_State* L);
static int GetWaterRendering(lua_State* L);
static int GetMapRendering(lua_State* L);

static int ObjectLabel(lua_State* L);
static int PushDebugGroup(lua_State* L);
static int PopDebugGroup(lua_State* L);
};

inline void LuaOpenGL::InitMatrixState(lua_State* L, const char* fn) {
Expand Down
4 changes: 3 additions & 1 deletion rts/Lua/LuaShaders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ GLint LuaShaders::GetUniformLocation(LuaShaders::Program* p, const char* name)
* @function gl.CreateShader ( table shaderParams )
* @tparam table shaderParams
* @treturn number shaderID
* @treturn number glProgID
*
* ({[ vertex = "glsl code" ,]
* [ tcs = "glsl code" ,]
Expand Down Expand Up @@ -738,7 +739,8 @@ int LuaShaders::CreateShader(lua_State* L)

// note: index, not raw ID
lua_pushnumber(L, shaders.AddProgram(p));
return 1;
lua_pushnumber(L, (int) prog);
return 2;
}


Expand Down
3 changes: 2 additions & 1 deletion rts/Lua/LuaVBO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ bool LuaVBOs::PushEntries(lua_State* L)
"UnbindBufferRange", &LuaVBOImpl::UnbindBufferRange,

"DumpDefinition", &LuaVBOImpl::DumpDefinition,
"GetBufferSize", &LuaVBOImpl::GetBufferSize
"GetBufferSize", &LuaVBOImpl::GetBufferSize,
"GetID", &LuaVBOImpl::GetID
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be nice if there were getters for every relevant object type.

);

gl.set("VBO", sol::lua_nil); // don't want this to be accessible directly without gl.GetVBO
Expand Down
11 changes: 11 additions & 0 deletions rts/Lua/LuaVBOImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,17 @@ void LuaVBOImpl::DumpDefinition()
LOG("%s", ss.str().c_str());
}

/*** Gets the OpenGL Buffer ID
*
* @function VBO:GetID
* @treturn number buffer ID
*/
uint32_t LuaVBOImpl::GetID()
{
VBOExistenceCheck(vbo, __func__);
return vbo->GetId();
}

void LuaVBOImpl::AllocGLBuffer(size_t byteSize)
{
if (defTarget == GL_UNIFORM_BUFFER && bufferSizeInBytes > UBO_SAFE_SIZE_BYTES) {
Expand Down
2 changes: 2 additions & 0 deletions rts/Lua/LuaVBOImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class LuaVBOImpl {
int UnbindBufferRange(const GLuint index, const sol::optional<int> elemOffsetOpt, const sol::optional<int> elemCountOpt, const sol::optional<GLenum> targetOpt);

void DumpDefinition();
uint32_t GetID();

public:
static bool Supported(GLenum target);
private:
Expand Down
1 change: 1 addition & 0 deletions rts/System/Platform/CpuID.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ namespace springproc {

for (int group = 0; group < system.num_cpu_types; ++group) {
cpu_id_t cpu_id = system.cpu_types[group];
cpuid_brand = std::string(cpu_id.brand_str);
switch(cpu_id.purpose) {
case PURPOSE_GENERAL:
case PURPOSE_PERFORMANCE:
Expand Down
4 changes: 4 additions & 0 deletions rts/System/Platform/CpuID.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#endif

#include <cstdint>
#include <string>

namespace springproc {
_noinline void ExecCPUID(unsigned int* a, unsigned int* b, unsigned int* c, unsigned int* d);
Expand Down Expand Up @@ -44,6 +45,7 @@ namespace springproc {
cores. */
int GetNumPhysicalCores() const { return numPhysicalCores; }
int GetNumLogicalCores() const { return numLogicalCores; }
std::string GetCPUBrand() const { return cpuid_brand; }

/** Total number of physical processor dies in the system. */
int GetTotalNumPackages() const { return totalNumPackages; }
Expand Down Expand Up @@ -71,6 +73,8 @@ namespace springproc {
uint64_t affinityMaskOfPackages[MAX_PROCESSORS];
uint64_t availableProceesorAffinityMask;

std::string cpuid_brand;

////////////////////////
// Intel specific fields

Expand Down
4 changes: 4 additions & 0 deletions rts/System/Platform/Threading.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@ namespace Threading {
return springproc::CPUID::GetInstance().GetNumPhysicalCores();
}

std::string GetCPUBrand() {
return springproc::CPUID::GetInstance().GetCPUBrand();
}

bool HasHyperThreading() { return (GetLogicalCpuCores() > GetPhysicalCpuCores()); }


Expand Down
2 changes: 2 additions & 0 deletions rts/System/Platform/Threading.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ namespace Threading {
int GetLogicalCpuCores(); /// physical + hyperthreading
bool HasHyperThreading();

std::string GetCPUBrand();

/**
* Inform the OS kernel that we are a cpu-intensive task
*/
Expand Down
4 changes: 4 additions & 0 deletions rts/lib/headlessStubs/glstub.c
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,10 @@ GLAPI void APIENTRY glDepthRange(GLdouble nearVal, GLdouble farVal) {}
GLAPI void APIENTRY glBeginConditionalRender(GLuint id, GLenum mode) {}
GLAPI void APIENTRY glEndConditionalRender(void) {}

GLAPI void APIENTRY glObjectLabel(GLenum identifier, GLuint name, GLsizei length, const GLchar *label) {}
GLAPI void APIENTRY glPushDebugGroup(GLenum source, GLuint id, GLsizei length, const GLchar *message) {}
GLAPI void APIENTRY glPopDebugGroup() {}

#ifdef __cplusplus
} // extern "C"
#endif
Expand Down