From 8fbc824aab43ddd721aed8d15afae491860af7ae Mon Sep 17 00:00:00 2001 From: David Rose Date: Thu, 16 Feb 2006 23:45:05 +0000 Subject: [PATCH] make PGui more thread-safe --- panda/src/gobj/geom.cxx | 7 +- panda/src/gobj/geomVertexArrayData.cxx | 7 +- panda/src/gobj/geomVertexData.cxx | 17 +- panda/src/pgui/pgTop.I | 13 -- panda/src/pgui/pgTop.cxx | 24 ++- panda/src/pgui/pgTop.h | 2 +- panda/src/text/textGlyph.I | 4 +- panda/src/tform/mouseWatcher.cxx | 253 +++++++++++++++++-------- panda/src/tform/mouseWatcher.h | 26 +-- panda/src/tform/mouseWatcherGroup.cxx | 41 +++- panda/src/tform/mouseWatcherGroup.h | 6 + 11 files changed, 279 insertions(+), 121 deletions(-) diff --git a/panda/src/gobj/geom.cxx b/panda/src/gobj/geom.cxx index fa5e6379a4..60bf9e10f7 100644 --- a/panda/src/gobj/geom.cxx +++ b/panda/src/gobj/geom.cxx @@ -37,7 +37,12 @@ TypeHandle Geom::_type_handle; //////////////////////////////////////////////////////////////////// Geom:: Geom(const GeomVertexData *data) { - set_vertex_data(data); + // Let's ensure the vertex data gets set on all stages at once. + OPEN_ITERATE_ALL_STAGES(_cycler) { + CDStageWriter cdata(_cycler, pipeline_stage); + cdata->_data = (GeomVertexData *)data; + } + CLOSE_ITERATE_ALL_STAGES(_cycler); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/gobj/geomVertexArrayData.cxx b/panda/src/gobj/geomVertexArrayData.cxx index 8e8ed87c85..947a2a795f 100644 --- a/panda/src/gobj/geomVertexArrayData.cxx +++ b/panda/src/gobj/geomVertexArrayData.cxx @@ -47,7 +47,12 @@ GeomVertexArrayData(const GeomVertexArrayFormat *array_format, GeomVertexArrayData::UsageHint usage_hint) : _array_format(array_format) { - set_usage_hint(usage_hint); + OPEN_ITERATE_ALL_STAGES(_cycler) { + CDStageWriter cdata(_cycler, pipeline_stage); + cdata->_usage_hint = usage_hint; + } + CLOSE_ITERATE_ALL_STAGES(_cycler); + _endian_reversed = false; nassertv(_array_format->is_registered()); } diff --git a/panda/src/gobj/geomVertexData.cxx b/panda/src/gobj/geomVertexData.cxx index a5902d3b41..f3de77d08f 100644 --- a/panda/src/gobj/geomVertexData.cxx +++ b/panda/src/gobj/geomVertexData.cxx @@ -67,14 +67,17 @@ GeomVertexData(const string &name, set_usage_hint(usage_hint); // Create some empty arrays as required by the format. - CDWriter cdata(_cycler, true); - - int num_arrays = _format->get_num_arrays(); - for (int i = 0; i < num_arrays; i++) { - PT(GeomVertexArrayData) array = new GeomVertexArrayData - (_format->get_array(i), usage_hint); - cdata->_arrays.push_back(array); + // Let's ensure the vertex data gets set on all stages at once. + OPEN_ITERATE_ALL_STAGES(_cycler) { + CDStageWriter cdata(_cycler, pipeline_stage); + int num_arrays = _format->get_num_arrays(); + for (int i = 0; i < num_arrays; i++) { + PT(GeomVertexArrayData) array = new GeomVertexArrayData + (_format->get_array(i), usage_hint); + cdata->_arrays.push_back(array); + } } + CLOSE_ITERATE_ALL_STAGES(_cycler); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/pgui/pgTop.I b/panda/src/pgui/pgTop.I index b9cea8036a..28dd675701 100644 --- a/panda/src/pgui/pgTop.I +++ b/panda/src/pgui/pgTop.I @@ -102,16 +102,3 @@ add_region(MouseWatcherRegion *region) { nassertv(_watcher_group != (PGMouseWatcherGroup *)NULL); _watcher_group->add_region(region); } - -//////////////////////////////////////////////////////////////////// -// Function: PGTop::clear_regions -// Access: Public -// Description: Removes all the regions from the group. -//////////////////////////////////////////////////////////////////// -INLINE void PGTop:: -clear_regions() { - if (_watcher_group == (PGMouseWatcherGroup *)NULL) { - return; - } - _watcher_group->clear_regions(); -} diff --git a/panda/src/pgui/pgTop.cxx b/panda/src/pgui/pgTop.cxx index d4420c0444..a269147cac 100644 --- a/panda/src/pgui/pgTop.cxx +++ b/panda/src/pgui/pgTop.cxx @@ -34,7 +34,6 @@ PGTop:: PGTop(const string &name) : PandaNode(name) { - _watcher_group = (PGMouseWatcherGroup *)NULL; _start_sort = 0; // A PGTop node normally has an infinite bounding volume. Screw @@ -107,9 +106,14 @@ has_cull_callback() const { //////////////////////////////////////////////////////////////////// bool PGTop:: cull_callback(CullTraverser *trav, CullTraverserData &data) { - // Empty our set of regions in preparation for re-adding whichever - // ones we encounter in the traversal that are current. - clear_regions(); + // We create a new MouseWatcherGroup for the purposes of collecting + // a new set of regions visible onscreen. + PT(PGMouseWatcherGroup) old_watcher_group; + if (_watcher_group != (PGMouseWatcherGroup *)NULL) { + _watcher_group->clear_top(this); + old_watcher_group = _watcher_group; + _watcher_group = new PGMouseWatcherGroup(this); + } // Now subsitute for the normal CullTraverser a special one of our // own choosing. This just carries around a pointer back to the @@ -119,6 +123,16 @@ cull_callback(CullTraverser *trav, CullTraverserData &data) { pg_trav._sort_index = _start_sort; pg_trav.traverse_below(data); + // Now tell the watcher about the new set of regions. Strictly + // speaking, we shouldn't do this until the frame that we're about + // to render has been presented; otherwise, we may make regions + // active before they are actually visible. But no one has + // complained about this so far. + if (_watcher_group != (PGMouseWatcherGroup *)NULL) { + nassertr(_watcher != (MouseWatcher *)NULL, false); + _watcher->replace_group(old_watcher_group, _watcher_group); + } + // We've taken care of the traversal, thank you. return false; } @@ -143,8 +157,6 @@ set_mouse_watcher(MouseWatcher *watcher) { _watcher_group = (PGMouseWatcherGroup *)NULL; if (_watcher != (MouseWatcher *)NULL) { - // We create a new PGMouseWatcherGroup, but we don't own the - // reference count; the watcher will own this for us. _watcher_group = new PGMouseWatcherGroup(this); _watcher->add_group(_watcher_group); } diff --git a/panda/src/pgui/pgTop.h b/panda/src/pgui/pgTop.h index f7b0d695fc..6ceb0da116 100644 --- a/panda/src/pgui/pgTop.h +++ b/panda/src/pgui/pgTop.h @@ -72,7 +72,7 @@ public: private: PT(MouseWatcher) _watcher; - PGMouseWatcherGroup *_watcher_group; + PT(PGMouseWatcherGroup) _watcher_group; int _start_sort; public: diff --git a/panda/src/text/textGlyph.I b/panda/src/text/textGlyph.I index d3be0f87aa..7a8ef80b4d 100644 --- a/panda/src/text/textGlyph.I +++ b/panda/src/text/textGlyph.I @@ -103,7 +103,9 @@ get_geom(Geom::UsageHint usage_hint) const { // this behavior to properly count references to this glyph. PT(Geom) new_geom = _geom->make_copy(); new_geom->set_usage_hint(usage_hint); - if (new_geom->get_vertex_data()->get_usage_hint() != usage_hint) { + const GeomVertexData *vdata = new_geom->get_vertex_data(); + nassertr(vdata != NULL, new_geom); + if (vdata->get_usage_hint() != usage_hint) { new_geom->modify_vertex_data()->set_usage_hint(usage_hint); } return new_geom; diff --git a/panda/src/tform/mouseWatcher.cxx b/panda/src/tform/mouseWatcher.cxx index af466b86dc..e6d2ee6b83 100644 --- a/panda/src/tform/mouseWatcher.cxx +++ b/panda/src/tform/mouseWatcher.cxx @@ -30,6 +30,7 @@ #include "displayRegion.h" #include "dcast.h" #include "indent.h" +#include "mutexHolder.h" #include @@ -96,6 +97,8 @@ MouseWatcher:: //////////////////////////////////////////////////////////////////// bool MouseWatcher:: remove_region(MouseWatcherRegion *region) { + MutexHolder holder(_lock); + remove_region_from(_current_regions, region); if (region == _preferred_region) { _preferred_region = (MouseWatcherRegion *)NULL; @@ -118,7 +121,9 @@ remove_region(MouseWatcherRegion *region) { //////////////////////////////////////////////////////////////////// MouseWatcherRegion *MouseWatcher:: get_over_region(const LPoint2f &pos) const { - VRegions regions; + MutexHolder holder(_lock); + + Regions regions; get_over_regions(regions, pos); return get_preferred_region(regions); } @@ -141,7 +146,7 @@ get_over_region(const LPoint2f &pos) const { //////////////////////////////////////////////////////////////////// bool MouseWatcher:: add_group(MouseWatcherGroup *group) { - // return _groups.insert(group).second; + MutexHolder holder(_lock); // See if the group is in the set/vector already PT(MouseWatcherGroup) pt = group; @@ -167,11 +172,18 @@ add_group(MouseWatcherGroup *group) { //////////////////////////////////////////////////////////////////// bool MouseWatcher:: remove_group(MouseWatcherGroup *group) { - remove_regions_from(_current_regions, group); - if (group->has_region(_preferred_region)) { + MutexHolder holder(_lock); + MutexHolder holder2(group->_lock); + + Regions only_a, only_b, both; + intersect_regions(only_a, only_b, both, + _current_regions, group->_regions); + _current_regions.swap(only_a); + + if (has_region_in(both, _preferred_region)) { _preferred_region = (MouseWatcherRegion *)NULL; } - if (group->has_region(_preferred_button_down_region)) { + if (has_region_in(both, _preferred_button_down_region)) { _preferred_button_down_region = (MouseWatcherRegion *)NULL; } @@ -189,6 +201,79 @@ remove_group(MouseWatcherGroup *group) { return false; } +//////////////////////////////////////////////////////////////////// +// Function: MouseWatcher::replace_group +// Access: Published +// Description: Atomically removes old_group fom the MouseWatcher, +// and replaces it with new_group. Presumably old_group +// and new_group might have some regions in common; +// these are handled properly. +// +// If old_group is not already present, simply adds +// new_group and returns false. Otherwise, removes +// old_group and adds new_group, and then returns true. +//////////////////////////////////////////////////////////////////// +bool MouseWatcher:: +replace_group(MouseWatcherGroup *old_group, MouseWatcherGroup *new_group) { + if (old_group == new_group) { + // Trivial. + return true; + } + + MutexHolder holder(_lock); + MutexHolder holder2(old_group->_lock); + MutexHolder holder3(new_group->_lock); + + // Figure out the list of regions that change + Regions remove, add, keep; + intersect_regions(remove, add, keep, + old_group->_regions, new_group->_regions); + + // Remove the old regions + if (!remove.empty()) { + Regions only_a, only_b, both; + intersect_regions(only_a, only_b, both, + _current_regions, remove); + _current_regions.swap(only_a); + + if (has_region_in(both, _preferred_region)) { + _preferred_region = (MouseWatcherRegion *)NULL; + } + if (has_region_in(both, _preferred_button_down_region)) { + _preferred_button_down_region = (MouseWatcherRegion *)NULL; + } + } + + // And add the new regions + if (!add.empty()) { + Regions new_list; + intersect_regions(new_list, new_list, new_list, + _current_regions, add); + _current_regions.swap(new_list); + } + + // Add the new group, if it's not already there. + PT(MouseWatcherGroup) pt = new_group; + Groups::iterator gi = + find(_groups.begin(), _groups.end(), pt); + if (gi == _groups.end()) { + _groups.push_back(new_group); + } + + // Remove the old group, if it is already there. + pt = old_group; + gi = find(_groups.begin(), _groups.end(), pt); + if (gi != _groups.end()) { + // Found it, now erase it + _groups.erase(gi); + return true; + } + + // Did not find the group to erase + return false; + +} + //////////////////////////////////////////////////////////////////// // Function: MouseWatcher::get_num_groups // Access: Published @@ -197,6 +282,7 @@ remove_group(MouseWatcherGroup *group) { //////////////////////////////////////////////////////////////////// int MouseWatcher:: get_num_groups() const { + MutexHolder holder(_lock); return _groups.size(); } @@ -208,6 +294,7 @@ get_num_groups() const { //////////////////////////////////////////////////////////////////// MouseWatcherGroup *MouseWatcher:: get_group(int n) const { + MutexHolder holder(_lock); nassertr(n >= 0 && n < (int)_groups.size(), NULL); return _groups[n]; } @@ -220,6 +307,7 @@ get_group(int n) const { //////////////////////////////////////////////////////////////////// void MouseWatcher:: output(ostream &out) const { + MutexHolder holder(_lock); DataNode::output(out); int count = _regions.size(); @@ -243,6 +331,7 @@ write(ostream &out, int indent_level) const { << "MouseWatcher " << get_name() << ":\n"; MouseWatcherGroup::write(out, indent_level + 2); + MutexHolder holder(_lock); if (!_groups.empty()) { Groups::const_iterator gi; for (gi = _groups.begin(); gi != _groups.end(); ++gi) { @@ -259,10 +348,12 @@ write(ostream &out, int indent_level) const { // Access: Protected // Description: Fills up the "regions" list with the set of regions // that the indicated point is over, sorted in order by -// pointer. +// pointer. Assumes the lock is held. //////////////////////////////////////////////////////////////////// void MouseWatcher:: -get_over_regions(MouseWatcher::VRegions ®ions, const LPoint2f &pos) const { +get_over_regions(MouseWatcher::Regions ®ions, const LPoint2f &pos) const { + nassertv(_lock.debug_is_locked()); + // Ensure the vector is empty before we begin. regions.clear(); @@ -308,15 +399,16 @@ get_over_regions(MouseWatcher::VRegions ®ions, const LPoint2f &pos) const { // Description: Returns the innermost region of all the regions // indicated in the given vector (usually, the regions // the mouse is over). This is the "preferred" region -// that gets some special treatment. +// that gets some special treatment. Assumes the lock +// is already held. //////////////////////////////////////////////////////////////////// MouseWatcherRegion *MouseWatcher:: -get_preferred_region(const MouseWatcher::VRegions ®ions) { +get_preferred_region(const MouseWatcher::Regions ®ions) { if (regions.empty()) { return (MouseWatcherRegion *)NULL; } - VRegions::const_iterator ri; + Regions::const_iterator ri; ri = regions.begin(); MouseWatcherRegion *preferred = *ri; ++ri; @@ -339,18 +431,21 @@ get_preferred_region(const MouseWatcher::VRegions ®ions) { // mouse to be over--to the indicated list, and throws // whatever events are appropriate because of that. // -// The list passed in is destroyed. +// The list passed in is destroyed. Assumes the lock is +// already held. //////////////////////////////////////////////////////////////////// void MouseWatcher:: -set_current_regions(MouseWatcher::VRegions ®ions) { +set_current_regions(MouseWatcher::Regions ®ions) { + nassertv(_lock.debug_is_locked()); + // Set up a parameter for passing through any change events. MouseWatcherParameter param; param.set_modifier_buttons(_mods); param.set_mouse(_mouse); // Now do a standard sorted comparison between the two vectors. - VRegions::const_iterator new_ri = regions.begin(); - VRegions::const_iterator old_ri = _current_regions.begin(); + Regions::const_iterator new_ri = regions.begin(); + Regions::const_iterator old_ri = _current_regions.begin(); // Queue up all the new regions so we can send the within patterns // all at once, after all of the without patterns have been thrown. @@ -436,17 +531,20 @@ set_current_regions(MouseWatcher::VRegions ®ions) { //////////////////////////////////////////////////////////////////// // Function: MouseWatcher::clear_current_regions // Access: Protected -// Description: Empties the set of current regions. +// Description: Empties the set of current regions. Assumes the lock +// is already held. //////////////////////////////////////////////////////////////////// void MouseWatcher:: clear_current_regions() { + nassertv(_lock.debug_is_locked()); + if (!_current_regions.empty()) { // Set up a parameter for passing through any change events. MouseWatcherParameter param; param.set_modifier_buttons(_mods); param.set_mouse(_mouse); - VRegions::const_iterator old_ri = _current_regions.begin(); + Regions::const_iterator old_ri = _current_regions.begin(); while (old_ri != _current_regions.end()) { // Here's a region we don't have any more. @@ -469,43 +567,45 @@ clear_current_regions() { //////////////////////////////////////////////////////////////////// // Function: MouseWatcher::intersect_regions // Access: Protected, Static -// Description: Sets result to be the intersection of the list of -// regions in regions_a and regions_b. It is assumed -// that both vectors are already sorted in pointer -// order. +// Description: Computes the list of regions that are in both +// regions_a and regions_b, as well as the list of +// regions only in regions_a, and the list of regions +// only in regions_b. Any or all of the three output +// lists may be the same object, but they must be +// different objects from both of the input lists. +// +// It is assumed that both vectors are already sorted in +// pointer order. It is also assumed that any relevant +// locks are already held. //////////////////////////////////////////////////////////////////// void MouseWatcher:: -intersect_regions(MouseWatcher::VRegions &result, - const MouseWatcher::VRegions ®ions_a, - const MouseWatcher::VRegions ®ions_b) { - // Get a temporary vector for storing the result in. We don't use - // result directly, because it might be the same vector as one of a - // or b. - VRegions temp; - +intersect_regions(MouseWatcher::Regions &only_a, + MouseWatcher::Regions &only_b, + MouseWatcher::Regions &both, + const MouseWatcher::Regions ®ions_a, + const MouseWatcher::Regions ®ions_b) { // Now do a standard sorted intersection between the two vectors. - VRegions::const_iterator a_ri = regions_a.begin(); - VRegions::const_iterator b_ri = regions_b.begin(); + Regions::const_iterator a_ri = regions_a.begin(); + Regions::const_iterator b_ri = regions_b.begin(); while (a_ri != regions_a.end() && b_ri != regions_b.end()) { if ((*a_ri) < (*b_ri)) { // Here's a region in a, not in b. + only_a.push_back(*a_ri); ++a_ri; } else if ((*b_ri) < (*a_ri)) { // Here's a region in b, not in a. + only_b.push_back(*b_ri); ++b_ri; } else { // Here's a region in both vectors. - temp.push_back(*a_ri); + both.push_back(*a_ri); ++a_ri; ++b_ri; } } - - // Now store the result! - result.swap(temp); } //////////////////////////////////////////////////////////////////// @@ -513,58 +613,39 @@ intersect_regions(MouseWatcher::VRegions &result, // Access: Protected, Static // Description: Removes the indicated region from the given vector. // Assumes the vector is sorted in pointer order. +// Returns true if removed, false if it wasn't there. +// Assumes any relevent locks are already held. //////////////////////////////////////////////////////////////////// -void MouseWatcher:: -remove_region_from(MouseWatcher::VRegions ®ions, +bool MouseWatcher:: +remove_region_from(MouseWatcher::Regions ®ions, MouseWatcherRegion *region) { PT(MouseWatcherRegion) ptr = region; - VRegions::iterator ri = lower_bound(regions.begin(), regions.end(), ptr); + Regions::iterator ri = lower_bound(regions.begin(), regions.end(), ptr); if (ri != regions.end() && (*ri) == ptr) { // The region is in the vector. Remove it. regions.erase(ri); + return true; } + + return false; } //////////////////////////////////////////////////////////////////// -// Function: MouseWatcher::remove_regions_from +// Function: MouseWatcher::has_region_in // Access: Protected, Static -// Description: Removes all the regions in the indicated group from -// the given vector. Assumes the vector is sorted in -// pointer order. +// Description: Returns true if the indicated region is a member of +// the given sorted list, false otherwise. //////////////////////////////////////////////////////////////////// -void MouseWatcher:: -remove_regions_from(MouseWatcher::VRegions ®ions, - MouseWatcherGroup *group) { - // Since the group stores a set of regions, which are also sorted in - // pointer order, we can just do an intersection operation here. - VRegions temp; - - VRegions::const_iterator a_ri = regions.begin(); - MouseWatcherGroup::Regions::const_iterator b_ri = group->_regions.begin(); - - while (a_ri != regions.end() && b_ri != group->_regions.end()) { - if ((*a_ri) < (*b_ri)) { - // Here's a region in the group, not in regions. - ++a_ri; - - } else if ((*b_ri) < (*a_ri)) { - // Here's a region in regions, not in the group. - temp.push_back(*b_ri); - ++b_ri; - - } else { - // Here's a region in the group and in regions. - ++a_ri; - ++b_ri; - } - } - - // Now store the result! - regions.swap(temp); +bool MouseWatcher:: +has_region_in(const MouseWatcher::Regions ®ions, + MouseWatcherRegion *region) { + PT(MouseWatcherRegion) ptr = region; + Regions::const_iterator ri = lower_bound(regions.begin(), regions.end(), ptr); + return (ri != regions.end() && (*ri) == ptr); } //////////////////////////////////////////////////////////////////// -// Function: MouseWatcher::throw_event_for +// Function: MouseWatcher::throw_event_pattern // Access: Protected // Description: Throws an event associated with the indicated region, // using the given pattern. @@ -629,6 +710,8 @@ throw_event_pattern(const string &pattern, const MouseWatcherRegion *region, //////////////////////////////////////////////////////////////////// void MouseWatcher:: move() { + nassertv(_lock.debug_is_locked()); + MouseWatcherParameter param; param.set_modifier_buttons(_mods); param.set_mouse(_mouse); @@ -646,6 +729,8 @@ move() { //////////////////////////////////////////////////////////////////// void MouseWatcher:: press(ButtonHandle button) { + nassertv(_lock.debug_is_locked()); + MouseWatcherParameter param; param.set_button(button); param.set_modifier_buttons(_mods); @@ -695,6 +780,8 @@ press(ButtonHandle button) { //////////////////////////////////////////////////////////////////// void MouseWatcher:: release(ButtonHandle button) { + nassertv(_lock.debug_is_locked()); + MouseWatcherParameter param; param.set_button(button); param.set_modifier_buttons(_mods); @@ -738,6 +825,8 @@ release(ButtonHandle button) { //////////////////////////////////////////////////////////////////// void MouseWatcher:: keystroke(int keycode) { + nassertv(_lock.debug_is_locked()); + MouseWatcherParameter param; param.set_keycode(keycode); param.set_modifier_buttons(_mods); @@ -785,6 +874,8 @@ keystroke(int keycode) { void MouseWatcher:: candidate(const wstring &candidate_string, size_t highlight_start, size_t highlight_end, size_t cursor_pos) { + nassertv(_lock.debug_is_locked()); + MouseWatcherParameter param; param.set_candidate(candidate_string, highlight_start, highlight_end, cursor_pos); param.set_modifier_buttons(_mods); @@ -827,6 +918,8 @@ candidate(const wstring &candidate_string, size_t highlight_start, //////////////////////////////////////////////////////////////////// void MouseWatcher:: global_keyboard_press(const MouseWatcherParameter ¶m) { + nassertv(_lock.debug_is_locked()); + Regions::const_iterator ri; for (ri = _regions.begin(); ri != _regions.end(); ++ri) { MouseWatcherRegion *region = (*ri); @@ -860,6 +953,8 @@ global_keyboard_press(const MouseWatcherParameter ¶m) { //////////////////////////////////////////////////////////////////// void MouseWatcher:: global_keyboard_release(const MouseWatcherParameter ¶m) { + nassertv(_lock.debug_is_locked()); + Regions::const_iterator ri; for (ri = _regions.begin(); ri != _regions.end(); ++ri) { MouseWatcherRegion *region = (*ri); @@ -886,11 +981,13 @@ global_keyboard_release(const MouseWatcherParameter ¶m) { //////////////////////////////////////////////////////////////////// // Function: MouseWatcher::enter_region // Access: Protected -// Description: Called internally to indicate the mouse pointer is no -// longer favoring the indicated region. +// Description: Called internally to indicate the mouse pointer is +// favoring the indicated region. //////////////////////////////////////////////////////////////////// void MouseWatcher:: enter_region(MouseWatcherRegion *region, const MouseWatcherParameter ¶m) { + nassertv(_lock.debug_is_locked()); + region->enter(param); throw_event_pattern(_enter_pattern, region, ButtonHandle::none()); if (_implicit_click) { @@ -908,6 +1005,8 @@ enter_region(MouseWatcherRegion *region, const MouseWatcherParameter ¶m) { //////////////////////////////////////////////////////////////////// void MouseWatcher:: exit_region(MouseWatcherRegion *region, const MouseWatcherParameter ¶m) { + nassertv(_lock.debug_is_locked()); + if (_implicit_click) { MouseWatcherParameter param1(param); param1.set_button(MouseButton::one()); @@ -925,6 +1024,8 @@ exit_region(MouseWatcherRegion *region, const MouseWatcherParameter ¶m) { //////////////////////////////////////////////////////////////////// void MouseWatcher:: set_no_mouse() { + nassertv(_lock.debug_is_locked()); + if (_has_mouse) { // Hide the mouse pointer. if (!_geometry.is_null()) { @@ -945,6 +1046,8 @@ set_no_mouse() { //////////////////////////////////////////////////////////////////// void MouseWatcher:: set_mouse(const LVecBase2f &xy, const LVecBase2f &pixel_xy) { + nassertv(_lock.debug_is_locked()); + if (!_geometry.is_null()) { // Transform the mouse pointer. _geometry->set_transform(TransformState::make_pos(LVecBase3f(xy[0], 0, xy[1]))); @@ -958,7 +1061,7 @@ set_mouse(const LVecBase2f &xy, const LVecBase2f &pixel_xy) { _mouse = xy; _mouse_pixel = pixel_xy; - VRegions regions; + Regions regions; get_over_regions(regions, _mouse); set_current_regions(regions); } @@ -997,6 +1100,8 @@ consider_keyboard_suppress(const MouseWatcherRegion *region) { //////////////////////////////////////////////////////////////////// void MouseWatcher:: do_transmit_data(const DataNodeTransmit &input, DataNodeTransmit &output) { + MutexHolder holder(_lock); + // Initially, we do not suppress any events to objects below us in // the data graph. _internal_suppress = 0; diff --git a/panda/src/tform/mouseWatcher.h b/panda/src/tform/mouseWatcher.h index d336c09417..0298dcfa8b 100644 --- a/panda/src/tform/mouseWatcher.h +++ b/panda/src/tform/mouseWatcher.h @@ -111,6 +111,7 @@ PUBLISHED: bool add_group(MouseWatcherGroup *group); bool remove_group(MouseWatcherGroup *group); + bool replace_group(MouseWatcherGroup *old_group, MouseWatcherGroup *new_group); int get_num_groups() const; MouseWatcherGroup *get_group(int n) const; @@ -119,20 +120,21 @@ public: virtual void write(ostream &out, int indent_level = 0) const; protected: - typedef pvector< PT(MouseWatcherRegion) > VRegions; - void get_over_regions(VRegions ®ions, const LPoint2f &pos) const; - static MouseWatcherRegion *get_preferred_region(const VRegions ®ions); + void get_over_regions(Regions ®ions, const LPoint2f &pos) const; + static MouseWatcherRegion *get_preferred_region(const Regions ®ions); - void set_current_regions(VRegions ®ions); + void set_current_regions(Regions ®ions); void clear_current_regions(); - static void intersect_regions(MouseWatcher::VRegions &result, - const MouseWatcher::VRegions ®ions_a, - const MouseWatcher::VRegions ®ions_b); - static void remove_region_from(MouseWatcher::VRegions ®ions, - MouseWatcherRegion *region); - static void remove_regions_from(MouseWatcher::VRegions ®ions, - MouseWatcherGroup *group); + static void intersect_regions(Regions &only_a, + Regions &only_b, + Regions &both, + const Regions ®ions_a, + const Regions ®ions_b); + static bool remove_region_from(Regions ®ions, + MouseWatcherRegion *region); + static bool has_region_in(const Regions ®ions, + MouseWatcherRegion *region); void throw_event_pattern(const string &pattern, const MouseWatcherRegion *region, @@ -171,7 +173,7 @@ protected: LPoint2f _mouse; LPoint2f _mouse_pixel; - VRegions _current_regions; + Regions _current_regions; PT(MouseWatcherRegion) _preferred_region; PT(MouseWatcherRegion) _preferred_button_down_region; bool _button_down; diff --git a/panda/src/tform/mouseWatcherGroup.cxx b/panda/src/tform/mouseWatcherGroup.cxx index a597288a06..0e02b9920e 100644 --- a/panda/src/tform/mouseWatcherGroup.cxx +++ b/panda/src/tform/mouseWatcherGroup.cxx @@ -19,6 +19,7 @@ #include "mouseWatcherGroup.h" #include "lineSegs.h" #include "indent.h" +#include "mutexHolder.h" TypeHandle MouseWatcherGroup::_type_handle; @@ -55,6 +56,8 @@ void MouseWatcherGroup:: add_region(MouseWatcherRegion *region) { PT(MouseWatcherRegion) pt = region; + MutexHolder holder(_lock); + // We will only bother to check for duplicates in the region list if // we are building a development Panda. The overhead for doing this // may be too high if we have many regions. @@ -82,6 +85,8 @@ add_region(MouseWatcherRegion *region) { //////////////////////////////////////////////////////////////////// bool MouseWatcherGroup:: has_region(MouseWatcherRegion *region) const { + MutexHolder holder(_lock); + // See if the region is in the vector. PT(MouseWatcherRegion) pt = region; Regions::const_iterator ri = @@ -103,6 +108,8 @@ has_region(MouseWatcherRegion *region) const { //////////////////////////////////////////////////////////////////// bool MouseWatcherGroup:: remove_region(MouseWatcherRegion *region) { + MutexHolder holder(_lock); + // See if the region is in the vector. PT(MouseWatcherRegion) pt = region; Regions::iterator ri = @@ -137,6 +144,8 @@ remove_region(MouseWatcherRegion *region) { //////////////////////////////////////////////////////////////////// MouseWatcherRegion *MouseWatcherGroup:: find_region(const string &name) const { + MutexHolder holder(_lock); + Regions::const_iterator ri; for (ri = _regions.begin(); ri != _regions.end(); ++ri) { MouseWatcherRegion *region = (*ri); @@ -155,6 +164,8 @@ find_region(const string &name) const { //////////////////////////////////////////////////////////////////// void MouseWatcherGroup:: clear_regions() { + MutexHolder holder(_lock); + _regions.clear(); #ifndef NDEBUG @@ -172,18 +183,26 @@ clear_regions() { //////////////////////////////////////////////////////////////////// int MouseWatcherGroup:: get_num_regions() const { + MutexHolder holder(_lock); + return _regions.size(); } //////////////////////////////////////////////////////////////////// // Function: MouseWatcherGroup::get_region // Access: Published -// Description: Returns the nth regions in the group. +// Description: Returns the nth region of the group; returns NULL if +// there is no nth region. Note that this is not +// thread-safe; another thread might have removed the +// nth region before you called this method. //////////////////////////////////////////////////////////////////// MouseWatcherRegion *MouseWatcherGroup:: get_region(int n) const { - nassertr(n >= 0 && n < (int)_regions.size(), NULL); - return _regions[n]; + MutexHolder holder(_lock); + if (n >= 0 && n < (int)_regions.size()) { + return _regions[n]; + } + return NULL; } //////////////////////////////////////////////////////////////////// @@ -203,6 +222,8 @@ output(ostream &out) const { //////////////////////////////////////////////////////////////////// void MouseWatcherGroup:: write(ostream &out, int indent_level) const { + MutexHolder holder(_lock); + Regions::const_iterator ri; for (ri = _regions.begin(); ri != _regions.end(); ++ri) { MouseWatcherRegion *region = (*ri); @@ -221,6 +242,8 @@ write(ostream &out, int indent_level) const { //////////////////////////////////////////////////////////////////// void MouseWatcherGroup:: show_regions(const NodePath &render2d) { + MutexHolder holder(_lock); + _show_regions = true; _show_regions_root = render2d.attach_new_node("show_regions"); _show_regions_root.set_bin("unsorted", 0); @@ -238,6 +261,8 @@ show_regions(const NodePath &render2d) { //////////////////////////////////////////////////////////////////// void MouseWatcherGroup:: set_color(const Colorf &color) { + MutexHolder holder(_lock); + _color = color; update_regions(); } @@ -252,6 +277,8 @@ set_color(const Colorf &color) { //////////////////////////////////////////////////////////////////// void MouseWatcherGroup:: hide_regions() { + MutexHolder holder(_lock); + _show_regions_root.remove_node(); _show_regions = false; _vizzes.clear(); @@ -263,10 +290,12 @@ hide_regions() { // Function: MouseWatcherGroup::update_regions // Access: Private // Description: Internally regenerates the show_regions() -// visualization. +// visualization. Assumes the lock is already held. //////////////////////////////////////////////////////////////////// void MouseWatcherGroup:: update_regions() { + nassertv(_lock.debug_is_locked()); + _show_regions_root.node()->remove_all_children(); _vizzes.clear(); _vizzes.reserve(_regions.size()); @@ -284,10 +313,12 @@ update_regions() { // Access: Private // Description: Creates a node to represent the indicated region, and // attaches it to the _show_regions_root. Does not add -// it to _vizzes. +// it to _vizzes. Assumes the lock is already held. //////////////////////////////////////////////////////////////////// PandaNode *MouseWatcherGroup:: make_viz_region(MouseWatcherRegion *region) { + nassertr(_lock.debug_is_locked(), NULL); + LineSegs ls("show_regions"); ls.set_color(_color); diff --git a/panda/src/tform/mouseWatcherGroup.h b/panda/src/tform/mouseWatcherGroup.h index e56ed6368a..696c5ddf32 100644 --- a/panda/src/tform/mouseWatcherGroup.h +++ b/panda/src/tform/mouseWatcherGroup.h @@ -26,6 +26,7 @@ #include "referenceCount.h" #include "pvector.h" #include "nodePath.h" +#include "pmutex.h" //////////////////////////////////////////////////////////////////// // Class : MouseWatcherGroup @@ -60,6 +61,11 @@ protected: typedef pvector< PT(MouseWatcherRegion) > Regions; Regions _regions; + // This mutex protects the above list of regions, as well as the + // below list of vizzes. It is also referenced directly by + // MouseWatcher, a derived class. + Mutex _lock; + private: #ifndef NDEBUG void update_regions();