From c3d52eeee1a5daacc374e09f54ca2b1cb269d77c Mon Sep 17 00:00:00 2001 From: Sam Edwards Date: Mon, 12 Nov 2018 17:24:15 -0700 Subject: [PATCH 01/29] express: Fix compiler error with HAVE_TAR --- panda/src/express/patchfile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/panda/src/express/patchfile.cxx b/panda/src/express/patchfile.cxx index 5aef4416d1..c7e212c260 100644 --- a/panda/src/express/patchfile.cxx +++ b/panda/src/express/patchfile.cxx @@ -32,7 +32,7 @@ #endif // HAVE_TAR #ifdef HAVE_TAR -istream *Patchfile::_tar_istream = nullptr; +std::istream *Patchfile::_tar_istream = nullptr; #endif // HAVE_TAR using std::endl; From 8f73f95e79e9927067a4ece7b42527dd466beb08 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 20:00:59 +0100 Subject: [PATCH 02/29] display: make PStats clear collectors per-window --- panda/src/display/graphicsEngine.cxx | 13 +++++++++++-- panda/src/display/graphicsOutput.I | 9 +++++++++ panda/src/display/graphicsOutput.cxx | 1 + panda/src/display/graphicsOutput.h | 2 ++ panda/src/display/graphicsStateGuardian.cxx | 1 - panda/src/display/graphicsStateGuardian.h | 1 - panda/src/glstuff/glGraphicsBuffer_src.cxx | 2 -- panda/src/glstuff/glGraphicsStateGuardian_src.cxx | 1 - panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx | 2 -- 9 files changed, 23 insertions(+), 9 deletions(-) diff --git a/panda/src/display/graphicsEngine.cxx b/panda/src/display/graphicsEngine.cxx index 496a05e273..c79b470c28 100644 --- a/panda/src/display/graphicsEngine.cxx +++ b/panda/src/display/graphicsEngine.cxx @@ -1431,7 +1431,11 @@ cull_and_draw_together(GraphicsEngine::Windows wlist, } if (win->begin_frame(GraphicsOutput::FM_render, current_thread)) { - win->clear(current_thread); + if (win->is_any_clear_active()) { + GraphicsStateGuardian *gsg = win->get_gsg(); + PStatGPUTimer timer(gsg, win->get_clear_window_pcollector(), current_thread); + win->clear(current_thread); + } int num_display_regions = win->get_num_active_display_regions(); for (int i = 0; i < num_display_regions; i++) { @@ -1476,6 +1480,7 @@ cull_and_draw_together(GraphicsOutput *win, DisplayRegion *dr, gsg->prepare_display_region(&dr_reader); if (dr_reader.is_any_clear_active()) { + PStatGPUTimer timer(gsg, win->get_clear_window_pcollector(), current_thread); gsg->clear(dr); } @@ -1651,7 +1656,10 @@ draw_bins(const GraphicsEngine::Windows &wlist, Thread *current_thread) { // a current context for PStatGPUTimer to work. { PStatGPUTimer timer(gsg, win->get_draw_window_pcollector(), current_thread); - win->clear(current_thread); + if (win->is_any_clear_active()) { + PStatGPUTimer timer(gsg, win->get_clear_window_pcollector(), current_thread); + win->clear(current_thread); + } if (display_cat.is_spam()) { display_cat.spam() @@ -2015,6 +2023,7 @@ do_draw(GraphicsOutput *win, GraphicsStateGuardian *gsg, DisplayRegion *dr, Thre win->change_scenes(&dr_reader); gsg->prepare_display_region(&dr_reader); if (dr_reader.is_any_clear_active()) { + PStatGPUTimer timer(gsg, win->get_clear_window_pcollector(), current_thread); gsg->clear(dr_reader.get_object()); } diff --git a/panda/src/display/graphicsOutput.I b/panda/src/display/graphicsOutput.I index c94c83f54c..992f634dd9 100644 --- a/panda/src/display/graphicsOutput.I +++ b/panda/src/display/graphicsOutput.I @@ -703,6 +703,15 @@ get_draw_window_pcollector() { return _draw_window_pcollector; } +/** + * Returns a PStatCollector for timing the clear operation for just this + * GraphicsOutput. + */ +INLINE PStatCollector &GraphicsOutput:: +get_clear_window_pcollector() { + return _clear_window_pcollector; +} + /** * Display the spam message associated with begin_frame */ diff --git a/panda/src/display/graphicsOutput.cxx b/panda/src/display/graphicsOutput.cxx index c8237059b8..33bfddb79f 100644 --- a/panda/src/display/graphicsOutput.cxx +++ b/panda/src/display/graphicsOutput.cxx @@ -77,6 +77,7 @@ GraphicsOutput(GraphicsEngine *engine, GraphicsPipe *pipe, _lock("GraphicsOutput"), _cull_window_pcollector(_cull_pcollector, name), _draw_window_pcollector(_draw_pcollector, name), + _clear_window_pcollector(_draw_window_pcollector, "Clear"), _size(0, 0) { #ifdef DO_MEMORY_USAGE diff --git a/panda/src/display/graphicsOutput.h b/panda/src/display/graphicsOutput.h index f426b41bee..49c257e17e 100644 --- a/panda/src/display/graphicsOutput.h +++ b/panda/src/display/graphicsOutput.h @@ -289,6 +289,7 @@ public: INLINE PStatCollector &get_cull_window_pcollector(); INLINE PStatCollector &get_draw_window_pcollector(); + INLINE PStatCollector &get_clear_window_pcollector(); protected: virtual void pixel_factor_changed(); @@ -409,6 +410,7 @@ protected: static PStatCollector _draw_pcollector; PStatCollector _cull_window_pcollector; PStatCollector _draw_window_pcollector; + PStatCollector _clear_window_pcollector; public: static TypeHandle get_class_type() { diff --git a/panda/src/display/graphicsStateGuardian.cxx b/panda/src/display/graphicsStateGuardian.cxx index 0e82a23f88..93c732e5d7 100644 --- a/panda/src/display/graphicsStateGuardian.cxx +++ b/panda/src/display/graphicsStateGuardian.cxx @@ -92,7 +92,6 @@ PStatCollector GraphicsStateGuardian::_transform_state_pcollector("State changes PStatCollector GraphicsStateGuardian::_texture_state_pcollector("State changes:Textures"); PStatCollector GraphicsStateGuardian::_draw_primitive_pcollector("Draw:Primitive:Draw"); PStatCollector GraphicsStateGuardian::_draw_set_state_pcollector("Draw:Set State"); -PStatCollector GraphicsStateGuardian::_clear_pcollector("Draw:Clear"); PStatCollector GraphicsStateGuardian::_flush_pcollector("Draw:Flush"); PStatCollector GraphicsStateGuardian::_compute_dispatch_pcollector("Draw:Compute dispatch"); diff --git a/panda/src/display/graphicsStateGuardian.h b/panda/src/display/graphicsStateGuardian.h index c64bbe692a..e1c9435fef 100644 --- a/panda/src/display/graphicsStateGuardian.h +++ b/panda/src/display/graphicsStateGuardian.h @@ -685,7 +685,6 @@ public: static PStatCollector _texture_state_pcollector; static PStatCollector _draw_primitive_pcollector; static PStatCollector _draw_set_state_pcollector; - static PStatCollector _clear_pcollector; static PStatCollector _flush_pcollector; static PStatCollector _compute_dispatch_pcollector; static PStatCollector _wait_occlusion_pcollector; diff --git a/panda/src/glstuff/glGraphicsBuffer_src.cxx b/panda/src/glstuff/glGraphicsBuffer_src.cxx index d5a8763fc8..3a65317c5e 100644 --- a/panda/src/glstuff/glGraphicsBuffer_src.cxx +++ b/panda/src/glstuff/glGraphicsBuffer_src.cxx @@ -113,8 +113,6 @@ clear(Thread *current_thread) { << get_name() << " " << (void *)this << "\n"; } - PStatGPUTimer timer(glgsg, glgsg->_clear_pcollector); - // Disable the scissor test, so we can clear the whole buffer. glDisable(GL_SCISSOR_TEST); glgsg->_scissor_enabled = false; diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 6ce88525f5..efa5592539 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -3405,7 +3405,6 @@ finish() { */ void CLP(GraphicsStateGuardian):: clear(DrawableRegion *clearable) { - PStatGPUTimer timer(this, _clear_pcollector); report_my_gl_errors(); if (!clearable->is_any_clear_active()) { diff --git a/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx b/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx index fd3c44c917..4adc287cb4 100644 --- a/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx +++ b/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx @@ -202,8 +202,6 @@ make_geom_munger(const RenderState *state, Thread *current_thread) { */ void TinyGraphicsStateGuardian:: clear(DrawableRegion *clearable) { - PStatTimer timer(_clear_pcollector); - if ((!clearable->get_clear_color_active())&& (!clearable->get_clear_depth_active())&& (!clearable->get_clear_stencil_active())) { From e759a1b6052be122eb9f9985e099f76f1ff6573a Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 21:01:43 +0100 Subject: [PATCH 03/29] display: give DisplayRegions a more recognisable name in PStats --- panda/src/display/displayRegion.cxx | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/panda/src/display/displayRegion.cxx b/panda/src/display/displayRegion.cxx index e1765861e1..6946ce6832 100644 --- a/panda/src/display/displayRegion.cxx +++ b/panda/src/display/displayRegion.cxx @@ -679,7 +679,24 @@ void DisplayRegion:: set_active_index(int index) { #ifdef DO_PSTATS std::ostringstream strm; - strm << "dr_" << index; + + // To make a more useful name for PStats and debug output, we add the scene + // graph name and camera name. + NodePath camera = get_camera(); + if (!camera.is_empty()) { + Camera *camera_node = DCAST(Camera, camera.node()); + if (camera_node != nullptr) { + NodePath scene_root = camera_node->get_scene(); + if (scene_root.is_empty()) { + scene_root = camera.get_top(); + } + strm << scene_root.get_name(); + } + } + + // And add the index in case we have two scene graphs with the same name. + strm << "#" << index; + string name = strm.str(); _cull_region_pcollector = PStatCollector(_window->get_cull_window_pcollector(), name); From d902ea5ce4e084c8dd39d2bd781a463893ba12fa Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 22:13:07 +0100 Subject: [PATCH 04/29] display: don't render window if all its DRs are inactive This is an optimization, which will skip begin_frame/end_frame for a buffer that isn't going to have anything rendered to it. Affects the RenderPipeline. --- panda/src/display/graphicsOutput.cxx | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/panda/src/display/graphicsOutput.cxx b/panda/src/display/graphicsOutput.cxx index 33bfddb79f..d473b4bee2 100644 --- a/panda/src/display/graphicsOutput.cxx +++ b/panda/src/display/graphicsOutput.cxx @@ -412,15 +412,38 @@ is_active() const { return false; } - CDReader cdata(_cycler); + CDLockedReader cdata(_cycler); + if (!cdata->_active) { + return false; + } + if (cdata->_one_shot_frame != -1) { // If one_shot is in effect, then we are active only for the one indicated // frame. if (cdata->_one_shot_frame != ClockObject::get_global_clock()->get_frame_count()) { return false; + } else { + return true; } } - return cdata->_active; + + // If the window has a clear value set, it is active. + if (is_any_clear_active()) { + return true; + } + + // If we triggered a copy operation, it is also active. + if (_trigger_copy) { + return true; + } + + // The window is active if at least one display region is active. + if (cdata->_active_display_regions_stale) { + CDWriter cdataw(((GraphicsOutput *)this)->_cycler, cdata, false); + ((GraphicsOutput *)this)->do_determine_display_regions(cdataw); + } + + return !cdata->_active_display_regions.empty(); } /** From b1eec5fae04b02f2c7fd7fbb71cd7b2f8163e6bb Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 22:15:31 +0100 Subject: [PATCH 05/29] CommonFilters: give passes a unique name for debugging/PStats --- direct/src/filter/CommonFilters.py | 26 +++++++++++++------------- direct/src/filter/FilterManager.py | 4 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/direct/src/filter/CommonFilters.py b/direct/src/filter/CommonFilters.py index 894b57c321..9e54cb82b8 100644 --- a/direct/src/filter/CommonFilters.py +++ b/direct/src/filter/CommonFilters.py @@ -184,8 +184,8 @@ class CommonFilters: if ("BlurSharpen" in configuration): blur0=self.textures["blur0"] blur1=self.textures["blur1"] - self.blur.append(self.manager.renderQuadInto(colortex=blur0,div=2)) - self.blur.append(self.manager.renderQuadInto(colortex=blur1)) + self.blur.append(self.manager.renderQuadInto("filter-blur0", colortex=blur0,div=2)) + self.blur.append(self.manager.renderQuadInto("filter-blur1", colortex=blur1)) self.blur[0].setShaderInput("src", self.textures["color"]) self.blur[0].setShader(self.loadShader("filter-blurx.sha")) self.blur[1].setShaderInput("src", blur0) @@ -195,9 +195,9 @@ class CommonFilters: ssao0=self.textures["ssao0"] ssao1=self.textures["ssao1"] ssao2=self.textures["ssao2"] - self.ssao.append(self.manager.renderQuadInto(colortex=ssao0)) - self.ssao.append(self.manager.renderQuadInto(colortex=ssao1,div=2)) - self.ssao.append(self.manager.renderQuadInto(colortex=ssao2)) + self.ssao.append(self.manager.renderQuadInto("filter-ssao0", colortex=ssao0)) + self.ssao.append(self.manager.renderQuadInto("filter-ssao1", colortex=ssao1,div=2)) + self.ssao.append(self.manager.renderQuadInto("filter-ssao2", colortex=ssao2)) self.ssao[0].setShaderInput("depth", self.textures["depth"]) self.ssao[0].setShaderInput("normal", self.textures["aux"]) self.ssao[0].setShaderInput("random", loader.loadTexture("maps/random.rgb")) @@ -215,21 +215,21 @@ class CommonFilters: bloom3=self.textures["bloom3"] if (bloomconf.size == "large"): scale=8 - downsampler="filter-down4.sha" + downsampler="filter-down4" elif (bloomconf.size == "medium"): scale=4 - downsampler="filter-copy.sha" + downsampler="filter-copy" else: scale=2 - downsampler="filter-copy.sha" - self.bloom.append(self.manager.renderQuadInto(colortex=bloom0, div=2, align=scale)) - self.bloom.append(self.manager.renderQuadInto(colortex=bloom1, div=scale, align=scale)) - self.bloom.append(self.manager.renderQuadInto(colortex=bloom2, div=scale, align=scale)) - self.bloom.append(self.manager.renderQuadInto(colortex=bloom3, div=scale, align=scale)) + downsampler="filter-copy" + self.bloom.append(self.manager.renderQuadInto("filter-bloomi", colortex=bloom0, div=2, align=scale)) + self.bloom.append(self.manager.renderQuadInto(downsampler, colortex=bloom1, div=scale, align=scale)) + self.bloom.append(self.manager.renderQuadInto("filter-bloomx", colortex=bloom2, div=scale, align=scale)) + self.bloom.append(self.manager.renderQuadInto("filter-bloomy", colortex=bloom3, div=scale, align=scale)) self.bloom[0].setShaderInput("src", self.textures["color"]) self.bloom[0].setShader(self.loadShader("filter-bloomi.sha")) self.bloom[1].setShaderInput("src", bloom0) - self.bloom[1].setShader(self.loadShader(downsampler)) + self.bloom[1].setShader(self.loadShader(downsampler + ".sha")) self.bloom[2].setShaderInput("src", bloom1) self.bloom[2].setShader(self.loadShader("filter-bloomx.sha")) self.bloom[3].setShaderInput("src", bloom2) diff --git a/direct/src/filter/FilterManager.py b/direct/src/filter/FilterManager.py index 1de63c702d..4150cba568 100644 --- a/direct/src/filter/FilterManager.py +++ b/direct/src/filter/FilterManager.py @@ -236,7 +236,7 @@ class FilterManager(DirectObject): return quad - def renderQuadInto(self, mul=1, div=1, align=1, depthtex=None, colortex=None, auxtex0=None, auxtex1=None): + def renderQuadInto(self, name="filter-stage", mul=1, div=1, align=1, depthtex=None, colortex=None, auxtex0=None, auxtex1=None): """ Creates an offscreen buffer for an intermediate computation. Installs a quad into the buffer. Returns @@ -250,7 +250,7 @@ class FilterManager(DirectObject): depthbits = bool(depthtex != None) - buffer = self.createBuffer("filter-stage", winx, winy, texgroup, depthbits) + buffer = self.createBuffer(name, winx, winy, texgroup, depthbits) if (buffer == None): return None From c18cdcf36ef238cf0979f21c1205fd56d16c81bd Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 22:57:56 +0100 Subject: [PATCH 06/29] 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(); } } From 53cec96c07db3ccb03f2fa54bbbc930de0272ee0 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 19 Nov 2018 22:59:35 +0100 Subject: [PATCH 07/29] Fix draw calls being listed under Primitive Setup in PStats, etc. Previously, all draw calls would be grouped under "Primitive Setup", rather than under the appropriate bin collector. This commit fixes that and adds a few other useful collectors as well. --- panda/src/glstuff/glGraphicsStateGuardian_src.cxx | 7 +++++-- panda/src/glstuff/glGraphicsStateGuardian_src.h | 1 + panda/src/gobj/geom.cxx | 7 +++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index c7f8d51f88..0aaada58a1 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -91,6 +91,7 @@ PStatCollector CLP(GraphicsStateGuardian)::_vertex_array_update_pcollector("Draw PStatCollector CLP(GraphicsStateGuardian)::_texture_update_pcollector("Draw:Update texture"); PStatCollector CLP(GraphicsStateGuardian)::_fbo_bind_pcollector("Draw:Bind FBO"); PStatCollector CLP(GraphicsStateGuardian)::_check_error_pcollector("Draw:Check errors"); +PStatCollector CLP(GraphicsStateGuardian)::_check_residency_pcollector("*:PStats:Check residency"); // The following noop functions are assigned to the corresponding glext // function pointers in the class, in case the functions are not defined by @@ -3949,6 +3950,7 @@ end_frame(Thread *current_thread) { // connects PStats, at which point it will then correct the assessment. No // harm done. if (has_fixed_function_pipeline() && PStatClient::is_connected()) { + PStatTimer timer(_check_residency_pcollector); check_nonresident_texture(_prepared_objects->_texture_residency.get_inactive_resident()); check_nonresident_texture(_prepared_objects->_texture_residency.get_active_resident()); @@ -7208,6 +7210,8 @@ do_issue_shade_model() { */ void CLP(GraphicsStateGuardian):: do_issue_shader() { + PStatTimer timer(_draw_set_state_shader_pcollector); + ShaderContext *context = 0; Shader *shader = (Shader *)_target_shader->get_shader(); @@ -10919,7 +10923,6 @@ set_state_and_transform(const RenderState *target, _instance_count = _target_shader->get_instance_count(); if (_target_shader != _state_shader) { - // PStatGPUTimer timer(this, _draw_set_state_shader_pcollector); do_issue_shader(); _state_shader = _target_shader; _state_mask.clear_bit(TextureAttrib::get_class_slot()); @@ -11075,7 +11078,7 @@ set_state_and_transform(const RenderState *target, int texture_slot = TextureAttrib::get_class_slot(); if (_target_rs->get_attrib(texture_slot) != _state_rs->get_attrib(texture_slot) || !_state_mask.get_bit(texture_slot)) { - // PStatGPUTimer timer(this, _draw_set_state_texture_pcollector); + PStatGPUTimer timer(this, _draw_set_state_texture_pcollector); determine_target_texture(); do_issue_texture(); diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.h b/panda/src/glstuff/glGraphicsStateGuardian_src.h index 9abce60cd5..f4fda4ab9e 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.h +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.h @@ -1126,6 +1126,7 @@ public: static PStatCollector _texture_update_pcollector; static PStatCollector _fbo_bind_pcollector; static PStatCollector _check_error_pcollector; + static PStatCollector _check_residency_pcollector; public: virtual TypeHandle get_type() const { diff --git a/panda/src/gobj/geom.cxx b/panda/src/gobj/geom.cxx index bafe523e0f..03409b4220 100644 --- a/panda/src/gobj/geom.cxx +++ b/panda/src/gobj/geom.cxx @@ -1844,8 +1844,11 @@ check_valid(const GeomVertexDataPipelineReader *data_reader) const { bool GeomPipelineReader:: draw(GraphicsStateGuardianBase *gsg, const GeomVertexDataPipelineReader *data_reader, bool force) const { - PStatTimer timer(Geom::_draw_primitive_setup_pcollector); - bool all_ok = gsg->begin_draw_primitives(this, data_reader, force); + bool all_ok; + { + PStatTimer timer(Geom::_draw_primitive_setup_pcollector); + all_ok = gsg->begin_draw_primitives(this, data_reader, force); + } if (all_ok) { Geom::Primitives::const_iterator pi; for (pi = _cdata->_primitives.begin(); From ec4b0825e99bbe2c898b8f63c07a62927a16c17f Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 20 Nov 2018 12:41:43 +0100 Subject: [PATCH 08/29] glgsg: restore more OpenGL state after draw callback --- .../glstuff/glGraphicsStateGuardian_src.cxx | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 0aaada58a1..c9a22615bf 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -10661,6 +10661,71 @@ reissue_transforms() { _current_vertex_format.clear(); memset(_vertex_attrib_columns, 0, sizeof(const GeomVertexColumn *) * 32); #endif + + // Since this is called by clear_state_and_transform(), we also should reset + // the states that won't automatically be respecified when clearing the + // state mask. + _active_color_write_mask = ColorWriteAttrib::C_all; + glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + + if (_dithering_enabled) { + glEnable(GL_DITHER); + } else { + glDisable(GL_DITHER); + } + if (_depth_test_enabled) { + glEnable(GL_DEPTH_TEST); + } else { + glDisable(GL_DEPTH_TEST); + } + if (_stencil_test_enabled) { + glEnable(GL_STENCIL_TEST); + } else { + glDisable(GL_STENCIL_TEST); + } + if (_blend_enabled) { + glEnable(GL_BLEND); + } else { + glDisable(GL_BLEND); + } + +#ifndef OPENGLES_2 + if (_multisample_mode != 0) { + glEnable(GL_MULTISAMPLE); + } else { + glDisable(GL_MULTISAMPLE); + glDisable(GL_SAMPLE_ALPHA_TO_ONE); + glDisable(GL_SAMPLE_ALPHA_TO_COVERAGE); + } + if (_line_smooth_enabled) { + glEnable(GL_LINE_SMOOTH); + } else { + glDisable(GL_LINE_SMOOTH); + } +#endif + +#ifndef OPENGLES + if (_polygon_smooth_enabled) { + glEnable(GL_POLYGON_SMOOTH); + } else { + glDisable(GL_POLYGON_SMOOTH); + } +#endif + +#ifdef SUPPORT_FIXED_FUNCTION + if (has_fixed_function_pipeline()) { + if (_alpha_test_enabled) { + glEnable(GL_ALPHA_TEST); + } else { + glDisable(GL_ALPHA_TEST); + } + if (_point_smooth_enabled) { + glEnable(GL_POINT_SMOOTH); + } else { + glDisable(GL_POINT_SMOOTH); + } + } +#endif } #ifdef SUPPORT_FIXED_FUNCTION From 02979fa106ada9b8f311a1aa698d63129d958a7d Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 20 Nov 2018 14:49:44 +0100 Subject: [PATCH 09/29] makepanda: use pkg-config for locating assimp --- makepanda/makepanda.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makepanda/makepanda.py b/makepanda/makepanda.py index 736434fb92..c29fe10389 100755 --- a/makepanda/makepanda.py +++ b/makepanda/makepanda.py @@ -828,7 +828,7 @@ if (COMPILER=="GCC"): SmartPkgEnable("EIGEN", "eigen3", (), ("Eigen/Dense",), target_pkg = 'ALWAYS') SmartPkgEnable("ARTOOLKIT", "", ("AR"), "AR/ar.h") SmartPkgEnable("FCOLLADA", "", ChooseLib(fcollada_libs, "FCOLLADA"), ("FCollada", "FCollada/FCollada.h")) - SmartPkgEnable("ASSIMP", "", ("assimp"), "assimp/Importer.hpp") + SmartPkgEnable("ASSIMP", "assimp", ("assimp"), "assimp/Importer.hpp") SmartPkgEnable("FFMPEG", ffmpeg_libs, ffmpeg_libs, ("libavformat/avformat.h", "libavcodec/avcodec.h", "libavutil/avutil.h")) SmartPkgEnable("SWSCALE", "libswscale", "libswscale", ("libswscale/swscale.h"), target_pkg = "FFMPEG", thirdparty_dir = "ffmpeg") SmartPkgEnable("SWRESAMPLE","libswresample", "libswresample", ("libswresample/swresample.h"), target_pkg = "FFMPEG", thirdparty_dir = "ffmpeg") From 356b604627edc333c766d8cfb92b2cf58c579864 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 20 Nov 2018 14:50:40 +0100 Subject: [PATCH 10/29] makepanda: link with IrrXML when using static assimp library Same fix as #432 but for Linux --- makepanda/makepanda.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/makepanda/makepanda.py b/makepanda/makepanda.py index c29fe10389..c6a01e7cd5 100755 --- a/makepanda/makepanda.py +++ b/makepanda/makepanda.py @@ -872,6 +872,13 @@ if (COMPILER=="GCC"): else: PkgDisable("OPENCV") + if not PkgSkip("ASSIMP") and \ + os.path.isfile(GetThirdpartyDir() + "assimp/lib/libassimp.a"): + # Also pick up IrrXML, which is needed when linking statically. + irrxml = GetThirdpartyDir() + "assimp/lib/libIrrXML.a" + if os.path.isfile(irrxml): + LibName("ASSIMP", irrxml) + rocket_libs = ("RocketCore", "RocketControls") if (GetOptimize() <= 3): rocket_libs += ("RocketDebugger",) From d093cbbb90f6c726f912d23962f5c1cab435d507 Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 22 Nov 2018 23:05:40 +0100 Subject: [PATCH 11/29] grutil: apply FPS meter improvements to scene graph analyzer too This fixes the aspect ratio scaling issue in particular. Fixes #456 --- panda/src/grutil/sceneGraphAnalyzerMeter.cxx | 42 +++++++++++++++++--- panda/src/grutil/sceneGraphAnalyzerMeter.h | 3 ++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/panda/src/grutil/sceneGraphAnalyzerMeter.cxx b/panda/src/grutil/sceneGraphAnalyzerMeter.cxx index 02d6ed3709..7b64422699 100644 --- a/panda/src/grutil/sceneGraphAnalyzerMeter.cxx +++ b/panda/src/grutil/sceneGraphAnalyzerMeter.cxx @@ -19,6 +19,7 @@ #include "depthTestAttrib.h" #include "depthWriteAttrib.h" #include "pStatTimer.h" +#include "omniBoundingVolume.h" #include // For sprintf/snprintf PStatCollector SceneGraphAnalyzerMeter::_show_analyzer_pcollector("*:Show scene graph analysis"); @@ -29,9 +30,16 @@ TypeHandle SceneGraphAnalyzerMeter::_type_handle; * */ SceneGraphAnalyzerMeter:: -SceneGraphAnalyzerMeter(const std::string &name, PandaNode *node) : TextNode(name) { +SceneGraphAnalyzerMeter(const std::string &name, PandaNode *node) : + TextNode(name), + _last_aspect_ratio(-1) { + set_cull_callback(); + // Don't do frustum culling, as the text will always be in view. + set_bounds(new OmniBoundingVolume()); + set_final(true); + Thread *current_thread = Thread::get_current_thread(); _update_interval = scene_graph_analyzer_meter_update_interval; @@ -41,7 +49,7 @@ SceneGraphAnalyzerMeter(const std::string &name, PandaNode *node) : TextNode(nam set_align(A_left); set_transform(LMatrix4::scale_mat(scene_graph_analyzer_meter_scale) * - LMatrix4::translate_mat(LVector3::rfu(-1.0f + scene_graph_analyzer_meter_side_margins * scene_graph_analyzer_meter_scale, 0.0f, 1.0f - scene_graph_analyzer_meter_scale))); + LMatrix4::translate_mat(LVector3::rfu(scene_graph_analyzer_meter_side_margins * scene_graph_analyzer_meter_scale, 0.0f, -scene_graph_analyzer_meter_scale))); set_card_color(0.0f, 0.0f, 0.0f, 0.4); set_card_as_margin(scene_graph_analyzer_meter_side_margins, scene_graph_analyzer_meter_side_margins, 0.1f, 0.0f); set_usage_hint(Geom::UH_client); @@ -77,6 +85,11 @@ setup_window(GraphicsOutput *window) { _root.set_material_off(1); _root.set_two_sided(1, 1); + // If we don't set this explicitly, Panda will cause it to be rendered + // in a back-to-front cull bin, which will cause the bounding volume + // to be computed unnecessarily. Saves a little bit of overhead. + _root.set_bin("unsorted", 0); + // Create a display region that covers the entire window. _display_region = _window->make_display_region(); _display_region->set_sort(scene_graph_analyzer_meter_layer_sort); @@ -87,10 +100,11 @@ setup_window(GraphicsOutput *window) { PT(Lens) lens = new OrthographicLens; - static const PN_stdfloat left = -1.0f; - static const PN_stdfloat right = 1.0f; - static const PN_stdfloat bottom = -1.0f; - static const PN_stdfloat top = 1.0f; + // We choose these values such that we can place the text against (0, 0). + static const PN_stdfloat left = 0.0f; + static const PN_stdfloat right = 2.0f; + static const PN_stdfloat bottom = -2.0f; + static const PN_stdfloat top = 0.0f; lens->set_film_size(right - left, top - bottom); lens->set_film_offset((right + left) * 0.5, (top + bottom) * 0.5); lens->set_near_far(-1000, 1000); @@ -138,6 +152,22 @@ cull_callback(CullTraverser *trav, CullTraverserData &data) { // Statistics PStatTimer timer(_show_analyzer_pcollector, current_thread); + // This is probably a good time to check if the aspect ratio on the window + // has changed. + int width = _display_region->get_pixel_width(); + int height = _display_region->get_pixel_height(); + PN_stdfloat aspect_ratio = 1; + if (width != 0 && height != 0) { + aspect_ratio = (PN_stdfloat)height / (PN_stdfloat)width; + } + + // Scale the transform by the calculated aspect ratio. + if (aspect_ratio != _last_aspect_ratio) { + _aspect_ratio_transform = TransformState::make_scale(LVecBase3(aspect_ratio, 1, 1)); + _last_aspect_ratio = aspect_ratio; + } + data._net_transform = data._net_transform->compose(_aspect_ratio_transform); + // Check to see if it's time to update. double now = _clock_object->get_frame_time(current_thread); double elapsed = now - _last_update; diff --git a/panda/src/grutil/sceneGraphAnalyzerMeter.h b/panda/src/grutil/sceneGraphAnalyzerMeter.h index 148aa37022..e7c184e81b 100644 --- a/panda/src/grutil/sceneGraphAnalyzerMeter.h +++ b/panda/src/grutil/sceneGraphAnalyzerMeter.h @@ -72,6 +72,9 @@ private: PandaNode *_node; ClockObject *_clock_object; + PN_stdfloat _last_aspect_ratio; + CPT(TransformState) _aspect_ratio_transform; + static PStatCollector _show_analyzer_pcollector; public: From 254cea63bb325c500dcf5f53be89491354fa4d7b Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 22 Nov 2018 23:13:58 +0100 Subject: [PATCH 12/29] display: fix assertion in threaded pipeline --- panda/src/display/graphicsOutput.cxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/panda/src/display/graphicsOutput.cxx b/panda/src/display/graphicsOutput.cxx index d473b4bee2..daf9b217b1 100644 --- a/panda/src/display/graphicsOutput.cxx +++ b/panda/src/display/graphicsOutput.cxx @@ -441,9 +441,10 @@ is_active() const { if (cdata->_active_display_regions_stale) { CDWriter cdataw(((GraphicsOutput *)this)->_cycler, cdata, false); ((GraphicsOutput *)this)->do_determine_display_regions(cdataw); + return !cdataw->_active_display_regions.empty(); + } else { + return !cdata->_active_display_regions.empty(); } - - return !cdata->_active_display_regions.empty(); } /** From 0a1b6df648683b815fd8c0ea960dbc49762a23fb Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 22 Nov 2018 23:14:29 +0100 Subject: [PATCH 13/29] glxdisplay: grab X11 lock around various GLX calls --- panda/src/glxdisplay/glxGraphicsBuffer.cxx | 8 +++++++- panda/src/glxdisplay/glxGraphicsPixmap.cxx | 7 ++++++- panda/src/glxdisplay/glxGraphicsStateGuardian.cxx | 6 ++++++ panda/src/glxdisplay/glxGraphicsWindow.cxx | 4 ++++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/panda/src/glxdisplay/glxGraphicsBuffer.cxx b/panda/src/glxdisplay/glxGraphicsBuffer.cxx index d777f7a239..6ede9a3e36 100644 --- a/panda/src/glxdisplay/glxGraphicsBuffer.cxx +++ b/panda/src/glxdisplay/glxGraphicsBuffer.cxx @@ -71,7 +71,10 @@ begin_frame(FrameMode mode, Thread *current_thread) { glxGraphicsStateGuardian *glxgsg; DCAST_INTO_R(glxgsg, _gsg, false); - glXMakeCurrent(_display, _pbuffer, glxgsg->_context); + { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + glXMakeCurrent(_display, _pbuffer, glxgsg->_context); + } // Now that we have made the context current to a window, we can reset the // GSG state if this is the first time it has been used. (We can't just @@ -125,6 +128,7 @@ end_frame(FrameMode mode, Thread *current_thread) { void glxGraphicsBuffer:: close_buffer() { if (_gsg != nullptr) { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); glXMakeCurrent(_display, None, nullptr); if (_pbuffer != None) { @@ -179,6 +183,8 @@ open_buffer() { nassertr(glxgsg->_supports_pbuffer, false); + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + static const int max_attrib_list = 32; int attrib_list[max_attrib_list]; int n = 0; diff --git a/panda/src/glxdisplay/glxGraphicsPixmap.cxx b/panda/src/glxdisplay/glxGraphicsPixmap.cxx index 7f551b8d4f..770a7d3d26 100644 --- a/panda/src/glxdisplay/glxGraphicsPixmap.cxx +++ b/panda/src/glxdisplay/glxGraphicsPixmap.cxx @@ -74,7 +74,10 @@ begin_frame(FrameMode mode, Thread *current_thread) { glxGraphicsStateGuardian *glxgsg; DCAST_INTO_R(glxgsg, _gsg, false); - glXMakeCurrent(_display, _glx_pixmap, glxgsg->_context); + { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + glXMakeCurrent(_display, _glx_pixmap, glxgsg->_context); + } // Now that we have made the context current to a window, we can reset the // GSG state if this is the first time it has been used. (We can't just @@ -127,6 +130,7 @@ end_frame(FrameMode mode, Thread *current_thread) { */ void glxGraphicsPixmap:: close_buffer() { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); if (_gsg != nullptr) { glXMakeCurrent(_display, None, nullptr); _gsg.clear(); @@ -197,6 +201,7 @@ open_buffer() { } } + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); _x_pixmap = XCreatePixmap(_display, _drawable, get_x_size(), get_y_size(), visual_info->depth); if (_x_pixmap == None) { diff --git a/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx b/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx index 76c66d817e..86ddeac589 100644 --- a/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx +++ b/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx @@ -64,6 +64,7 @@ glxGraphicsStateGuardian(GraphicsEngine *engine, GraphicsPipe *pipe, */ glxGraphicsStateGuardian:: ~glxGraphicsStateGuardian() { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); destroy_temp_xwindow(); if (_visuals != nullptr) { XFree(_visuals); @@ -224,6 +225,7 @@ choose_pixel_format(const FrameBufferProperties &properties, X11_Display *display, int screen, bool need_pbuffer, bool need_pixmap) { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); _display = display; _screen = screen; _context = nullptr; @@ -457,6 +459,7 @@ gl_get_error() const { */ void glxGraphicsStateGuardian:: query_gl_version() { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); PosixGraphicsStateGuardian::query_gl_version(); show_glx_client_string("GLX_VENDOR", GLX_VENDOR); @@ -483,6 +486,7 @@ query_gl_version() { */ void glxGraphicsStateGuardian:: get_extra_extensions() { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); save_extensions(glXQueryExtensionsString(_display, _screen)); } @@ -497,6 +501,8 @@ do_get_extension_func(const char *name) { nassertr(name != nullptr, nullptr); if (glx_get_proc_address) { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + // First, check if we have glXGetProcAddress available. This will be // superior if we can get it. diff --git a/panda/src/glxdisplay/glxGraphicsWindow.cxx b/panda/src/glxdisplay/glxGraphicsWindow.cxx index 5e9a670cff..85757f2faa 100644 --- a/panda/src/glxdisplay/glxGraphicsWindow.cxx +++ b/panda/src/glxdisplay/glxGraphicsWindow.cxx @@ -154,6 +154,8 @@ end_flip() { */ void glxGraphicsWindow:: close_window() { + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + if (_gsg != nullptr) { glXMakeCurrent(_display, None, nullptr); _gsg.clear(); @@ -204,6 +206,8 @@ open_window() { return false; } + LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + if (glxgsg->_fbconfig != None) { setup_colormap(glxgsg->_fbconfig); } else { From 8ad0cb6b57073c763ada3e1718932c7e9625bff2 Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 22 Nov 2018 23:52:18 +0100 Subject: [PATCH 14/29] glgsg: add support for p3d_FragData fragment output This is necessary for GLSL 1.30 which deprecates gl_FragData but does not yet support layout(location=) specifiers Also fix some function pointer checks for pre-GL 3.0 Fixes #455 --- .../glstuff/glGraphicsStateGuardian_src.cxx | 42 +++++++++++++++---- .../src/glstuff/glGraphicsStateGuardian_src.h | 2 + panda/src/glstuff/glShaderContext_src.cxx | 5 +++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index c9a22615bf..4a80e7044a 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -1754,14 +1754,6 @@ reset() { get_extension_func("glUniform3iv"); _glUniform4iv = (PFNGLUNIFORM4IVPROC) get_extension_func("glUniform4iv"); - _glUniform1uiv = (PFNGLUNIFORM1UIVPROC) - get_extension_func("glUniform1uiv"); - _glUniform2uiv = (PFNGLUNIFORM2UIVPROC) - get_extension_func("glUniform2uiv"); - _glUniform3uiv = (PFNGLUNIFORM3UIVPROC) - get_extension_func("glUniform3uiv"); - _glUniform4uiv = (PFNGLUNIFORM4UIVPROC) - get_extension_func("glUniform4uiv"); _glUniformMatrix3fv = (PFNGLUNIFORMMATRIX3FVPROC) get_extension_func("glUniformMatrix3fv"); _glUniformMatrix4fv = (PFNGLUNIFORMMATRIX4FVPROC) @@ -1776,9 +1768,35 @@ reset() { get_extension_func("glVertexAttribPointer"); if (is_at_least_gl_version(3, 0)) { + _glBindFragDataLocation = (PFNGLBINDFRAGDATALOCATIONPROC) + get_extension_func("glBindFragDataLocation"); _glVertexAttribIPointer = (PFNGLVERTEXATTRIBIPOINTERPROC) get_extension_func("glVertexAttribIPointer"); + _glUniform1uiv = (PFNGLUNIFORM1UIVPROC) + get_extension_func("glUniform1uiv"); + _glUniform2uiv = (PFNGLUNIFORM2UIVPROC) + get_extension_func("glUniform2uiv"); + _glUniform3uiv = (PFNGLUNIFORM3UIVPROC) + get_extension_func("glUniform3uiv"); + _glUniform4uiv = (PFNGLUNIFORM4UIVPROC) + get_extension_func("glUniform4uiv"); + + } else if (has_extension("GL_EXT_gpu_shader4")) { + _glBindFragDataLocation = (PFNGLBINDFRAGDATALOCATIONPROC) + get_extension_func("glBindFragDataLocationEXT"); + _glVertexAttribIPointer = (PFNGLVERTEXATTRIBIPOINTERPROC) + get_extension_func("glVertexAttribIPointerEXT"); + _glUniform1uiv = (PFNGLUNIFORM1UIVPROC) + get_extension_func("glUniform1uivEXT"); + _glUniform2uiv = (PFNGLUNIFORM2UIVPROC) + get_extension_func("glUniform2uivEXT"); + _glUniform3uiv = (PFNGLUNIFORM3UIVPROC) + get_extension_func("glUniform3uivEXT"); + _glUniform4uiv = (PFNGLUNIFORM4UIVPROC) + get_extension_func("glUniform4uivEXT"); + } else { + _glBindFragDataLocation = nullptr; _glVertexAttribIPointer = nullptr; } if (is_at_least_gl_version(4, 1) || @@ -1807,6 +1825,7 @@ reset() { _glVertexAttribPointer = (PFNGLVERTEXATTRIBPOINTERPROC) get_extension_func("glVertexAttribPointerARB"); + _glBindFragDataLocation = nullptr; _glVertexAttribIPointer = nullptr; _glVertexAttribLPointer = nullptr; } @@ -1858,6 +1877,13 @@ reset() { } else { _glVertexAttribIPointer = nullptr; } + + if (has_extension("GL_EXT_blend_func_extended")) { + _glBindFragDataLocation = (PFNGLBINDFRAGDATALOCATIONPROC) + get_extension_func("glBindFragDataLocationEXT"); + } else { + _glBindFragDataLocation = nullptr; + } #endif #ifndef OPENGLES_1 diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.h b/panda/src/glstuff/glGraphicsStateGuardian_src.h index f4fda4ab9e..4329938d06 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.h +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.h @@ -146,6 +146,7 @@ typedef void (APIENTRYP PFNGLBLENDFUNCSEPARATEPROC) (GLenum sfactorRGB, GLenum d // GLSL shader functions typedef void (APIENTRYP PFNGLATTACHSHADERPROC) (GLuint program, GLuint shader); typedef void (APIENTRYP PFNGLBINDATTRIBLOCATIONPROC) (GLuint program, GLuint index, const GLchar *name); +typedef void (APIENTRYP PFNGLBINDFRAGDATALOCATIONPROC) (GLuint program, GLuint color, const GLchar *name); typedef void (APIENTRYP PFNGLCOMPILESHADERPROC) (GLuint shader); typedef GLuint (APIENTRYP PFNGLCREATEPROGRAMPROC) (void); typedef GLuint (APIENTRYP PFNGLCREATESHADERPROC) (GLenum type); @@ -963,6 +964,7 @@ public: // GLSL functions PFNGLATTACHSHADERPROC _glAttachShader; PFNGLBINDATTRIBLOCATIONPROC _glBindAttribLocation; + PFNGLBINDFRAGDATALOCATIONPROC _glBindFragDataLocation; PFNGLCOMPILESHADERPROC _glCompileShader; PFNGLCREATEPROGRAMPROC _glCreateProgram; PFNGLCREATESHADERPROC _glCreateShader; diff --git a/panda/src/glstuff/glShaderContext_src.cxx b/panda/src/glstuff/glShaderContext_src.cxx index b3a608d068..bf6c6097fa 100644 --- a/panda/src/glstuff/glShaderContext_src.cxx +++ b/panda/src/glstuff/glShaderContext_src.cxx @@ -3219,6 +3219,11 @@ glsl_compile_and_link() { _glgsg->_glBindAttribLocation(_glsl_program, 8, "texcoord"); } + // Also bind the p3d_FragData array to the first index always. + if (_glgsg->_glBindFragDataLocation != nullptr) { + _glgsg->_glBindFragDataLocation(_glsl_program, 0, "p3d_FragData"); + } + // If we requested to retrieve the shader, we should indicate that before // linking. bool retrieve_binary = false; From bafb0ac3dbe7683737081b70dcc70ee876e78e9e Mon Sep 17 00:00:00 2001 From: rdb Date: Fri, 23 Nov 2018 00:12:22 +0100 Subject: [PATCH 15/29] x11display: add x-init-threads var to call XInitThreads() This is off by default, but could be used if you stumble upon a race condition issue with X11 and threading. --- panda/src/x11display/config_x11display.cxx | 5 +++++ panda/src/x11display/config_x11display.h | 1 + panda/src/x11display/x11GraphicsPipe.cxx | 6 ++++++ 3 files changed, 12 insertions(+) diff --git a/panda/src/x11display/config_x11display.cxx b/panda/src/x11display/config_x11display.cxx index 4a633303ba..d31c4c1c23 100644 --- a/panda/src/x11display/config_x11display.cxx +++ b/panda/src/x11display/config_x11display.cxx @@ -40,6 +40,11 @@ ConfigVariableBool x_error_abort "of an error from the X window system. This can make it easier " "to discover where these errors are generated.")); +ConfigVariableBool x_init_threads +("x-init-threads", false, + PRC_DESC("Set this true to ask Panda3D to call XInitThreads() upon loading " + "the display module, which may help with some threading issues.")); + ConfigVariableInt x_wheel_up_button ("x-wheel-up-button", 4, PRC_DESC("This is the mouse button index of the wheel_up event: which " diff --git a/panda/src/x11display/config_x11display.h b/panda/src/x11display/config_x11display.h index bc40aad627..157f09c5fc 100644 --- a/panda/src/x11display/config_x11display.h +++ b/panda/src/x11display/config_x11display.h @@ -26,6 +26,7 @@ extern EXPCL_PANDAX11 void init_libx11display(); extern ConfigVariableString display_cfg; extern ConfigVariableBool x_error_abort; +extern ConfigVariableBool x_init_threads; extern ConfigVariableInt x_wheel_up_button; extern ConfigVariableInt x_wheel_down_button; diff --git a/panda/src/x11display/x11GraphicsPipe.cxx b/panda/src/x11display/x11GraphicsPipe.cxx index 2348d58706..479b3fde0c 100644 --- a/panda/src/x11display/x11GraphicsPipe.cxx +++ b/panda/src/x11display/x11GraphicsPipe.cxx @@ -66,6 +66,12 @@ x11GraphicsPipe(const std::string &display) : _im = (XIM)nullptr; _hidden_cursor = None; + // According to the documentation, we should call this before making any + // other Xlib calls if we wish to use the Xlib locking system. + if (x_init_threads) { + XInitThreads(); + } + install_error_handlers(); _display = XOpenDisplay(display_spec.c_str()); From 3f91615a2263f95fda40ae5cc427bcf67e2064f5 Mon Sep 17 00:00:00 2001 From: rdb Date: Fri, 23 Nov 2018 00:22:34 +0100 Subject: [PATCH 16/29] glgsg: reset color write mask before calling draw callback --- panda/src/glstuff/glGraphicsStateGuardian_src.cxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 4a80e7044a..8e9105de1c 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -3754,6 +3754,10 @@ clear_before_callback() { _glClientActiveTexture(GL_TEXTURE0); #endif + // It's also quite reasonable to presume there aren't any funny color write + // mask settings active. + clear_color_write_mask(); + // Clear the bound sampler object, so that we do not inadvertently override // the callback's desired sampler settings. #ifndef OPENGLES_1 From e32388c2f83e79dadf8b2b8b23fd34649eb1f7ce Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 24 Nov 2018 22:44:09 +0100 Subject: [PATCH 17/29] interrogate: fix crash reading static property --- dtool/src/interrogate/interfaceMakerPythonNative.cxx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index fd8b5432cd..59575668d4 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/dtool/src/interrogate/interfaceMakerPythonNative.cxx @@ -6975,7 +6975,11 @@ write_getset(ostream &out, Object *obj, Property *property) { out << " if (wrap != nullptr) {\n" " wrap->_getitem_func = &Dtool_" << ClassName << "_" << ielem.get_name() << "_Mapping_Getitem;\n"; if (!property->_setter_remaps.empty()) { - out << " if (!DtoolInstance_IS_CONST(self)) {\n"; + if (property->_has_this) { + out << " if (!DtoolInstance_IS_CONST(self)) {\n"; + } else { + out << " {\n"; + } out << " wrap->_setitem_func = &Dtool_" << ClassName << "_" << ielem.get_name() << "_Mapping_Setitem;\n"; out << " }\n"; } @@ -7006,7 +7010,11 @@ write_getset(ostream &out, Object *obj, Property *property) { " wrap->_len_func = &Dtool_" << ClassName << "_" << ielem.get_name() << "_Len;\n" " wrap->_getitem_func = &Dtool_" << ClassName << "_" << ielem.get_name() << "_Sequence_Getitem;\n"; if (!property->_setter_remaps.empty()) { - out << " if (!DtoolInstance_IS_CONST(self)) {\n"; + if (property->_has_this) { + out << " if (!DtoolInstance_IS_CONST(self)) {\n"; + } else { + out << " {\n"; + } out << " wrap->_setitem_func = &Dtool_" << ClassName << "_" << ielem.get_name() << "_Sequence_Setitem;\n"; if (property->_inserter != nullptr) { out << " wrap->_insert_func = &Dtool_" << ClassName << "_" << ielem.get_name() << "_Sequence_insert;\n"; From 544ef137ee927ea51c3ed697578732d965668512 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 24 Nov 2018 22:44:55 +0100 Subject: [PATCH 18/29] x11display: fix crash with multithreading and NVIDIA driver --- panda/src/x11display/x11GraphicsWindow.cxx | 17 +++++++++++++++++ panda/src/x11display/x11GraphicsWindow.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/panda/src/x11display/x11GraphicsWindow.cxx b/panda/src/x11display/x11GraphicsWindow.cxx index 17f6ca0a2a..9a1555c27e 100644 --- a/panda/src/x11display/x11GraphicsWindow.cxx +++ b/panda/src/x11display/x11GraphicsWindow.cxx @@ -218,6 +218,23 @@ move_pointer(int device, int x, int y) { } } +/** + * Clears the entire framebuffer before rendering, according to the settings + * of get_color_clear_active() and get_depth_clear_active() (inherited from + * DrawableRegion). + * + * This function is called only within the draw thread. + */ +void x11GraphicsWindow:: +clear(Thread *current_thread) { + if (is_any_clear_active()) { + // Evidently the NVIDIA driver may call glXCreateNewContext inside + // prepare_display_region, so we need to hold the X11 lock. + LightReMutexHolder holder(x11GraphicsPipe::_x_mutex); + GraphicsOutput::clear(current_thread); + } +} + /** * This function will be called within the draw thread before beginning * rendering for a given frame. It should do whatever setup is required, and diff --git a/panda/src/x11display/x11GraphicsWindow.h b/panda/src/x11display/x11GraphicsWindow.h index 078b016262..906a64b623 100644 --- a/panda/src/x11display/x11GraphicsWindow.h +++ b/panda/src/x11display/x11GraphicsWindow.h @@ -36,6 +36,8 @@ public: virtual MouseData get_pointer(int device) const; virtual bool move_pointer(int device, int x, int y); + + virtual void clear(Thread *current_thread); virtual bool begin_frame(FrameMode mode, Thread *current_thread); virtual void end_frame(FrameMode mode, Thread *current_thread); From 7c0a77af78cbdba4b6e136ee9775c4e1259d3377 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 24 Nov 2018 22:45:41 +0100 Subject: [PATCH 19/29] display: disable depth test before DisplayRegion draw callback Having depth test disabled is the default OpenGL state, and callbacks may quite reasonably expect to see the default state. Kivy seems to expect this, for one. --- panda/src/display/graphicsEngine.cxx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/panda/src/display/graphicsEngine.cxx b/panda/src/display/graphicsEngine.cxx index dd160d55b5..c06c2b207f 100644 --- a/panda/src/display/graphicsEngine.cxx +++ b/panda/src/display/graphicsEngine.cxx @@ -47,6 +47,7 @@ #include "displayRegionCullCallbackData.h" #include "displayRegionDrawCallbackData.h" #include "callbackGraphicsWindow.h" +#include "depthTestAttrib.h" #if defined(WIN32) #define WINDOWS_LEAN_AND_MEAN @@ -2046,9 +2047,12 @@ do_draw(GraphicsOutput *win, GraphicsStateGuardian *gsg, DisplayRegion *dr, Thre if (cbobj != nullptr) { // Issue the draw callback on this DisplayRegion. - // Set the GSG to the initial state. + // Set the GSG to the initial state. We disable depth testing since that + // is the default OpenGL state, and some libraries (eg. Kivy) expect that. + static CPT(RenderState) state = RenderState::make( + DepthTestAttrib::make(DepthTestAttrib::M_none)); gsg->clear_before_callback(); - gsg->set_state_and_transform(RenderState::make_empty(), TransformState::make_identity()); + gsg->set_state_and_transform(state, TransformState::make_identity()); DisplayRegionDrawCallbackData cbdata(cull_result, scene_setup); cbobj->do_callback(&cbdata); From 272f13023e24dfd84ebc6f9ba3240bd471537ac0 Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 25 Nov 2018 15:26:12 +0100 Subject: [PATCH 20/29] glgsg: unbind buffers after draw callback Some libraries (eg. Kivy) leave their buffers bound, so this takes care of that. --- panda/src/glstuff/glGraphicsStateGuardian_src.cxx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 8e9105de1c..9118366a93 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -10692,6 +10692,20 @@ reissue_transforms() { memset(_vertex_attrib_columns, 0, sizeof(const GeomVertexColumn *) * 32); #endif + // Some libraries (Kivy) leave their buffers bound. How clumsy of them. + if (_supports_buffers) { + _glBindBuffer(GL_ARRAY_BUFFER, 0); + _glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0); + _current_vbuffer_index = 0; + _current_ibuffer_index = 0; + } +#ifndef OPENGLES + if (_supports_glsl) { + _glDisableVertexAttribArray(0); + _glDisableVertexAttribArray(1); + } +#endif + // Since this is called by clear_state_and_transform(), we also should reset // the states that won't automatically be respecified when clearing the // state mask. From c427357db9910839982f5a11112f872b54792cf9 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 27 Nov 2018 17:09:22 +0100 Subject: [PATCH 21/29] pnmimage: fix PixelSpec coercion, add PixelSpec unit test --- panda/src/pnmimage/pnmImageHeader.I | 23 ----------------------- panda/src/pnmimage/pnmImageHeader.h | 5 +++-- tests/pnmimage/test_pnmimage.py | 21 +++++++++++++++++++++ 3 files changed, 24 insertions(+), 25 deletions(-) create mode 100644 tests/pnmimage/test_pnmimage.py diff --git a/panda/src/pnmimage/pnmImageHeader.I b/panda/src/pnmimage/pnmImageHeader.I index 0e4adbaf11..630e41ff2a 100644 --- a/panda/src/pnmimage/pnmImageHeader.I +++ b/panda/src/pnmimage/pnmImageHeader.I @@ -295,29 +295,6 @@ PixelSpec(const xel &rgb, xelval alpha) : { } -/** - * - */ -INLINE PNMImageHeader::PixelSpec:: -PixelSpec(const PixelSpec ©) : - _red(copy._red), - _green(copy._green), - _blue(copy._blue), - _alpha(copy._alpha) -{ -} - -/** - * - */ -INLINE void PNMImageHeader::PixelSpec:: -operator = (const PixelSpec ©) { - _red = copy._red; - _green = copy._green; - _blue = copy._blue; - _alpha = copy._alpha; -} - /** * */ diff --git a/panda/src/pnmimage/pnmImageHeader.h b/panda/src/pnmimage/pnmImageHeader.h index 162dd21784..7030e06d2e 100644 --- a/panda/src/pnmimage/pnmImageHeader.h +++ b/panda/src/pnmimage/pnmImageHeader.h @@ -113,6 +113,9 @@ PUBLISHED: // make_histogram(). Note that pixels are stored by integer value, not by // floating-point scaled value. class EXPCL_PANDA_PNMIMAGE PixelSpec { + public: + INLINE PixelSpec() = default; + PUBLISHED: INLINE PixelSpec(xelval gray_value); INLINE PixelSpec(xelval gray_value, xelval alpha); @@ -120,8 +123,6 @@ PUBLISHED: INLINE PixelSpec(xelval red, xelval green, xelval blue, xelval alpha); INLINE PixelSpec(const xel &rgb); INLINE PixelSpec(const xel &rgb, xelval alpha); - INLINE PixelSpec(const PixelSpec ©); - INLINE void operator = (const PixelSpec ©); INLINE bool operator < (const PixelSpec &other) const; INLINE bool operator == (const PixelSpec &other) const; diff --git a/tests/pnmimage/test_pnmimage.py b/tests/pnmimage/test_pnmimage.py new file mode 100644 index 0000000000..0826e4f0b8 --- /dev/null +++ b/tests/pnmimage/test_pnmimage.py @@ -0,0 +1,21 @@ +from panda3d.core import PNMImage, PNMImageHeader + + +def test_pixelspec_ctor(): + assert tuple(PNMImage.PixelSpec(1)) == (1, 1, 1, 0) + assert tuple(PNMImage.PixelSpec(1, 2)) == (1, 1, 1, 2) + assert tuple(PNMImage.PixelSpec(1, 2, 3)) == (1, 2, 3, 0) + assert tuple(PNMImage.PixelSpec(1, 2, 3, 4)) == (1, 2, 3, 4) + + assert tuple(PNMImage.PixelSpec((1, 2, 3))) == (1, 2, 3, 0) + assert tuple(PNMImage.PixelSpec((1, 2, 3), 4)) == (1, 2, 3, 4) + + # Copy constructor + spec = PNMImage.PixelSpec(1, 2, 3, 4) + assert tuple(PNMImage.PixelSpec(spec)) == (1, 2, 3, 4) + + +def test_pixelspec_coerce(): + img = PNMImage(1, 1, 4) + img.set_pixel(0, 0, (1, 2, 3, 4)) + assert img.get_pixel(0, 0) == (1, 2, 3, 4) From da079c5ffea85c110627044a920a3ffcd147bba0 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 27 Nov 2018 17:09:46 +0100 Subject: [PATCH 22/29] glxdisplay: remove lock in dtor, which causes crash on shutdown --- panda/src/glxdisplay/glxGraphicsStateGuardian.cxx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx b/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx index 86ddeac589..d620427afa 100644 --- a/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx +++ b/panda/src/glxdisplay/glxGraphicsStateGuardian.cxx @@ -64,7 +64,9 @@ glxGraphicsStateGuardian(GraphicsEngine *engine, GraphicsPipe *pipe, */ glxGraphicsStateGuardian:: ~glxGraphicsStateGuardian() { - LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); + // Actually, the lock might have already destructed, so we can't reliably + // grab the X11 lock here. + //LightReMutexHolder holder(glxGraphicsPipe::_x_mutex); destroy_temp_xwindow(); if (_visuals != nullptr) { XFree(_visuals); From 5dd0db300b78dcc652908c84952b07fb643f4516 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 27 Nov 2018 20:59:51 +0100 Subject: [PATCH 23/29] flac: fix leak; properly close stream upon closing FlacAudioCursor --- panda/src/movies/flacAudioCursor.cxx | 6 +++++- panda/src/movies/flacAudioCursor.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/panda/src/movies/flacAudioCursor.cxx b/panda/src/movies/flacAudioCursor.cxx index 19eb71cbc9..0e3bbb163a 100644 --- a/panda/src/movies/flacAudioCursor.cxx +++ b/panda/src/movies/flacAudioCursor.cxx @@ -59,7 +59,8 @@ FlacAudioCursor:: FlacAudioCursor(FlacAudio *src, std::istream *stream) : MovieAudioCursor(src), _is_valid(false), - _drflac(nullptr) + _drflac(nullptr), + _stream(stream) { nassertv(stream != nullptr); nassertv(stream->good()); @@ -91,6 +92,9 @@ FlacAudioCursor:: if (_drflac != nullptr) { drflac_close(_drflac); } + if (_stream != nullptr) { + VirtualFileSystem::close_read_file(_stream); + } } /** diff --git a/panda/src/movies/flacAudioCursor.h b/panda/src/movies/flacAudioCursor.h index 55d59d5488..1de46bad01 100644 --- a/panda/src/movies/flacAudioCursor.h +++ b/panda/src/movies/flacAudioCursor.h @@ -41,6 +41,7 @@ public: protected: drflac *_drflac; + std::istream *_stream; public: static TypeHandle get_class_type() { From 7ed9655e06c3e65847493246fb4f09851e0919d9 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 27 Nov 2018 21:11:57 +0100 Subject: [PATCH 24/29] openal: fix leak of sound data when uncache_sound on stream This is an addendum to cd2ea97b1ffb65512f5ee8ba0665f46345ef7795 (fix for #428) which did not properly delete the SoundData when uncaching sounds that were loaded as streams. --- panda/src/audiotraits/openalAudioManager.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/panda/src/audiotraits/openalAudioManager.cxx b/panda/src/audiotraits/openalAudioManager.cxx index c205f55402..edf7dfe222 100644 --- a/panda/src/audiotraits/openalAudioManager.cxx +++ b/panda/src/audiotraits/openalAudioManager.cxx @@ -553,6 +553,7 @@ uncache_sound(const Filename &file_name) { if (sd->_movie->get_filename() == path || sd->_movie->get_filename() == file_name) { exqi = _expiring_streams.erase(exqi); + delete sd; continue; } } From 32df05b5286d3e798576c4ea2ff026c94dce90fa Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 27 Nov 2018 21:16:03 +0100 Subject: [PATCH 25/29] Fix crash when unmounting/closing multifile while streams are open It's not really reasonable to expect a user to find every occurrence of a cached resource that might be using an open stream and remove it or crash otherwise. This is fixed by keeping the multifile stream open as long as any substreams are still pointing to it, using a simplified reference counting (care is taken not to fully make StreamWrapper reference-counted, since it's not in express and existing uses should not be broken). Fixes #449 Also see #428 --- dtool/src/prc/streamWrapper.I | 18 ++++++++++++++++++ dtool/src/prc/streamWrapper.h | 10 ++++++++++ panda/src/express/multifile.cxx | 5 ++++- panda/src/express/subStreamBuf.cxx | 13 +++++++++++++ panda/src/express/subStreamBuf.h | 1 + 5 files changed, 46 insertions(+), 1 deletion(-) diff --git a/dtool/src/prc/streamWrapper.I b/dtool/src/prc/streamWrapper.I index 0f9d65d414..6d2578262c 100644 --- a/dtool/src/prc/streamWrapper.I +++ b/dtool/src/prc/streamWrapper.I @@ -58,6 +58,24 @@ release() { _lock.unlock(); } +/** + * Increments the reference count. Only has impact if the class that manages + * this StreamWrapper's lifetime (eg. Multifile) respects it. + */ +INLINE void StreamWrapperBase:: +ref() const { + AtomicAdjust::inc(_ref_count); +} + +/** + * Decrements the reference count. Only has impact if the class that manages + * this StreamWrapper's lifetime (eg. Multifile) respects it. + */ +INLINE bool StreamWrapperBase:: +unref() const { + return AtomicAdjust::dec(_ref_count); +} + /** * */ diff --git a/dtool/src/prc/streamWrapper.h b/dtool/src/prc/streamWrapper.h index b5a4d5bf72..a837540129 100644 --- a/dtool/src/prc/streamWrapper.h +++ b/dtool/src/prc/streamWrapper.h @@ -16,6 +16,7 @@ #include "dtoolbase.h" #include "mutexImpl.h" +#include "atomicAdjust.h" /** * The base class for both IStreamWrapper and OStreamWrapper, this provides @@ -30,8 +31,17 @@ PUBLISHED: INLINE void acquire(); INLINE void release(); +public: + INLINE void ref() const; + INLINE bool unref() const; + private: MutexImpl _lock; + + // This isn't really designed as a reference counted class, but it is useful + // to treat it as one when dealing with substreams created by Multifile. + mutable AtomicAdjust::Integer _ref_count = 1; + #ifdef SIMPLE_THREADS // In the SIMPLE_THREADS case, we need to use a bool flag, because MutexImpl // defines to nothing in this case--but we still need to achieve a form of diff --git a/panda/src/express/multifile.cxx b/panda/src/express/multifile.cxx index e625861f87..cdb1c56ec8 100644 --- a/panda/src/express/multifile.cxx +++ b/panda/src/express/multifile.cxx @@ -333,7 +333,10 @@ close() { if (_owns_stream) { // We prefer to delete the IStreamWrapper over the ostream, if possible. if (_read != nullptr) { - delete _read; + // Only delete it if no SubStream is still referencing it. + if (!_read->unref()) { + delete _read; + } } else if (_write != nullptr) { delete _write; } diff --git a/panda/src/express/subStreamBuf.cxx b/panda/src/express/subStreamBuf.cxx index 4d773e359f..14a05f1bfb 100644 --- a/panda/src/express/subStreamBuf.cxx +++ b/panda/src/express/subStreamBuf.cxx @@ -88,6 +88,13 @@ open(IStreamWrapper *source, OStreamWrapper *dest, streampos start, streampos en _append = append; _gpos = _start; _ppos = _start; + + if (source != nullptr) { + source->ref(); + } + if (dest != nullptr) { + dest->ref(); + } } /** @@ -98,6 +105,12 @@ close() { // Make sure the write buffer is flushed. sync(); + if (_source != nullptr && !_source->unref()) { + delete _source; + } + if (_dest != nullptr && !_dest->unref()) { + delete _dest; + } _source = nullptr; _dest = nullptr; _start = 0; diff --git a/panda/src/express/subStreamBuf.h b/panda/src/express/subStreamBuf.h index 779f43d94c..1f0c511e51 100644 --- a/panda/src/express/subStreamBuf.h +++ b/panda/src/express/subStreamBuf.h @@ -23,6 +23,7 @@ class EXPCL_PANDA_EXPRESS SubStreamBuf : public std::streambuf { public: SubStreamBuf(); + SubStreamBuf(const SubStreamBuf ©) = delete; virtual ~SubStreamBuf(); void open(IStreamWrapper *source, OStreamWrapper *dest, std::streampos start, std::streampos end, bool append); From 85cb742f79bf718f2086070f86f5dde6e2b1d504 Mon Sep 17 00:00:00 2001 From: rdb Date: Wed, 28 Nov 2018 16:14:45 +0100 Subject: [PATCH 26/29] ffmpeg: drain avcodec contexts on close, fixes leak Fixes #398 --- panda/src/ffmpeg/ffmpegAudioCursor.cxx | 27 ++++++++++++++++---------- panda/src/ffmpeg/ffmpegVideoCursor.cxx | 9 ++++++++- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/panda/src/ffmpeg/ffmpegAudioCursor.cxx b/panda/src/ffmpeg/ffmpegAudioCursor.cxx index 809797fb85..4b17800c2b 100644 --- a/panda/src/ffmpeg/ffmpegAudioCursor.cxx +++ b/panda/src/ffmpeg/ffmpegAudioCursor.cxx @@ -210,6 +210,23 @@ FfmpegAudioCursor:: */ void FfmpegAudioCursor:: cleanup() { + if (_audio_ctx && _audio_ctx->codec) { +#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(57, 37, 100) + // We need to drain the codec to prevent a memory leak. + avcodec_send_packet(_audio_ctx, nullptr); + while (avcodec_receive_frame(_audio_ctx, _frame) == 0) {} + avcodec_flush_buffers(_audio_ctx); +#endif + + avcodec_close(_audio_ctx); +#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(55, 52, 0) + avcodec_free_context(&_audio_ctx); +#else + delete _audio_ctx; +#endif + } + _audio_ctx = nullptr; + if (_frame) { #if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(55, 45, 101) av_frame_free(&_frame); @@ -237,16 +254,6 @@ cleanup() { _buffer = nullptr; } - if ((_audio_ctx)&&(_audio_ctx->codec)) { - avcodec_close(_audio_ctx); -#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(55, 52, 0) - avcodec_free_context(&_audio_ctx); -#else - delete _audio_ctx; -#endif - } - _audio_ctx = nullptr; - if (_format_ctx) { _ffvfile.close(); _format_ctx = nullptr; diff --git a/panda/src/ffmpeg/ffmpegVideoCursor.cxx b/panda/src/ffmpeg/ffmpegVideoCursor.cxx index 47c6a54ae2..ef23a9414e 100644 --- a/panda/src/ffmpeg/ffmpegVideoCursor.cxx +++ b/panda/src/ffmpeg/ffmpegVideoCursor.cxx @@ -595,7 +595,14 @@ close_stream() { // Hold the global lock while we free avcodec objects. ReMutexHolder av_holder(_av_lock); - if ((_video_ctx)&&(_video_ctx->codec)) { + if (_video_ctx && _video_ctx->codec) { +#if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(57, 37, 100) + // We need to drain the codec to prevent a memory leak. + avcodec_send_packet(_video_ctx, nullptr); + while (avcodec_receive_frame(_video_ctx, _frame) == 0) {} + avcodec_flush_buffers(_video_ctx); +#endif + avcodec_close(_video_ctx); #if LIBAVCODEC_VERSION_INT >= AV_VERSION_INT(55, 52, 0) avcodec_free_context(&_video_ctx); From 69f8f8b7b74a8930de6915d43fa411f30d3ca8e8 Mon Sep 17 00:00:00 2001 From: rdb Date: Wed, 28 Nov 2018 16:22:27 +0100 Subject: [PATCH 27/29] ffmpeg: remove call deprecated in ffmpeg's libavformat 58.9.100 --- panda/src/ffmpeg/ffmpegVirtualFile.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/panda/src/ffmpeg/ffmpegVirtualFile.cxx b/panda/src/ffmpeg/ffmpegVirtualFile.cxx index 8ca576cd90..09c5092844 100644 --- a/panda/src/ffmpeg/ffmpegVirtualFile.cxx +++ b/panda/src/ffmpeg/ffmpegVirtualFile.cxx @@ -189,7 +189,10 @@ register_protocol() { } // Here's a good place to call this global ffmpeg initialization function. + // However, ffmpeg (but not libav) deprecated this, hence this check. +#if LIBAVFORMAT_VERSION_MICRO < 100 || LIBAVFORMAT_VERSION_INT < AV_VERSION_INT(58, 9, 100) av_register_all(); +#endif // And this one. avformat_network_init(); From 594e6b394b199c9295e6c0ed728b2c82c6f94e47 Mon Sep 17 00:00:00 2001 From: rdb Date: Wed, 28 Nov 2018 16:46:49 +0100 Subject: [PATCH 28/29] chan: add various property interfaces to animation system --- panda/src/chan/animBundle.h | 3 +++ panda/src/chan/animBundleNode.h | 2 ++ panda/src/chan/animChannelMatrixDynamic.h | 2 ++ panda/src/chan/animChannelMatrixXfmTable.h | 2 ++ panda/src/chan/animChannelScalarDynamic.I | 22 +++++++++++++++++++++ panda/src/chan/animChannelScalarDynamic.cxx | 13 +++++------- panda/src/chan/animChannelScalarDynamic.h | 5 +++++ panda/src/chan/animChannelScalarTable.h | 2 ++ 8 files changed, 43 insertions(+), 8 deletions(-) diff --git a/panda/src/chan/animBundle.h b/panda/src/chan/animBundle.h index 659f15e58c..77c07e26d7 100644 --- a/panda/src/chan/animBundle.h +++ b/panda/src/chan/animBundle.h @@ -38,6 +38,9 @@ PUBLISHED: INLINE double get_base_frame_rate() const; INLINE int get_num_frames() const; + MAKE_PROPERTY(base_frame_rate, get_base_frame_rate); + MAKE_PROPERTY(num_frames, get_num_frames); + virtual void output(std::ostream &out) const; protected: diff --git a/panda/src/chan/animBundleNode.h b/panda/src/chan/animBundleNode.h index fbf9dcbbe2..6152d37707 100644 --- a/panda/src/chan/animBundleNode.h +++ b/panda/src/chan/animBundleNode.h @@ -41,6 +41,8 @@ public: PUBLISHED: INLINE AnimBundle *get_bundle() const; + MAKE_PROPERTY(bundle, get_bundle); + static AnimBundle *find_anim_bundle(PandaNode *root); private: diff --git a/panda/src/chan/animChannelMatrixDynamic.h b/panda/src/chan/animChannelMatrixDynamic.h index 589b2e7a10..5602f027c6 100644 --- a/panda/src/chan/animChannelMatrixDynamic.h +++ b/panda/src/chan/animChannelMatrixDynamic.h @@ -57,6 +57,8 @@ PUBLISHED: INLINE const TransformState *get_value_transform() const; INLINE PandaNode *get_value_node() const; + MAKE_PROPERTY(value_node, get_value_node, set_value_node); + protected: virtual AnimGroup *make_copy(AnimGroup *parent) const; diff --git a/panda/src/chan/animChannelMatrixXfmTable.h b/panda/src/chan/animChannelMatrixXfmTable.h index a5923e59c0..047f145453 100644 --- a/panda/src/chan/animChannelMatrixXfmTable.h +++ b/panda/src/chan/animChannelMatrixXfmTable.h @@ -59,6 +59,8 @@ PUBLISHED: INLINE bool has_table(char table_id) const; INLINE void clear_table(char table_id); + MAKE_MAP_PROPERTY(tables, has_table, get_table, set_table, clear_table); + public: virtual void write(std::ostream &out, int indent_level) const; diff --git a/panda/src/chan/animChannelScalarDynamic.I b/panda/src/chan/animChannelScalarDynamic.I index e165b58b44..549b9bfad7 100644 --- a/panda/src/chan/animChannelScalarDynamic.I +++ b/panda/src/chan/animChannelScalarDynamic.I @@ -10,3 +10,25 @@ * @author drose * @date 2003-10-20 */ + +/** + * Gets the value of the channel. This will return the value explicitly + * specified by set_value() unless a value node was specified using + * set_value_node(). + */ +INLINE PN_stdfloat AnimChannelScalarDynamic:: +get_value() const { + if (_value_node != nullptr) { + return _value->get_pos()[0]; + } else { + return _float_value; + } +} + +/** + * Returns the node that was set via set_value_node(), if any. + */ +INLINE PandaNode *AnimChannelScalarDynamic:: +get_value_node() const { + return _value_node; +} diff --git a/panda/src/chan/animChannelScalarDynamic.cxx b/panda/src/chan/animChannelScalarDynamic.cxx index 8a9835a550..c69ad0e6b9 100644 --- a/panda/src/chan/animChannelScalarDynamic.cxx +++ b/panda/src/chan/animChannelScalarDynamic.cxx @@ -84,16 +84,12 @@ has_changed(int, double, int, double) { */ void AnimChannelScalarDynamic:: get_value(int, PN_stdfloat &value) { - if (_value_node != nullptr) { - value = _value->get_pos()[0]; - - } else { - value = _float_value; - } + value = get_value(); } /** - * Explicitly sets the value. + * Explicitly sets the value. This will remove any node assigned via + * set_value_node(). */ void AnimChannelScalarDynamic:: set_value(PN_stdfloat value) { @@ -104,7 +100,8 @@ set_value(PN_stdfloat value) { /** * Specifies a node whose transform will be queried each frame to implicitly - * specify the transform of this joint. + * specify the transform of this joint. This will override the values set by + * set_value(). */ void AnimChannelScalarDynamic:: set_value_node(PandaNode *value_node) { diff --git a/panda/src/chan/animChannelScalarDynamic.h b/panda/src/chan/animChannelScalarDynamic.h index ab009a4f1f..7455adb4b9 100644 --- a/panda/src/chan/animChannelScalarDynamic.h +++ b/panda/src/chan/animChannelScalarDynamic.h @@ -41,11 +41,16 @@ public: virtual bool has_changed(int last_frame, double last_frac, int this_frame, double this_frac); virtual void get_value(int frame, PN_stdfloat &value); + INLINE PN_stdfloat get_value() const; + INLINE PandaNode *get_value_node() const; PUBLISHED: void set_value(PN_stdfloat value); void set_value_node(PandaNode *node); + MAKE_PROPERTY(value, get_value, set_value); + MAKE_PROPERTY(value_node, get_value_node, set_value_node); + protected: virtual AnimGroup *make_copy(AnimGroup *parent) const; diff --git a/panda/src/chan/animChannelScalarTable.h b/panda/src/chan/animChannelScalarTable.h index e150d8d82e..10e54815e3 100644 --- a/panda/src/chan/animChannelScalarTable.h +++ b/panda/src/chan/animChannelScalarTable.h @@ -44,6 +44,8 @@ PUBLISHED: INLINE bool has_table() const; INLINE void clear_table(); + MAKE_PROPERTY2(table, has_table, get_table, set_table, clear_table); + public: virtual void write(std::ostream &out, int indent_level) const; From 97d4e32a067839130fabc0e3341db56a13127c27 Mon Sep 17 00:00:00 2001 From: rdb Date: Wed, 28 Nov 2018 17:35:20 +0100 Subject: [PATCH 29/29] general: use nassert_raise instead of nassertv(false) et al Even a brief error message in the assertion is infinitely more useful to a user who is not at home in the source code, especially for assertions that may reasonably be triggered by honest user mistakes. --- direct/src/dcparser/dcClass.cxx | 2 +- dtool/src/prc/notifyCategory.cxx | 2 +- panda/src/chan/animChannelMatrixXfmTable.cxx | 2 +- panda/src/chan/animChannelScalarTable.cxx | 2 +- panda/src/display/graphicsStateGuardian.cxx | 2 +- panda/src/egg/eggGroup.cxx | 3 ++- panda/src/egg/eggVertexPool.cxx | 3 ++- panda/src/egg/eggXfmAnimData.cxx | 3 ++- panda/src/egg/eggXfmSAnim.cxx | 3 ++- panda/src/event/asyncFuture.cxx | 2 +- panda/src/event/pointerEvent.cxx | 4 ++-- panda/src/glstuff/glGeomContext_src.cxx | 2 +- .../glstuff/glGraphicsStateGuardian_src.cxx | 4 ++-- panda/src/gobj/geom.cxx | 2 +- panda/src/gobj/geomCacheManager.cxx | 2 +- panda/src/gobj/geomPrimitive.cxx | 8 +++---- panda/src/gobj/geomTrifans.cxx | 2 +- panda/src/gobj/geomVertexArrayData.cxx | 2 +- panda/src/gobj/geomVertexData.cxx | 3 ++- panda/src/gobj/shader.cxx | 2 +- panda/src/gobj/texture.cxx | 13 ++++++---- .../src/gsgbase/graphicsStateGuardianBase.cxx | 2 +- panda/src/nativenet/socket_address.I | 6 ++--- panda/src/nativenet/socket_address.cxx | 4 ++-- panda/src/net/connection.cxx | 3 ++- panda/src/net/datagramTCPHeader.cxx | 3 ++- .../parametrics/parametricCurveCollection.cxx | 2 +- panda/src/pgraph/clipPlaneAttrib.cxx | 8 +++---- panda/src/pgraph/cullBinManager.cxx | 2 +- panda/src/pgraph/lightAttrib.cxx | 8 +++---- panda/src/pgraph/nodePath.cxx | 9 ++++--- panda/src/pgraph/pandaNode.cxx | 3 ++- panda/src/pgraph/renderAttribRegistry.cxx | 2 +- panda/src/pgraph/sceneGraphReducer.cxx | 3 ++- panda/src/pgraphnodes/ambientLight.cxx | 2 +- panda/src/pipeline/pythonThread.cxx | 3 ++- panda/src/pnmimage/pfmFile.cxx | 24 ++++++++++++------- panda/src/pnmimage/pnmImage.cxx | 18 +++++++------- panda/src/pnmimagetypes/pnmFileTypePfm.cxx | 3 ++- .../pnmimagetypes/pnmFileTypeSGIWriter.cxx | 3 ++- panda/src/putil/sparseArray.I | 2 +- panda/src/text/textAssembler.cxx | 2 +- panda/src/vision/webcamVideoCursorV4L.cxx | 3 ++- pandatool/src/assimp/pandaIOSystem.cxx | 2 +- pandatool/src/eggcharbase/eggBackPointer.cxx | 2 +- 45 files changed, 108 insertions(+), 79 deletions(-) diff --git a/direct/src/dcparser/dcClass.cxx b/direct/src/dcparser/dcClass.cxx index 4c428db595..374ed47d3c 100644 --- a/direct/src/dcparser/dcClass.cxx +++ b/direct/src/dcparser/dcClass.cxx @@ -1240,7 +1240,7 @@ shadow_inherited_field(const string &name) { } // If we get here, the named field wasn't in the list. Huh. - nassertv(false); + nassert_raise("named field not in list"); } /** diff --git a/dtool/src/prc/notifyCategory.cxx b/dtool/src/prc/notifyCategory.cxx index 11a3c52587..3d856ceb08 100644 --- a/dtool/src/prc/notifyCategory.cxx +++ b/dtool/src/prc/notifyCategory.cxx @@ -110,7 +110,7 @@ out(NotifySeverity severity, bool prefix) const { nout << *this << "(" << severity << "): "; } if (assert_abort) { - nassertr(false, nout); + nassert_raise("unprotected debug statement"); } return nout; diff --git a/panda/src/chan/animChannelMatrixXfmTable.cxx b/panda/src/chan/animChannelMatrixXfmTable.cxx index c6c27b8b7a..5370a9833e 100644 --- a/panda/src/chan/animChannelMatrixXfmTable.cxx +++ b/panda/src/chan/animChannelMatrixXfmTable.cxx @@ -239,7 +239,7 @@ set_table(char table_id, const CPTA_stdfloat &table) { if (table.size() > 1 && (int)table.size() < num_frames) { // The new table has an invalid number of frames--it doesn't match the // bundle's requirement. - nassertv(false); + nassert_raise("mismatched number of frames"); return; } diff --git a/panda/src/chan/animChannelScalarTable.cxx b/panda/src/chan/animChannelScalarTable.cxx index 54953164e7..ae95b3d1ea 100644 --- a/panda/src/chan/animChannelScalarTable.cxx +++ b/panda/src/chan/animChannelScalarTable.cxx @@ -104,7 +104,7 @@ set_table(const CPTA_stdfloat &table) { if (table.size() > 1 && (int)table.size() < num_frames) { // The new table has an invalid number of frames--it doesn't match the // bundle's requirement. - nassertv(false); + nassert_raise("mismatched number of frames"); return; } diff --git a/panda/src/display/graphicsStateGuardian.cxx b/panda/src/display/graphicsStateGuardian.cxx index 93c732e5d7..1b0caeabd0 100644 --- a/panda/src/display/graphicsStateGuardian.cxx +++ b/panda/src/display/graphicsStateGuardian.cxx @@ -743,7 +743,7 @@ issue_timer_query(int pstats_index) { */ void GraphicsStateGuardian:: dispatch_compute(int num_groups_x, int num_groups_y, int num_groups_z) { - nassertv(false /* Compute shaders not supported by GSG */); + nassert_raise("Compute shaders not supported by GSG"); } /** diff --git a/panda/src/egg/eggGroup.cxx b/panda/src/egg/eggGroup.cxx index a5ff1dee06..6bc6f152f0 100644 --- a/panda/src/egg/eggGroup.cxx +++ b/panda/src/egg/eggGroup.cxx @@ -192,7 +192,8 @@ write(ostream &out, int indent_level) const { default: // invalid group type - nassertv(false); + nassert_raise("invalid EggGroup type"); + return; } if (is_of_type(EggBin::get_class_type())) { diff --git a/panda/src/egg/eggVertexPool.cxx b/panda/src/egg/eggVertexPool.cxx index 6f2768599e..65d5f747bf 100644 --- a/panda/src/egg/eggVertexPool.cxx +++ b/panda/src/egg/eggVertexPool.cxx @@ -445,7 +445,8 @@ add_vertex(EggVertex *vertex, int index) { } // Oops, you duplicated a vertex index. - nassertr(false, nullptr); + nassert_raise("duplicate vertex index"); + return nullptr; } _unique_vertices.insert(vertex); diff --git a/panda/src/egg/eggXfmAnimData.cxx b/panda/src/egg/eggXfmAnimData.cxx index 73a7128036..0216ea1bdd 100644 --- a/panda/src/egg/eggXfmAnimData.cxx +++ b/panda/src/egg/eggXfmAnimData.cxx @@ -142,7 +142,8 @@ get_value(int row, LMatrix4d &mat) const { default: // The contents string contained an invalid letter. - nassertv(false); + nassert_raise("invalid letter in contents string"); + return; } } diff --git a/panda/src/egg/eggXfmSAnim.cxx b/panda/src/egg/eggXfmSAnim.cxx index 43a784b33f..4158d46491 100644 --- a/panda/src/egg/eggXfmSAnim.cxx +++ b/panda/src/egg/eggXfmSAnim.cxx @@ -368,7 +368,8 @@ get_value(int row, LMatrix4d &mat) const { default: // One of the child tables had an invalid name. - nassertv(false); + nassert_raise("invalid name in child table"); + return; } } } diff --git a/panda/src/event/asyncFuture.cxx b/panda/src/event/asyncFuture.cxx index 16540954ed..4805650260 100644 --- a/panda/src/event/asyncFuture.cxx +++ b/panda/src/event/asyncFuture.cxx @@ -298,7 +298,7 @@ wake_task(AsyncTask *task) { return; default: - nassertv(false); + nassert_raise("unexpected task state"); return; } } diff --git a/panda/src/event/pointerEvent.cxx b/panda/src/event/pointerEvent.cxx index 7ee7d3621c..752210cf86 100644 --- a/panda/src/event/pointerEvent.cxx +++ b/panda/src/event/pointerEvent.cxx @@ -29,7 +29,7 @@ output(std::ostream &out) const { */ void PointerEvent:: write_datagram(Datagram &dg) const { - nassertv(false && "This function not implemented yet."); + nassert_raise("This function not implemented yet."); } /** @@ -37,5 +37,5 @@ write_datagram(Datagram &dg) const { */ void PointerEvent:: read_datagram(DatagramIterator &scan) { - nassertv(false && "This function not implemented yet."); + nassert_raise("This function not implemented yet."); } diff --git a/panda/src/glstuff/glGeomContext_src.cxx b/panda/src/glstuff/glGeomContext_src.cxx index 2640c1d855..f798dedb36 100644 --- a/panda/src/glstuff/glGeomContext_src.cxx +++ b/panda/src/glstuff/glGeomContext_src.cxx @@ -32,7 +32,7 @@ get_display_list(GLuint &index, const CLP(GeomMunger) *munger, UpdateSeq modified) { #if defined(OPENGLES) || !defined(SUPPORT_FIXED_FUNCTION) // Display lists not supported by OpenGL ES. - nassertr(false, false); + nassert_raise("OpenGL ES does not support display lists"); return false; #else diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 9118366a93..14fec5d146 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -14144,8 +14144,8 @@ extract_texture_image(PTA_uchar &image, size_t &page_size, Texture::ComponentType type, Texture::CompressionMode compression, int n) { #ifdef OPENGLES // Extracting texture data unsupported in OpenGL ES. - nassertr(false, false); - return false; + nassert_raise("OpenGL ES does not support extracting texture data"); + return false; #else // Make sure the GL driver does not align textures, otherwise we get corrupt diff --git a/panda/src/gobj/geom.cxx b/panda/src/gobj/geom.cxx index 03409b4220..8bede7327b 100644 --- a/panda/src/gobj/geom.cxx +++ b/panda/src/gobj/geom.cxx @@ -1504,7 +1504,7 @@ clear_prepared(PreparedGraphicsObjects *prepared_objects) { } else { // If this assertion fails, clear_prepared() was given a prepared_objects // that the geom didn't know about. - nassertv(false); + nassert_raise("unknown PreparedGraphicsObjects"); } } diff --git a/panda/src/gobj/geomCacheManager.cxx b/panda/src/gobj/geomCacheManager.cxx index 4eb06a25b4..4911a981c6 100644 --- a/panda/src/gobj/geomCacheManager.cxx +++ b/panda/src/gobj/geomCacheManager.cxx @@ -47,7 +47,7 @@ GeomCacheManager() : GeomCacheManager:: ~GeomCacheManager() { // Shouldn't be deleting this global object. - nassertv(false); + nassert_raise("attempt to delete GeomCacheManager"); } /** diff --git a/panda/src/gobj/geomPrimitive.cxx b/panda/src/gobj/geomPrimitive.cxx index da3c10dffd..011d66b132 100644 --- a/panda/src/gobj/geomPrimitive.cxx +++ b/panda/src/gobj/geomPrimitive.cxx @@ -221,7 +221,7 @@ add_vertex(int vertex) { ((uint32_t *)ptr)[num_rows] = vertex; break; default: - nassertv(false); + nassert_raise("unsupported index type"); break; } } @@ -1510,7 +1510,7 @@ clear_prepared(PreparedGraphicsObjects *prepared_objects) { } else { // If this assertion fails, clear_prepared() was given a prepared_objects // which the data array didn't know about. - nassertv(false); + nassert_raise("unknown PreparedGraphicsObjects"); } } @@ -2230,7 +2230,7 @@ get_vertex(int i) const { return ((uint32_t *)ptr)[i]; break; default: - nassertr(false, -1); + nassert_raise("unsupported index type"); return -1; } @@ -2296,7 +2296,7 @@ get_referenced_vertices(BitArray &bits) const { } break; default: - nassertv(false); + nassert_raise("unsupported index type"); break; } } else { diff --git a/panda/src/gobj/geomTrifans.cxx b/panda/src/gobj/geomTrifans.cxx index a99481e92e..5a6c518867 100644 --- a/panda/src/gobj/geomTrifans.cxx +++ b/panda/src/gobj/geomTrifans.cxx @@ -139,7 +139,7 @@ CPT(GeomVertexArrayData) GeomTrifans:: rotate_impl() const { // Actually, we can't rotate fans without chaging the winding order. It's // an error to define a flat shade model for a GeomTrifan. - nassertr(false, nullptr); + nassert_raise("GeomTrifans cannot have flat shading model"); return nullptr; } diff --git a/panda/src/gobj/geomVertexArrayData.cxx b/panda/src/gobj/geomVertexArrayData.cxx index 998ca1ad14..f6d04beac5 100644 --- a/panda/src/gobj/geomVertexArrayData.cxx +++ b/panda/src/gobj/geomVertexArrayData.cxx @@ -351,7 +351,7 @@ clear_prepared(PreparedGraphicsObjects *prepared_objects) { } else { // If this assertion fails, clear_prepared() was given a prepared_objects // which the data array didn't know about. - nassertv(false); + nassert_raise("unknown PreparedGraphicsObjects"); } } diff --git a/panda/src/gobj/geomVertexData.cxx b/panda/src/gobj/geomVertexData.cxx index d97197e25c..45522427c2 100644 --- a/panda/src/gobj/geomVertexData.cxx +++ b/panda/src/gobj/geomVertexData.cxx @@ -1121,7 +1121,8 @@ do_set_color(GeomVertexData *vdata, const LColor &color) { const GeomVertexColumn *column; int array_index; if (!format->get_array_info(InternalName::get_color(), array_index, column)) { - nassertv(false); + nassert_raise("no color column"); + return; } size_t stride = format->get_array(array_index)->get_stride(); diff --git a/panda/src/gobj/shader.cxx b/panda/src/gobj/shader.cxx index 4b635b00e7..4a1092eb9c 100644 --- a/panda/src/gobj/shader.cxx +++ b/panda/src/gobj/shader.cxx @@ -3705,7 +3705,7 @@ clear_prepared(PreparedGraphicsObjects *prepared_objects) { } else { // If this assertion fails, clear_prepared() was given a prepared_objects // which the texture didn't know about. - nassertv(false); + nassert_raise("unknown PreparedGraphicsObjects"); } } diff --git a/panda/src/gobj/texture.cxx b/panda/src/gobj/texture.cxx index c0cd14db0a..1ba5b891e3 100644 --- a/panda/src/gobj/texture.cxx +++ b/panda/src/gobj/texture.cxx @@ -4897,8 +4897,8 @@ do_read_ktx(CData *cdata, istream &in, const string &filename, bool header_only) } break; default: - nassertr(false, false); - break; + nassert_raise("unexpected channel count"); + return false; } } @@ -6591,8 +6591,9 @@ do_reconsider_image_properties(CData *cdata, int x_size, int y_size, int num_com default: // Eh? - nassertr(false, false); + nassert_raise("unexpected channel count"); cdata->_format = F_rgb; + return false; } } @@ -7989,7 +7990,8 @@ convert_from_pfm(PTA_uchar &image, size_t page_size, int z, break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + return; } nassertv((unsigned char *)p == &image[idx] + page_size); @@ -8199,7 +8201,8 @@ convert_to_pfm(PfmFile &pfm, int x_size, int y_size, break; default: - nassertr(false, false); + nassert_raise("unexpected channel count"); + return false; } nassertr((unsigned char *)p == &image[idx] + page_size, false); diff --git a/panda/src/gsgbase/graphicsStateGuardianBase.cxx b/panda/src/gsgbase/graphicsStateGuardianBase.cxx index 5ac46dd4af..d8da758ee8 100644 --- a/panda/src/gsgbase/graphicsStateGuardianBase.cxx +++ b/panda/src/gsgbase/graphicsStateGuardianBase.cxx @@ -54,7 +54,7 @@ set_default_gsg(GraphicsStateGuardianBase *default_gsg) { LightMutexHolder holder(gsg_list->_lock); if (find(gsg_list->_gsgs.begin(), gsg_list->_gsgs.end(), default_gsg) == gsg_list->_gsgs.end()) { // The specified GSG doesn't exist or it has already destructed. - nassertv(false); + nassert_raise("GSG not found or already destructed"); return; } diff --git a/panda/src/nativenet/socket_address.I b/panda/src/nativenet/socket_address.I index 87213df01d..3fbc5c82d0 100644 --- a/panda/src/nativenet/socket_address.I +++ b/panda/src/nativenet/socket_address.I @@ -43,7 +43,7 @@ Socket_Address(const struct sockaddr &inaddr) { _addr6 = (const sockaddr_in6 &)inaddr; } else { - nassertv(false); + nassert_raise("unsupported address family"); clear(); } } @@ -106,7 +106,7 @@ operator == (const Socket_Address &in) const { } // Unsupported address family. - nassertr(false, false); + nassert_raise("unsupported address family"); return false; } @@ -224,7 +224,7 @@ operator < (const Socket_Address &in) const { } // Unsupported address family. - nassertr(false, false); + nassert_raise("unsupported address family"); return false; } diff --git a/panda/src/nativenet/socket_address.cxx b/panda/src/nativenet/socket_address.cxx index 3d7882aeed..3b8c19d3dd 100644 --- a/panda/src/nativenet/socket_address.cxx +++ b/panda/src/nativenet/socket_address.cxx @@ -98,7 +98,7 @@ get_ip() const { getnameinfo(&_addr, sizeof(sockaddr_in6), buf, sizeof(buf), nullptr, 0, NI_NUMERICHOST); } else { - nassertr(false, std::string()); + nassert_raise("unsupported address family"); } return std::string(buf); @@ -124,7 +124,7 @@ get_ip_port() const { sprintf(buf + strlen(buf), "]:%hu", get_port()); } else { - nassertr(false, std::string()); + nassert_raise("unsupported address family"); } return std::string(buf); diff --git a/panda/src/net/connection.cxx b/panda/src/net/connection.cxx index 27f5c8bb24..e98f53cc79 100644 --- a/panda/src/net/connection.cxx +++ b/panda/src/net/connection.cxx @@ -478,7 +478,8 @@ check_send_error(bool okflag) { if (!okflag) { static ConfigVariableBool abort_send_error("abort-send-error", false); if (abort_send_error) { - nassertr(false, false); + nassert_raise("send error"); + return false; } // Assume any error means the connection has been reset; tell our manager diff --git a/panda/src/net/datagramTCPHeader.cxx b/panda/src/net/datagramTCPHeader.cxx index 44db216b3d..3cdbcc9fc0 100644 --- a/panda/src/net/datagramTCPHeader.cxx +++ b/panda/src/net/datagramTCPHeader.cxx @@ -46,7 +46,8 @@ DatagramTCPHeader(const NetDatagram &datagram, int header_size) { break; default: - nassertv(false); + nassert_raise("invalid header size"); + return; } nassertv((int)_header.get_length() == header_size); diff --git a/panda/src/parametrics/parametricCurveCollection.cxx b/panda/src/parametrics/parametricCurveCollection.cxx index 96ce9bd503..7d7581732c 100644 --- a/panda/src/parametrics/parametricCurveCollection.cxx +++ b/panda/src/parametrics/parametricCurveCollection.cxx @@ -276,7 +276,7 @@ get_timewarp_curve(int n) const { n--; } } - nassertr(false, nullptr); + nassert_raise("index out of range"); return nullptr; } diff --git a/panda/src/pgraph/clipPlaneAttrib.cxx b/panda/src/pgraph/clipPlaneAttrib.cxx index de3f1183c9..c17a17b272 100644 --- a/panda/src/pgraph/clipPlaneAttrib.cxx +++ b/panda/src/pgraph/clipPlaneAttrib.cxx @@ -72,7 +72,7 @@ make(ClipPlaneAttrib::Operation op, PlaneNode *plane) { return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } @@ -110,7 +110,7 @@ make(ClipPlaneAttrib::Operation op, PlaneNode *plane1, PlaneNode *plane2) { return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } @@ -152,7 +152,7 @@ make(ClipPlaneAttrib::Operation op, PlaneNode *plane1, PlaneNode *plane2, return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } @@ -197,7 +197,7 @@ make(ClipPlaneAttrib::Operation op, PlaneNode *plane1, PlaneNode *plane2, return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } diff --git a/panda/src/pgraph/cullBinManager.cxx b/panda/src/pgraph/cullBinManager.cxx index 5826905c89..cfacd85a03 100644 --- a/panda/src/pgraph/cullBinManager.cxx +++ b/panda/src/pgraph/cullBinManager.cxx @@ -199,7 +199,7 @@ make_new_bin(int bin_index, GraphicsStateGuardianBase *gsg, } // Hmm, unknown (or unregistered) bin type. - nassertr(false, nullptr); + nassert_raise("unknown bin type"); return nullptr; } diff --git a/panda/src/pgraph/lightAttrib.cxx b/panda/src/pgraph/lightAttrib.cxx index 1d6fce8cf7..bdd1553a5e 100644 --- a/panda/src/pgraph/lightAttrib.cxx +++ b/panda/src/pgraph/lightAttrib.cxx @@ -116,7 +116,7 @@ make(LightAttrib::Operation op, Light *light) { return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } @@ -154,7 +154,7 @@ make(LightAttrib::Operation op, Light *light1, Light *light2) { return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } @@ -196,7 +196,7 @@ make(LightAttrib::Operation op, Light *light1, Light *light2, return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } @@ -241,7 +241,7 @@ make(LightAttrib::Operation op, Light *light1, Light *light2, return attrib; } - nassertr(false, make()); + nassert_raise("invalid operation"); return make(); } diff --git a/panda/src/pgraph/nodePath.cxx b/panda/src/pgraph/nodePath.cxx index a21df38d19..0f2639bccc 100644 --- a/panda/src/pgraph/nodePath.cxx +++ b/panda/src/pgraph/nodePath.cxx @@ -718,7 +718,8 @@ get_state(const NodePath &other, Thread *current_thread) const { } else { pgraph_cat.error() << *this << " is not related to " << other << "\n"; - nassertr(false, RenderState::make_empty()); + nassert_raise("unrelated nodes"); + return RenderState::make_empty(); } } @@ -792,7 +793,8 @@ get_transform(const NodePath &other, Thread *current_thread) const { } else { pgraph_cat.error() << *this << " is not related to " << other << "\n"; - nassertr(false, TransformState::make_identity()); + nassert_raise("unrelated nodes"); + return TransformState::make_identity(); } } @@ -877,7 +879,8 @@ get_prev_transform(const NodePath &other, Thread *current_thread) const { } else { pgraph_cat.error() << *this << " is not related to " << other << "\n"; - nassertr(false, TransformState::make_identity()); + nassert_raise("unrelated nodes"); + return TransformState::make_identity(); } } diff --git a/panda/src/pgraph/pandaNode.cxx b/panda/src/pgraph/pandaNode.cxx index 6994e643f8..7d53cbcaf7 100644 --- a/panda/src/pgraph/pandaNode.cxx +++ b/panda/src/pgraph/pandaNode.cxx @@ -2383,7 +2383,8 @@ r_copy_subgraph(PandaNode::InstanceMap &inst_map, Thread *current_thread) const << "Don't know how to copy nodes of type " << get_type() << "\n"; if (no_unsupported_copy) { - nassertr(false, nullptr); + nassert_raise("unsupported copy"); + return nullptr; } } diff --git a/panda/src/pgraph/renderAttribRegistry.cxx b/panda/src/pgraph/renderAttribRegistry.cxx index 939f9753f7..7919d75799 100644 --- a/panda/src/pgraph/renderAttribRegistry.cxx +++ b/panda/src/pgraph/renderAttribRegistry.cxx @@ -78,7 +78,7 @@ register_slot(TypeHandle type_handle, int sort, RenderAttrib *default_attrib) { pgraph_cat->error() << "Too many registered RenderAttribs; not registering " << type_handle << "\n"; - nassertr(false, 0); + nassert_raise("out of RenderAttrib slots"); return 0; } diff --git a/panda/src/pgraph/sceneGraphReducer.cxx b/panda/src/pgraph/sceneGraphReducer.cxx index 03179d38dd..06a80e5a4d 100644 --- a/panda/src/pgraph/sceneGraphReducer.cxx +++ b/panda/src/pgraph/sceneGraphReducer.cxx @@ -333,7 +333,8 @@ r_apply_attribs(PandaNode *node, const AccumulatedAttribs &attribs, << child_node->get_type() << "\n"; if (no_unsupported_copy) { - nassertv(false); + nassert_raise("unsupported copy"); + return; } resist_copy = true; diff --git a/panda/src/pgraphnodes/ambientLight.cxx b/panda/src/pgraphnodes/ambientLight.cxx index deb6eed6cc..f072f1ba6f 100644 --- a/panda/src/pgraphnodes/ambientLight.cxx +++ b/panda/src/pgraphnodes/ambientLight.cxx @@ -85,7 +85,7 @@ void AmbientLight:: bind(GraphicsStateGuardianBase *, const NodePath &, int) { // AmbientLights aren't bound to light id's; this function should never be // called. - nassertv(false); + nassert_raise("cannot bind AmbientLight"); } /** diff --git a/panda/src/pipeline/pythonThread.cxx b/panda/src/pipeline/pythonThread.cxx index 53a811401e..13b31e0539 100644 --- a/panda/src/pipeline/pythonThread.cxx +++ b/panda/src/pipeline/pythonThread.cxx @@ -161,7 +161,8 @@ call_python_func(PyObject *function, PyObject *args) { #ifndef HAVE_THREADS // Shouldn't be possible to come here without having some kind of // threading support enabled. - nassertr(false, nullptr); + nassert_raise("threading support disabled"); + return nullptr; #else #ifdef SIMPLE_THREADS diff --git a/panda/src/pnmimage/pfmFile.cxx b/panda/src/pnmimage/pfmFile.cxx index 57b9671d1a..23ba55a44d 100644 --- a/panda/src/pnmimage/pfmFile.cxx +++ b/panda/src/pnmimage/pfmFile.cxx @@ -346,7 +346,8 @@ load(const PNMImage &pnmimage) { break; default: - nassertr(false, false); + nassert_raise("unexpected channel count"); + return false; } return true; } @@ -410,7 +411,8 @@ store(PNMImage &pnmimage) const { break; default: - nassertr(false, false); + nassert_raise("unexpected channel count"); + return false; } return true; } @@ -877,7 +879,8 @@ set_no_data_nan(int num_channels) { _has_point = has_point_nan_4; break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + break; } } else { clear_no_data_value(); @@ -909,7 +912,8 @@ set_no_data_value(const LPoint4f &no_data_value) { _has_point = has_point_4; break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + break; } } @@ -938,7 +942,8 @@ set_no_data_threshold(const LPoint4f &no_data_value) { _has_point = has_point_threshold_4; break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + break; } } @@ -1121,7 +1126,8 @@ quick_filter_from(const PfmFile &from) { break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + return; } new_data.push_back(0.0); @@ -1826,7 +1832,8 @@ compute_planar_bounds(const LPoint2f ¢er, PN_float32 point_dist, PN_float32 break; default: - nassertr(false, nullptr); + nassert_raise("invalid coordinate system"); + return nullptr; } // Rotate the bounding volume back into the original space of the screen. @@ -1879,7 +1886,8 @@ compute_sample_point(LPoint3f &result, break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + break; } } diff --git a/panda/src/pnmimage/pnmImage.cxx b/panda/src/pnmimage/pnmImage.cxx index 9c6c3a68d9..a26528f99a 100644 --- a/panda/src/pnmimage/pnmImage.cxx +++ b/panda/src/pnmimage/pnmImage.cxx @@ -596,8 +596,8 @@ set_color_space(ColorSpace color_space) { break; default: - nassertv(false); - break; + nassert_raise("invalid color space"); + return; } // Initialize the new encoding settings. @@ -852,7 +852,7 @@ get_channel_val(int x, int y, int channel) const { pnmimage_cat.error() << "Invalid request for channel " << channel << " in " << get_num_channels() << "-channel image.\n"; - nassertr(false, 0); + nassert_raise("unexpected channel count"); return 0; } } @@ -888,7 +888,8 @@ set_channel_val(int x, int y, int channel, xelval value) { break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + break; } } @@ -918,7 +919,7 @@ get_channel(int x, int y, int channel) const { pnmimage_cat.error() << "Invalid request for channel " << channel << " in " << get_num_channels() << "-channel image.\n"; - nassertr(false, 0); + nassert_raise("unexpected channel count"); return 0; } } @@ -954,7 +955,8 @@ set_channel(int x, int y, int channel, float value) { break; default: - nassertv(false); + nassert_raise("unexpected channel count"); + break; } } @@ -2126,7 +2128,7 @@ setup_encoding() { break; default: - nassertv(false); + nassert_raise("invalid color space"); break; } } else { @@ -2153,7 +2155,7 @@ setup_encoding() { break; default: - nassertv(false); + nassert_raise("invalid color space"); break; } } diff --git a/panda/src/pnmimagetypes/pnmFileTypePfm.cxx b/panda/src/pnmimagetypes/pnmFileTypePfm.cxx index 89324f3c61..1303cc9c6c 100644 --- a/panda/src/pnmimagetypes/pnmFileTypePfm.cxx +++ b/panda/src/pnmimagetypes/pnmFileTypePfm.cxx @@ -283,7 +283,8 @@ write_pfm(const PfmFile &pfm) { break; default: - nassertr(false, false); + nassert_raise("unexpected channel count"); + return false; } (*_file) << pfm.get_x_size() << " " << pfm.get_y_size() << "\n"; diff --git a/panda/src/pnmimagetypes/pnmFileTypeSGIWriter.cxx b/panda/src/pnmimagetypes/pnmFileTypeSGIWriter.cxx index f5b143e867..4bec4292d5 100644 --- a/panda/src/pnmimagetypes/pnmFileTypeSGIWriter.cxx +++ b/panda/src/pnmimagetypes/pnmFileTypeSGIWriter.cxx @@ -135,7 +135,8 @@ write_header() { break; default: - nassertr(false, false); + nassert_raise("unexpected channel count"); + return false; } // For some reason, we have problems with SGI image files whose pixmax value diff --git a/panda/src/putil/sparseArray.I b/panda/src/putil/sparseArray.I index 682d5f93be..ccfd90b5ab 100644 --- a/panda/src/putil/sparseArray.I +++ b/panda/src/putil/sparseArray.I @@ -93,7 +93,7 @@ has_max_num_bits() { */ INLINE int SparseArray:: get_max_num_bits() { - nassertr(false, 0); + nassert_raise("SparseArray has no maximum bit count"); return 0; } diff --git a/panda/src/text/textAssembler.cxx b/panda/src/text/textAssembler.cxx index 3925715c6d..32cf41eb89 100644 --- a/panda/src/text/textAssembler.cxx +++ b/panda/src/text/textAssembler.cxx @@ -2536,7 +2536,7 @@ get_primitive(TypeHandle prim_type) { return _points; } - nassertr(false, nullptr); + nassert_raise("unexpected primitive type"); return nullptr; } diff --git a/panda/src/vision/webcamVideoCursorV4L.cxx b/panda/src/vision/webcamVideoCursorV4L.cxx index 8bb77658f6..197bda3228 100644 --- a/panda/src/vision/webcamVideoCursorV4L.cxx +++ b/panda/src/vision/webcamVideoCursorV4L.cxx @@ -487,7 +487,8 @@ fetch_buffer() { block[i + 2] = ex; } #else - nassertr(false /* Not compiled with JPEG support*/, nullptr); + nassert_raise("JPEG support not compiled-in"); + return nullptr; #endif break; } diff --git a/pandatool/src/assimp/pandaIOSystem.cxx b/pandatool/src/assimp/pandaIOSystem.cxx index 87dea240d2..241790f646 100644 --- a/pandatool/src/assimp/pandaIOSystem.cxx +++ b/pandatool/src/assimp/pandaIOSystem.cxx @@ -79,7 +79,7 @@ Open(const char *file, const char *mode) { return new PandaIOStream(*stream); } else { - nassertr(false, nullptr); // Not implemented on purpose. + nassert_raise("write mode not implemented"); return nullptr; } } diff --git a/pandatool/src/eggcharbase/eggBackPointer.cxx b/pandatool/src/eggcharbase/eggBackPointer.cxx index b1e269624a..cc6110d075 100644 --- a/pandatool/src/eggcharbase/eggBackPointer.cxx +++ b/pandatool/src/eggcharbase/eggBackPointer.cxx @@ -40,7 +40,7 @@ get_frame_rate() const { void EggBackPointer:: extend_to(int num_frames) { // Whoops, can't extend this kind of table! - nassertv(false); + nassert_raise("can't extend this kind of table"); } /**