Page MenuHomeWildfire Games

Use GL_KHR_debug
Needs ReviewPublic

Authored by linkmauve on Dec 19 2019, 9:51 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

This extension is also core in OpenGL 4.3 and OpenGL ES 3.2. It lets us associate a string to pretty much any object, push and pop debug groups, and get a callback whenever some part of the stack wants to talk with us.

Test Plan
  • Build in debug mode.
  • Test with a debugger such as apitrace
  • Textures should now have their original path attached.
  • Listing of functions should be much easier to read, as they are grouped by purpose.
  • Driver information about performance issues and such should be printed in the terminal.

Event Timeline

linkmauve created this revision.Dec 19 2019, 9:51 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/803/display/redirect

linkmauve updated this revision to Diff 10660.Dec 19 2019, 10:07 PM
linkmauve edited the test plan for this revision. (Show Details)

#ifdef the glObjectLabelKHR() call to avoid it being executed when not debugging.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/external_libraries/glext_funcs.h
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2013"

source/lib/res/graphics/ogl_tex.cpp
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1319/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/805/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/external_libraries/glext_funcs.h
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2013"

source/lib/res/graphics/ogl_tex.cpp
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1321/display/redirect

linkmauve updated this revision to Diff 10662.Dec 19 2019, 10:46 PM

Fix the copyright years.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/806/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1322/display/redirect

linkmauve updated this revision to Diff 10739.Dec 22 2019, 5:03 PM
linkmauve retitled this revision from RFC: Use GL_KHR_debug to associate its path to a GL texture to Use GL_KHR_debug.
linkmauve edited the summary of this revision. (Show Details)
linkmauve edited the test plan for this revision. (Show Details)
  • Add debug groups push and pop using RAII.
  • Add debug message callback with filtering.
Owners added a subscriber: Restricted Owners Package.Dec 22 2019, 5:03 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/862/display/redirect

linkmauve updated this revision to Diff 10740.Dec 22 2019, 5:09 PM

Fix build failure.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/863/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/ogl.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/lib/ogl.h
| 198| class·ogl_DebugGroup
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classogl_DebugGroup{' is invalid C code. Use --std or --language to configure the language.

source/lib/ogl.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1378/display/redirect

linkmauve updated this revision to Diff 10741.Dec 22 2019, 5:30 PM
  • Update the copyright years.
  • Simplify the ogl_DebugGroup class.
  • Add a fallback for when we’re not compiling in debug mode.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/864/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/ogl.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/lib/ogl.h
| 198| class·ogl_DebugGroup
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classogl_DebugGroup{' is invalid C code. Use --std or --language to configure the language.

source/lib/ogl.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1379/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1380/display/redirect

vladislavbelov added inline comments.
source/lib/external_libraries/glext_funcs.h
156

We have increasing number of version, so I think it'd be better to put this section at the end of the #else - #endif section. Like it's done for GLES.

source/lib/ogl.h
212

It doesn't make sense to generate even an empty struct. I suggest to replace ogl_DebugGroup group("Draw GUI element"); by one macro like OGL_DEBUG_GROUP("Draw GUI element");, like it's done for profiler.

BTW can't we just add groups in PROFILER2? Because we already have GL queries there and it seems reasonable to have them together.

linkmauve added inline comments.Jan 4 2020, 5:31 PM
source/lib/ogl.h
212

Note that your PROFILE2() macro does exactly the same thing, by creating a CProfile2Region object named profile2__ (see source/ps/Profiler2.h:536), except this object contains a m_Name pointer so it can’t be eluded by the compiler. It shouldn’t contain it though, you don’t reuse it at all after the constructor is done, that sounds like an easy optimisation.

Adding groups the same way would be ok, but I’m not sure why go through the indirection of your profiler when you just want to have raii for OpenGL calls.

Stan updated this revision to Diff 11154.EditedJan 23 2020, 11:54 AM
Stan added a subscriber: Stan.

Fix windows build. Apitrace doesn't work for me though, it segfaults directly

EDIT: I do get warnings such as these though:

warning: OpenGL other of severity notification from the API id 131185: Buffer detailed info: Buffer object 1 (bound to GL_ELEMENT_ARRAY_BUFFER_ARB, usage hint is GL_STATIC_DRAW) will use VIDEO memory as the source for buffer object operations.

Also lots of warnings for animated meshes (even though they are not moving in atlas) and one or two for static meshes.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/212/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/gui/ObjectTypes/CMiniMap.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/ps/CLogger.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/renderer/TerrainRenderer.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/lib/ogl.h
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/lib/res/graphics/ogl_tex.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/gui/CGUIText.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/gui/GUIRenderer.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/lib/ogl.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/renderer/WaterManager.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1634/display/redirect

Stan added a comment.Jan 24 2020, 10:51 AM

Just confirmed that the crash in debug mode of apitrace is not linked to this patch. So only thing to fix is Mac Os.