From c18cdcf36ef238cf0979f21c1205fd56d16c81bd Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 22:57:56 +0100 Subject: [PATCH] display: add support for debug markers, to help with debugging This is useful when running Panda in a tool like apitrace, so that the different calls in a frame are ordered in a neat hierarchy. --- panda/src/display/displayRegion.I | 8 +++++ panda/src/display/displayRegion.cxx | 10 +++--- panda/src/display/displayRegion.h | 3 ++ panda/src/display/graphicsEngine.cxx | 33 ++++++++++++++----- panda/src/glstuff/glGraphicsBuffer_src.cxx | 6 ++++ .../src/glstuff/glGraphicsStateGuardian_src.I | 24 ++++++++++++++ .../glstuff/glGraphicsStateGuardian_src.cxx | 18 ++++++++++ .../src/glstuff/glGraphicsStateGuardian_src.h | 8 +++++ panda/src/glxdisplay/glxGraphicsWindow.cxx | 29 ++++++++++++++++ panda/src/glxdisplay/glxGraphicsWindow.h | 1 + panda/src/gsgbase/graphicsStateGuardianBase.h | 3 ++ panda/src/pgraph/cullResult.cxx | 3 ++ panda/src/wgldisplay/wglGraphicsWindow.cxx | 5 +++ 13 files changed, 139 insertions(+), 12 deletions(-) diff --git a/panda/src/display/displayRegion.I b/panda/src/display/displayRegion.I index 96f2383bf9..71b3a069cf 100644 --- a/panda/src/display/displayRegion.I +++ b/panda/src/display/displayRegion.I @@ -523,6 +523,14 @@ get_draw_region_pcollector() { return _draw_region_pcollector; } +/** + * Returns a unique name used for debugging. + */ +INLINE const std::string &DisplayRegion:: +get_debug_name() const { + return _debug_name; +} + /** * */ diff --git a/panda/src/display/displayRegion.cxx b/panda/src/display/displayRegion.cxx index 6946ce6832..4629a38f75 100644 --- a/panda/src/display/displayRegion.cxx +++ b/panda/src/display/displayRegion.cxx @@ -677,7 +677,7 @@ do_compute_pixels(int i, int x_size, int y_size, CData *cdata) { */ void DisplayRegion:: set_active_index(int index) { -#ifdef DO_PSTATS +#if defined(DO_PSTATS) || !defined(NDEBUG) std::ostringstream strm; // To make a more useful name for PStats and debug output, we add the scene @@ -697,10 +697,12 @@ set_active_index(int index) { // And add the index in case we have two scene graphs with the same name. strm << "#" << index; - string name = strm.str(); + _debug_name = strm.str(); +#endif - _cull_region_pcollector = PStatCollector(_window->get_cull_window_pcollector(), name); - _draw_region_pcollector = PStatCollector(_window->get_draw_window_pcollector(), name); +#ifdef DO_PSTATS + _cull_region_pcollector = PStatCollector(_window->get_cull_window_pcollector(), _debug_name); + _draw_region_pcollector = PStatCollector(_window->get_draw_window_pcollector(), _debug_name); #endif // DO_PSTATS } diff --git a/panda/src/display/displayRegion.h b/panda/src/display/displayRegion.h index e97829be2c..280edaa333 100644 --- a/panda/src/display/displayRegion.h +++ b/panda/src/display/displayRegion.h @@ -184,6 +184,8 @@ public: INLINE PStatCollector &get_cull_region_pcollector(); INLINE PStatCollector &get_draw_region_pcollector(); + INLINE const std::string &get_debug_name() const; + struct Region { INLINE Region(); @@ -277,6 +279,7 @@ private: PStatCollector _cull_region_pcollector; PStatCollector _draw_region_pcollector; + std::string _debug_name; public: static TypeHandle get_class_type() { diff --git a/panda/src/display/graphicsEngine.cxx b/panda/src/display/graphicsEngine.cxx index c79b470c28..dd160d55b5 100644 --- a/panda/src/display/graphicsEngine.cxx +++ b/panda/src/display/graphicsEngine.cxx @@ -1174,7 +1174,8 @@ extract_texture_data(Texture *tex, GraphicsStateGuardian *gsg) { */ void GraphicsEngine:: dispatch_compute(const LVecBase3i &work_groups, const ShaderAttrib *sattr, GraphicsStateGuardian *gsg) { - nassertv(sattr->get_shader() != nullptr); + const Shader *shader = sattr->get_shader(); + nassertv(shader != nullptr); nassertv(gsg != nullptr); ReMutexHolder holder(_lock); @@ -1184,8 +1185,10 @@ dispatch_compute(const LVecBase3i &work_groups, const ShaderAttrib *sattr, Graph string draw_name = gsg->get_threading_model().get_draw_name(); if (draw_name.empty()) { // A single-threaded environment. No problem. + gsg->push_group_marker(std::string("Compute ") + shader->get_filename(Shader::ST_compute).get_basename()); gsg->set_state_and_transform(state, TransformState::make_identity()); gsg->dispatch_compute(work_groups[0], work_groups[1], work_groups[2]); + gsg->pop_group_marker(); } else { // A multi-threaded environment. We have to wait until the draw thread @@ -1434,7 +1437,9 @@ cull_and_draw_together(GraphicsEngine::Windows wlist, if (win->is_any_clear_active()) { GraphicsStateGuardian *gsg = win->get_gsg(); PStatGPUTimer timer(gsg, win->get_clear_window_pcollector(), current_thread); + gsg->push_group_marker("Clear"); win->clear(current_thread); + gsg->pop_group_marker(); } int num_display_regions = win->get_num_active_display_regions(); @@ -1472,6 +1477,8 @@ cull_and_draw_together(GraphicsOutput *win, DisplayRegion *dr, GraphicsStateGuardian *gsg = win->get_gsg(); nassertv(gsg != nullptr); + gsg->push_group_marker(dr->get_debug_name()); + PT(SceneSetup) scene_setup; { @@ -1517,6 +1524,8 @@ cull_and_draw_together(GraphicsOutput *win, DisplayRegion *dr, gsg->end_scene(); } } + + gsg->pop_group_marker(); } /** @@ -1658,7 +1667,9 @@ draw_bins(const GraphicsEngine::Windows &wlist, Thread *current_thread) { PStatGPUTimer timer(gsg, win->get_draw_window_pcollector(), current_thread); if (win->is_any_clear_active()) { PStatGPUTimer timer(gsg, win->get_clear_window_pcollector(), current_thread); + win->get_gsg()->push_group_marker("Clear"); win->clear(current_thread); + win->get_gsg()->pop_group_marker(); } if (display_cat.is_spam()) { @@ -2008,6 +2019,8 @@ do_draw(GraphicsOutput *win, GraphicsStateGuardian *gsg, DisplayRegion *dr, Thre // Statistics PStatGPUTimer timer(gsg, dr->get_draw_region_pcollector(), current_thread); + gsg->push_group_marker(dr->get_debug_name()); + PT(CullResult) cull_result; PT(SceneSetup) scene_setup; { @@ -2043,11 +2056,7 @@ do_draw(GraphicsOutput *win, GraphicsStateGuardian *gsg, DisplayRegion *dr, Thre // We don't trust the state the callback may have left us in. gsg->clear_state_and_transform(); - // The callback has taken care of the drawing. - return; - } - - if (cull_result == nullptr || scene_setup == nullptr) { + } else if (cull_result == nullptr || scene_setup == nullptr) { // Nothing to see here. } else if (dr->is_stereo()) { @@ -2068,6 +2077,8 @@ do_draw(GraphicsOutput *win, GraphicsStateGuardian *gsg, DisplayRegion *dr, Thre gsg->end_scene(); } } + + gsg->pop_group_marker(); } /** @@ -2677,8 +2688,14 @@ thread_main() { case TS_do_compute: nassertd(_gsg != nullptr && _state != nullptr) break; - _gsg->set_state_and_transform(_state, TransformState::make_identity()); - _gsg->dispatch_compute(_work_groups[0], _work_groups[1], _work_groups[2]); + { + const ShaderAttrib *sattr; + _state->get_attrib(sattr); + _gsg->push_group_marker(std::string("Compute ") + sattr->get_shader()->get_filename(Shader::ST_compute).get_basename()); + _gsg->set_state_and_transform(_state, TransformState::make_identity()); + _gsg->dispatch_compute(_work_groups[0], _work_groups[1], _work_groups[2]); + _gsg->pop_group_marker(); + } break; case TS_do_extract: diff --git a/panda/src/glstuff/glGraphicsBuffer_src.cxx b/panda/src/glstuff/glGraphicsBuffer_src.cxx index 3a65317c5e..06c26a590d 100644 --- a/panda/src/glstuff/glGraphicsBuffer_src.cxx +++ b/panda/src/glstuff/glGraphicsBuffer_src.cxx @@ -230,6 +230,9 @@ begin_frame(FrameMode mode, Thread *current_thread) { } } + CLP(GraphicsStateGuardian) *glgsg = (CLP(GraphicsStateGuardian) *)_gsg.p(); + glgsg->push_group_marker(std::string(CLASSPREFIX_QUOTED "GraphicsBuffer ") + get_name()); + // Figure out the desired size of the buffer. if (mode == FM_render) { clear_cube_map_selection(); @@ -255,6 +258,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { if (_needs_rebuild) { // If we still need rebuild, something went wrong with // rebuild_bitplanes(). + glgsg->pop_group_marker(); return false; } @@ -1314,6 +1318,8 @@ end_frame(FrameMode mode, Thread *current_thread) { clear_cube_map_selection(); } report_my_gl_errors(); + + glgsg->pop_group_marker(); } /** diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.I b/panda/src/glstuff/glGraphicsStateGuardian_src.I index bdb9db6eb5..5cb8d8db10 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.I +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.I @@ -11,6 +11,30 @@ * @date 1999-02-02 */ +/** + * If debug markers are enabled, pushes the beginning of a group marker. + */ +INLINE void CLP(GraphicsStateGuardian):: +push_group_marker(const std::string &marker) { +#if !defined(NDEBUG) && !defined(OPENGLES_1) + if (_glPushGroupMarker != nullptr) { + _glPushGroupMarker(marker.size(), marker.data()); + } +#endif +} + +/** + * If debug markers are enabled, closes a group debug marker. + */ +INLINE void CLP(GraphicsStateGuardian):: +pop_group_marker() { +#if !defined(NDEBUG) && !defined(OPENGLES_1) + if (_glPopGroupMarker != nullptr) { + _glPopGroupMarker(); + } +#endif +} + /** * Checks for any outstanding error codes and outputs them, if found. If * NDEBUG is defined, this function does nothing. The return value is true if diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index efa5592539..c7f8d51f88 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -616,6 +616,22 @@ reset() { // Print out a list of all extensions. report_extensions(); + // Check if we are running under a profiling tool such as apitrace. +#if !defined(NDEBUG) && !defined(OPENGLES_1) + if (has_extension("GL_EXT_debug_marker")) { + _glPushGroupMarker = (PFNGLPUSHGROUPMARKEREXTPROC) + get_extension_func("glPushGroupMarkerEXT"); + _glPopGroupMarker = (PFNGLPOPGROUPMARKEREXTPROC) + get_extension_func("glPopGroupMarkerEXT"); + + // Start a group right away. + push_group_marker("reset"); + } else { + _glPushGroupMarker = nullptr; + _glPopGroupMarker = nullptr; + } +#endif + // Initialize OpenGL debugging output first, if enabled and supported. _supports_debug = false; _use_object_labels = false; @@ -3373,6 +3389,8 @@ reset() { } #endif + pop_group_marker(); + // Now that the GSG has been initialized, make it available for // optimizations. add_gsg(this); diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.h b/panda/src/glstuff/glGraphicsStateGuardian_src.h index 99c8296336..9abce60cd5 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.h +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.h @@ -275,6 +275,9 @@ public: static void APIENTRY debug_callback(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar *message, GLvoid *userParam); + INLINE virtual void push_group_marker(const std::string &marker) final; + INLINE virtual void pop_group_marker() final; + virtual void reset(); virtual void prepare_display_region(DisplayRegionPipelineReader *dr); @@ -1091,6 +1094,11 @@ public: GLuint _white_texture; #ifndef NDEBUG +#ifndef OPENGLES_1 + PFNGLPUSHGROUPMARKEREXTPROC _glPushGroupMarker; + PFNGLPOPGROUPMARKEREXTPROC _glPopGroupMarker; +#endif + bool _show_texture_usage; int _show_texture_usage_max_size; int _show_texture_usage_index; diff --git a/panda/src/glxdisplay/glxGraphicsWindow.cxx b/panda/src/glxdisplay/glxGraphicsWindow.cxx index a7a2472a84..5e9a670cff 100644 --- a/panda/src/glxdisplay/glxGraphicsWindow.cxx +++ b/panda/src/glxdisplay/glxGraphicsWindow.cxx @@ -89,6 +89,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { glxgsg->reset_if_new(); if (mode == FM_render) { + glxgsg->push_group_marker(std::string("glxGraphicsWindow ") + get_name()); // begin_render_texture(); clear_cube_map_selection(); } @@ -97,6 +98,34 @@ begin_frame(FrameMode mode, Thread *current_thread) { return _gsg->begin_frame(current_thread); } + +/** + * This function will be called within the draw thread after rendering is + * completed for a given frame. It should do whatever finalization is + * required. + */ +void glxGraphicsWindow:: +end_frame(FrameMode mode, Thread *current_thread) { + end_frame_spam(mode); + nassertv(_gsg != nullptr); + + if (mode == FM_render) { + // end_render_texture(); + copy_to_textures(); + } + + _gsg->end_frame(current_thread); + + if (mode == FM_render) { + trigger_flip(); + clear_cube_map_selection(); + + glxGraphicsStateGuardian *glxgsg; + DCAST_INTO_V(glxgsg, _gsg); + glxgsg->pop_group_marker(); + } +} + /** * This function will be called within the draw thread after begin_flip() has * been called on all windows, to finish the exchange of the front and back diff --git a/panda/src/glxdisplay/glxGraphicsWindow.h b/panda/src/glxdisplay/glxGraphicsWindow.h index 4c583c186c..53ebc9133c 100644 --- a/panda/src/glxdisplay/glxGraphicsWindow.h +++ b/panda/src/glxdisplay/glxGraphicsWindow.h @@ -36,6 +36,7 @@ public: virtual ~glxGraphicsWindow() {}; virtual bool begin_frame(FrameMode mode, Thread *current_thread); + virtual void end_frame(FrameMode mode, Thread *current_thread); virtual void end_flip(); protected: diff --git a/panda/src/gsgbase/graphicsStateGuardianBase.h b/panda/src/gsgbase/graphicsStateGuardianBase.h index e993b74de2..1dd7def8a2 100644 --- a/panda/src/gsgbase/graphicsStateGuardianBase.h +++ b/panda/src/gsgbase/graphicsStateGuardianBase.h @@ -235,6 +235,9 @@ public: #endif } + virtual void push_group_marker(const std::string &marker) {} + virtual void pop_group_marker() {} + PUBLISHED: static GraphicsStateGuardianBase *get_default_gsg(); static void set_default_gsg(GraphicsStateGuardianBase *default_gsg); diff --git a/panda/src/pgraph/cullResult.cxx b/panda/src/pgraph/cullResult.cxx index faee06ee53..65c9555bff 100644 --- a/panda/src/pgraph/cullResult.cxx +++ b/panda/src/pgraph/cullResult.cxx @@ -295,7 +295,10 @@ draw(Thread *current_thread) { nassertv(bin_index >= 0); if (bin_index < (int)_bins.size() && _bins[bin_index] != nullptr) { + + _gsg->push_group_marker(_bins[bin_index]->get_name()); _bins[bin_index]->draw(force, current_thread); + _gsg->pop_group_marker(); } } } diff --git a/panda/src/wgldisplay/wglGraphicsWindow.cxx b/panda/src/wgldisplay/wglGraphicsWindow.cxx index 6ff5b88c36..1c4fbe9773 100644 --- a/panda/src/wgldisplay/wglGraphicsWindow.cxx +++ b/panda/src/wgldisplay/wglGraphicsWindow.cxx @@ -87,6 +87,7 @@ begin_frame(FrameMode mode, Thread *current_thread) { wglgsg->reset_if_new(); if (mode == FM_render) { + wglgsg->push_group_marker(std::string("wglGraphicsWindow ") + get_name()); clear_cube_map_selection(); } @@ -114,6 +115,10 @@ end_frame(FrameMode mode, Thread *current_thread) { if (mode == FM_render) { trigger_flip(); clear_cube_map_selection(); + + wglGraphicsStateGuardian *wglgsg; + DCAST_INTO_V(wglgsg, _gsg); + wglgsg->pop_group_marker(); } }