From 3b4d4b0804ae7539bd271cbc7812311ee268c12c Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 30 Apr 2018 19:13:54 -0400 Subject: [PATCH 1/8] glgsg: fix broken upload of downscaled texture without mipmaps Fixes #306 --- panda/src/glstuff/glGraphicsStateGuardian_src.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 97917acee0..f5a42e2e45 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -12647,7 +12647,7 @@ upload_texture_image(CLP(TextureContext) *gtc, bool needs_reload, int depth = tex->get_expected_mipmap_z_size(mipmap_bias); // Determine the number of images to upload. - int num_levels = 1; + int num_levels = mipmap_bias + 1; if (uses_mipmaps) { num_levels = tex->get_expected_num_mipmap_levels(); } From 9db74bca1d855230edbc5c3b6616e56c5de44127 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 1 May 2018 15:06:27 +0200 Subject: [PATCH 2/8] task: use consistent ordering for tasks with same sort value We don't guarantee a specific order in this case, especially because they can be run in either order if there is more than one thread, but it is still useful to have a defined order for single-threaded task chains. To that end, tasks are now run in the order in which they were added to taskMgr.add (in absence of any other ordering constraints). Fixes #309 --- panda/src/event/asyncTask.h | 1 + panda/src/event/asyncTaskChain.cxx | 6 +++++- panda/src/event/asyncTaskChain.h | 9 ++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/panda/src/event/asyncTask.h b/panda/src/event/asyncTask.h index d514b2f5a3..9712daa0f5 100644 --- a/panda/src/event/asyncTask.h +++ b/panda/src/event/asyncTask.h @@ -119,6 +119,7 @@ protected: double _wake_time; int _sort; int _priority; + unsigned int _implicit_sort; State _state; Thread *_servicing_thread; diff --git a/panda/src/event/asyncTaskChain.cxx b/panda/src/event/asyncTaskChain.cxx index c7d732eca9..bb82e00f92 100644 --- a/panda/src/event/asyncTaskChain.cxx +++ b/panda/src/event/asyncTaskChain.cxx @@ -51,7 +51,8 @@ AsyncTaskChain(AsyncTaskManager *manager, const string &name) : _needs_cleanup(false), _current_frame(0), _time_in_frame(0.0), - _block_till_next_frame(false) + _block_till_next_frame(false), + _next_implicit_sort(0) { } @@ -418,6 +419,9 @@ do_add(AsyncTask *task) { task->_start_time = now; task->_start_frame = _manager->_clock->get_frame_count(); + // Remember the order in which tasks were added to the chain. + task->_implicit_sort = _next_implicit_sort++; + _manager->add_task_by_name(task); if (task->has_delay()) { diff --git a/panda/src/event/asyncTaskChain.h b/panda/src/event/asyncTaskChain.h index 512a7b9fa0..5f81f6c3dd 100644 --- a/panda/src/event/asyncTaskChain.h +++ b/panda/src/event/asyncTaskChain.h @@ -146,7 +146,12 @@ protected: if (a->get_priority() != b->get_priority()) { return a->get_priority() < b->get_priority(); } - return a->get_start_time() > b->get_start_time(); + if (a->get_start_time() != b->get_start_time()) { + return a->get_start_time() > b->get_start_time(); + } + // Failing any other ordering criteria, we sort the tasks based on the + // order in which they were added to the task chain. + return a->_implicit_sort > b->_implicit_sort; } }; @@ -186,6 +191,8 @@ protected: double _time_in_frame; bool _block_till_next_frame; + unsigned int _next_implicit_sort; + static PStatCollector _task_pcollector; static PStatCollector _wait_pcollector; From b45726001e817278783500b8b358513979d0b031 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 5 May 2018 20:53:04 +0200 Subject: [PATCH 3/8] task: fix double free when failing to retrieve coroutine exception --- panda/src/event/pythonTask.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/panda/src/event/pythonTask.cxx b/panda/src/event/pythonTask.cxx index 996587885e..6a818ad19c 100644 --- a/panda/src/event/pythonTask.cxx +++ b/panda/src/event/pythonTask.cxx @@ -94,6 +94,9 @@ PythonTask:: PyErr_Restore(_exception, _exc_value, _exc_traceback); PyErr_Print(); PyErr_Restore(nullptr, nullptr, nullptr); + _exception = nullptr; + _exc_value = nullptr; + _exc_traceback = nullptr; } #endif From 11e21af52cead7c7f1fb5cfa613ea28dee76ea6a Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 5 May 2018 22:01:10 +0200 Subject: [PATCH 4/8] showbase: fix iris/fade transitions for extreme aspect ratios Fixes #311 --- direct/src/showbase/Transitions.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/direct/src/showbase/Transitions.py b/direct/src/showbase/Transitions.py index e26d0e93ec..aacb7afcc5 100644 --- a/direct/src/showbase/Transitions.py +++ b/direct/src/showbase/Transitions.py @@ -94,7 +94,9 @@ class Transitions: """ #self.noTransitions() masad: this creates a one frame pop, is it necessary? self.loadFade() - transitionIval = Sequence(Func(self.fade.reparentTo, aspect2d, DGG.FADE_SORT_INDEX), + + parent = aspect2d if self.fadeModel else render2d + transitionIval = Sequence(Func(self.fade.reparentTo, parent, DGG.FADE_SORT_INDEX), Func(self.fade.showThrough), # in case aspect2d is hidden for some reason self.lerpFunc(self.fade, t, self.alphaOff, @@ -115,7 +117,8 @@ class Transitions: self.noTransitions() self.loadFade() - transitionIval = Sequence(Func(self.fade.reparentTo,aspect2d,DGG.FADE_SORT_INDEX), + parent = aspect2d if self.fadeModel else render2d + transitionIval = Sequence(Func(self.fade.reparentTo, parent, DGG.FADE_SORT_INDEX), Func(self.fade.showThrough), # in case aspect2d is hidden for some reason self.lerpFunc(self.fade, t, self.alphaOn, @@ -167,7 +170,9 @@ class Transitions: # Fade out immediately with no lerp self.noTransitions() self.loadFade() - self.fade.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) + + parent = aspect2d if self.fadeModel else render2d + self.fade.reparentTo(parent, DGG.FADE_SORT_INDEX) self.fade.setColor(self.alphaOn) elif ConfigVariableBool('no-loading-screen', False): if finishIval: @@ -191,7 +196,9 @@ class Transitions: #print "transitiosn: fadeScreen" self.noTransitions() self.loadFade() - self.fade.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) + + parent = aspect2d if self.fadeModel else render2d + self.fade.reparentTo(parent, DGG.FADE_SORT_INDEX) self.fade.setColor(self.alphaOn[0], self.alphaOn[1], self.alphaOn[2], @@ -206,7 +213,9 @@ class Transitions: #print "transitiosn: fadeScreenColor" self.noTransitions() self.loadFade() - self.fade.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) + + parent = aspect2d if self.fadeModel else render2d + self.fade.reparentTo(parent, DGG.FADE_SORT_INDEX) self.fade.setColor(color) def noFade(self): @@ -250,8 +259,9 @@ class Transitions: else: self.iris.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) + scale = 0.18 * max(base.a2dRight, base.a2dTop) self.transitionIval = Sequence(LerpScaleInterval(self.iris, t, - scale = 0.18, + scale = scale, startScale = 0.01), Func(self.iris.detachNode), name = self.irisTaskName, @@ -277,9 +287,10 @@ class Transitions: else: self.iris.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) + scale = 0.18 * max(base.a2dRight, base.a2dTop) self.transitionIval = Sequence(LerpScaleInterval(self.iris, t, scale = 0.01, - startScale = 0.18), + startScale = scale), Func(self.iris.detachNode), # Use the fade to cover up the hole that the iris would leave Func(self.fadeOut, 0), From cf58de4d04da6f9fd176bd8c127546699eeb6444 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 5 May 2018 23:02:11 +0200 Subject: [PATCH 5/8] showbase: make base.movie() awaitable (by returning a future) --- direct/src/showbase/ShowBase.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/direct/src/showbase/ShowBase.py b/direct/src/showbase/ShowBase.py index 367ae57d1a..13b9839255 100644 --- a/direct/src/showbase/ShowBase.py +++ b/direct/src/showbase/ShowBase.py @@ -2672,15 +2672,18 @@ class ShowBase(DirectObject.DirectObject): output file name (e.g. if sd = 4, movie_0001.png) - source is the Window, Buffer, DisplayRegion, or Texture from which to save the resulting images. The default is the main window. + + The task is returned, so that it can be awaited. """ globalClock.setMode(ClockObject.MNonRealTime) globalClock.setDt(1.0/float(fps)) - t = taskMgr.add(self._movieTask, namePrefix + '_task') + t = self.taskMgr.add(self._movieTask, namePrefix + '_task') t.frameIndex = 0 # Frame 0 is not captured. t.numFrames = int(duration * fps) t.source = source t.outputString = namePrefix + '_%0' + repr(sd) + 'd.' + format t.setUponDeath(lambda state: globalClock.setMode(ClockObject.MNormal)) + return t def _movieTask(self, state): if state.frameIndex != 0: From 71eee6df3fc9ce0099ea985e4d843790eee59216 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 5 May 2018 23:05:50 +0200 Subject: [PATCH 6/8] showbase: make iris/fade/letterbox transitions awaitable This allows using a coroutine to build up a more complex sequence including transitions (eg. scripted cutscene), as well as provide a standard way to register callbacks upon completion of the transition. --- direct/src/showbase/Transitions.py | 58 +++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/direct/src/showbase/Transitions.py b/direct/src/showbase/Transitions.py index aacb7afcc5..5c449c6997 100644 --- a/direct/src/showbase/Transitions.py +++ b/direct/src/showbase/Transitions.py @@ -23,7 +23,9 @@ class Transitions: scale=3.0, pos=Vec3(0, 0, 0)): self.transitionIval = None + self.__transitionFuture = None self.letterboxIval = None + self.__letterboxFuture = None self.iris = None self.fade = None self.letterbox = None @@ -151,11 +153,17 @@ class Transitions: self.noTransitions() self.loadFade() self.fade.detachNode() + fut = AsyncFuture() + fut.setResult(None) + return fut else: # Create a sequence that lerps the color out, then # parents the fade to hidden self.transitionIval = self.getFadeInIval(t, finishIval) + self.transitionIval.append(Func(self.__finishTransition)) + self.__transitionFuture = AsyncFuture() self.transitionIval.start() + return self.__transitionFuture def fadeOut(self, t=0.5, finishIval=None): """ @@ -181,8 +189,16 @@ class Transitions: else: # Create a sequence that lerps the color out, then # parents the fade to hidden - self.transitionIval = self.getFadeOutIval(t,finishIval) + self.transitionIval = self.getFadeOutIval(t, finishIval) + self.transitionIval.append(Func(self.__finishTransition)) + self.__transitionFuture = AsyncFuture() self.transitionIval.start() + return self.__transitionFuture + + # Immediately done, so return a dummy future. + fut = AsyncFuture() + fut.setResult(None) + return fut def fadeOutActive(self): return self.fade and self.fade.getColor()[3] > 0 @@ -226,6 +242,9 @@ class Transitions: if self.transitionIval: self.transitionIval.pause() self.transitionIval = None + if self.__transitionFuture: + self.__transitionFuture.cancel() + self.__transitionFuture = None if self.fade: # Make sure to reset the color, since fadeOutActive() is looking at it self.fade.setColor(self.alphaOff) @@ -256,6 +275,9 @@ class Transitions: self.loadIris() if (t == 0): self.iris.detachNode() + fut = AsyncFuture() + fut.setResult(None) + return fut else: self.iris.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) @@ -264,11 +286,14 @@ class Transitions: scale = scale, startScale = 0.01), Func(self.iris.detachNode), + Func(self.__finishTransition), name = self.irisTaskName, ) + self.__transitionFuture = AsyncFuture() if finishIval: self.transitionIval.append(finishIval) self.transitionIval.start() + return self.__transitionFuture def irisOut(self, t=0.5, finishIval=None): """ @@ -284,6 +309,9 @@ class Transitions: if (t == 0): self.iris.detachNode() self.fadeOut(0) + fut = AsyncFuture() + fut.setResult(None) + return fut else: self.iris.reparentTo(aspect2d, DGG.FADE_SORT_INDEX) @@ -294,11 +322,14 @@ class Transitions: Func(self.iris.detachNode), # Use the fade to cover up the hole that the iris would leave Func(self.fadeOut, 0), + Func(self.__finishTransition), name = self.irisTaskName, ) + self.__transitionFuture = AsyncFuture() if finishIval: self.transitionIval.append(finishIval) self.transitionIval.start() + return self.__transitionFuture def noIris(self): """ @@ -322,6 +353,11 @@ class Transitions: # Letterbox is not really a transition, it is a screen overlay # self.noLetterbox() + def __finishTransition(self): + if self.__transitionFuture: + self.__transitionFuture.setResult(None) + self.__transitionFuture = None + ################################################## # Letterbox ################################################## @@ -394,9 +430,17 @@ class Transitions: if self.letterboxIval: self.letterboxIval.pause() self.letterboxIval = None + if self.__letterboxFuture: + self.__letterboxFuture.cancel() + self.__letterboxFuture = None if self.letterbox: self.letterbox.stash() + def __finishLetterbox(self): + if self.__letterboxFuture: + self.__letterboxFuture.setResult(None) + self.__letterboxFuture = None + def letterboxOn(self, t=0.25, finishIval=None): """ Move black bars in over t seconds. @@ -407,7 +451,11 @@ class Transitions: if (t == 0): self.letterboxBottom.setPos(0, 0, -1) self.letterboxTop.setPos(0, 0, 0.8) + fut = AsyncFuture() + fut.setResult(None) + return fut else: + self.__letterboxFuture = AsyncFuture() self.letterboxIval = Sequence(Parallel( LerpPosInterval(self.letterboxBottom, t, @@ -420,11 +468,13 @@ class Transitions: # startPos = Vec3(0, 0, 1), ), ), + Func(self.__finishLetterbox), name = self.letterboxTaskName, ) if finishIval: self.letterboxIval.append(finishIval) self.letterboxIval.start() + return self.__letterboxFuture def letterboxOff(self, t=0.25, finishIval=None): """ @@ -435,7 +485,11 @@ class Transitions: self.letterbox.unstash() if (t == 0): self.letterbox.stash() + fut = AsyncFuture() + fut.setResult(None) + return fut else: + self.__letterboxFuture = AsyncFuture() self.letterboxIval = Sequence(Parallel( LerpPosInterval(self.letterboxBottom, t, @@ -449,9 +503,11 @@ class Transitions: ), ), Func(self.letterbox.stash), + Func(self.__finishLetterbox), Func(messenger.send,'letterboxOff'), name = self.letterboxTaskName, ) if finishIval: self.letterboxIval.append(finishIval) self.letterboxIval.start() + return self.__letterboxFuture From 8a98bf42a3bf5ecd473fe5cfd77059b17a340747 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 5 May 2018 23:09:39 +0200 Subject: [PATCH 7/8] general: enable use of tie and tuple on macOS (from TR1, for now) --- dtool/src/dtoolbase/dtoolbase_cc.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dtool/src/dtoolbase/dtoolbase_cc.h b/dtool/src/dtoolbase/dtoolbase_cc.h index 6777ce5611..81968085bc 100644 --- a/dtool/src/dtoolbase/dtoolbase_cc.h +++ b/dtool/src/dtoolbase/dtoolbase_cc.h @@ -122,6 +122,11 @@ typedef ios::seekdir ios_seekdir; // Apple has an outdated libstdc++. Not all is lost, though, as we can fill // in some important missing functions. #if defined(__GLIBCXX__) && __GLIBCXX__ <= 20070719 +#include + +using std::tr1::tuple; +using std::tr1::tie; + typedef decltype(nullptr) nullptr_t; template struct remove_reference {typedef T type;}; From 549301d0f01976eb321055dc2a168368089050ef Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 5 May 2018 23:10:41 +0200 Subject: [PATCH 8/8] putil: keep reference to objects queued for writing This prevents a crash when a reference counted object is destroyed right after a write_pointer call. Fixes #310 --- panda/src/putil/bamWriter.cxx | 30 ++++++++++++++++++++++++++++-- panda/src/putil/bamWriter.h | 3 ++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/panda/src/putil/bamWriter.cxx b/panda/src/putil/bamWriter.cxx index 679600b664..0fcfd2fe1f 100644 --- a/panda/src/putil/bamWriter.cxx +++ b/panda/src/putil/bamWriter.cxx @@ -94,6 +94,10 @@ BamWriter:: for (si = _state_map.begin(); si != _state_map.end(); ++si) { TypedWritable *object = (TypedWritable *)(*si).first; object->remove_bam_writer(this); + + if ((*si).second._refcount != nullptr) { + unref_delete((*si).second._refcount); + } } } @@ -529,6 +533,9 @@ object_destructs(TypedWritable *object) { // we're in trouble when we do write it out. nassertv(!(*si).second._written_seq.is_initial()); + // This cannot be called if we are still holding a reference to it. + nassertv((*si).second._refcount == nullptr); + int object_id = (*si).second._object_id; _freed_object_ids.push_back(object_id); @@ -606,8 +613,10 @@ enqueue_object(const TypedWritable *object) { // No, it hasn't, so assign it the next number in sequence arbitrarily. object_id = _next_object_id; - bool inserted = - _state_map.insert(StateMap::value_type(object, StoreState(_next_object_id))).second; + StateMap::iterator si; + bool inserted; + tie(si, inserted) = + _state_map.insert(StateMap::value_type(object, StoreState(_next_object_id))); nassertr(inserted, false); // Store ourselves on the TypedWritable so that we get notified when it @@ -615,6 +624,14 @@ enqueue_object(const TypedWritable *object) { (const_cast(object))->add_bam_writer(this); _next_object_id++; + // Increase the reference count if this inherits from ReferenceCount, + // until we get a chance to write this object for the first time. + const ReferenceCount *rc = ((TypedWritable *)object)->as_reference_count(); + if (rc != nullptr) { + rc->ref(); + (*si).second._refcount = rc; + } + } else { // Yes, it has; get the object ID. object_id = (*si).second._object_id; @@ -703,6 +720,15 @@ flush_queue() { (*si).second._written_seq = _writing_seq; (*si).second._modified = object->get_bam_modified(); + // Now release any reference we hold to it, so that it may destruct. + const ReferenceCount *rc = (*si).second._refcount; + if (rc != nullptr) { + // We need to assign this pointer to null before deleting the object, + // since that may end up calling object_destructs. + (*si).second._refcount = nullptr; + unref_delete(rc); + } + } else { // On subsequent times when we write a particular object, we write // simply TypeHandle::none(), followed by the object ID. The occurrence diff --git a/panda/src/putil/bamWriter.h b/panda/src/putil/bamWriter.h index c2b04b3757..bcd4a76ce6 100644 --- a/panda/src/putil/bamWriter.h +++ b/panda/src/putil/bamWriter.h @@ -140,8 +140,9 @@ private: int _object_id; UpdateSeq _written_seq; UpdateSeq _modified; + const ReferenceCount *_refcount; - StoreState(int object_id) : _object_id(object_id) {} + StoreState(int object_id) : _object_id(object_id), _refcount(nullptr) {} }; typedef phash_map StateMap; StateMap _state_map;