From 3a45104793bf8c9f8cd0e31546e6b91a08e742f1 Mon Sep 17 00:00:00 2001 From: barnold1953 Date: Thu, 22 May 2014 17:55:25 -0700 Subject: [PATCH 1/3] Made overlays thread safe for write operations, which solved a script crash. --- interface/src/ui/overlays/Overlays.cpp | 6 +++--- interface/src/ui/overlays/Overlays.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index 9e4a594bbc..6c6f150ae7 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -96,7 +96,6 @@ void Overlays::render3D() { } } -// TODO: make multi-threaded safe unsigned int Overlays::addOverlay(const QString& type, const QScriptValue& properties) { unsigned int thisID = 0; bool created = false; @@ -140,6 +139,7 @@ unsigned int Overlays::addOverlay(const QString& type, const QScriptValue& prope } if (created) { + QWriteLocker lock(&_lock); thisID = _nextOverlayID; _nextOverlayID++; if (is3D) { @@ -152,9 +152,9 @@ unsigned int Overlays::addOverlay(const QString& type, const QScriptValue& prope return thisID; } -// TODO: make multi-threaded safe bool Overlays::editOverlay(unsigned int id, const QScriptValue& properties) { Overlay* thisOverlay = NULL; + QWriteLocker lock(&_lock); if (_overlays2D.contains(id)) { thisOverlay = _overlays2D[id]; } else if (_overlays3D.contains(id)) { @@ -167,9 +167,9 @@ bool Overlays::editOverlay(unsigned int id, const QScriptValue& properties) { return false; } -// TODO: make multi-threaded safe void Overlays::deleteOverlay(unsigned int id) { Overlay* overlayToDelete; + QWriteLocker lock(&_lock); if (_overlays2D.contains(id)) { overlayToDelete = _overlays2D.take(id); } else if (_overlays3D.contains(id)) { diff --git a/interface/src/ui/overlays/Overlays.h b/interface/src/ui/overlays/Overlays.h index b3477be0c2..20015b8af9 100644 --- a/interface/src/ui/overlays/Overlays.h +++ b/interface/src/ui/overlays/Overlays.h @@ -45,6 +45,7 @@ private: QList _overlaysToDelete; unsigned int _nextOverlayID; QGLWidget* _parent; + QReadWriteLock _lock; }; From 9553271fa1da0a52aecabf5cde750d63ebe81d7d Mon Sep 17 00:00:00 2001 From: barnold1953 Date: Fri, 23 May 2014 10:44:40 -0700 Subject: [PATCH 2/3] Added some missing locks to make Overlays thread safe for reading as well. --- interface/src/ui/overlays/Overlays.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index 6c6f150ae7..bcac86d7f1 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -39,6 +39,7 @@ void Overlays::init(QGLWidget* parent) { } void Overlays::update(float deltatime) { + QWriteLocker lock(&_lock); foreach (Overlay* thisOverlay, _overlays2D) { thisOverlay->update(deltatime); } @@ -52,12 +53,14 @@ void Overlays::update(float deltatime) { } void Overlays::render2D() { + QReadLocker lock(&_lock); foreach(Overlay* thisOverlay, _overlays2D) { thisOverlay->render(); } } void Overlays::render3D() { + QReadLocker lock(&_lock); if (_overlays3D.size() == 0) { return; } @@ -182,6 +185,7 @@ void Overlays::deleteOverlay(unsigned int id) { } unsigned int Overlays::getOverlayAtPoint(const glm::vec2& point) { + QReadLocker lock(&_lock); QMapIterator i(_overlays2D); i.toBack(); while (i.hasPrevious()) { From 6817c9682e5483eb4aff765f054f8ecf956eb972 Mon Sep 17 00:00:00 2001 From: barnold1953 Date: Fri, 23 May 2014 11:17:55 -0700 Subject: [PATCH 3/3] Added a new lock specific to deleting, and improved safety --- interface/src/ui/overlays/Overlays.cpp | 43 ++++++++++++++++---------- interface/src/ui/overlays/Overlays.h | 1 + 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index bcac86d7f1..da7b05ead7 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -39,15 +39,22 @@ void Overlays::init(QGLWidget* parent) { } void Overlays::update(float deltatime) { - QWriteLocker lock(&_lock); - foreach (Overlay* thisOverlay, _overlays2D) { - thisOverlay->update(deltatime); + + { + QWriteLocker lock(&_lock); + foreach(Overlay* thisOverlay, _overlays2D) { + thisOverlay->update(deltatime); + } + foreach(Overlay* thisOverlay, _overlays3D) { + thisOverlay->update(deltatime); + } } - foreach (Overlay* thisOverlay, _overlays3D) { - thisOverlay->update(deltatime); - } - while (!_overlaysToDelete.isEmpty()) { - delete _overlaysToDelete.takeLast(); + + if (!_overlaysToDelete.isEmpty()) { + QWriteLocker lock(&_deleteLock); + do { + delete _overlaysToDelete.takeLast(); + } while (!_overlaysToDelete.isEmpty()); } } @@ -172,15 +179,19 @@ bool Overlays::editOverlay(unsigned int id, const QScriptValue& properties) { void Overlays::deleteOverlay(unsigned int id) { Overlay* overlayToDelete; - QWriteLocker lock(&_lock); - if (_overlays2D.contains(id)) { - overlayToDelete = _overlays2D.take(id); - } else if (_overlays3D.contains(id)) { - overlayToDelete = _overlays3D.take(id); - } else { - return; + + { + QWriteLocker lock(&_lock); + if (_overlays2D.contains(id)) { + overlayToDelete = _overlays2D.take(id); + } else if (_overlays3D.contains(id)) { + overlayToDelete = _overlays3D.take(id); + } else { + return; + } } - + + QWriteLocker lock(&_deleteLock); _overlaysToDelete.push_back(overlayToDelete); } diff --git a/interface/src/ui/overlays/Overlays.h b/interface/src/ui/overlays/Overlays.h index 20015b8af9..2fbdb993f4 100644 --- a/interface/src/ui/overlays/Overlays.h +++ b/interface/src/ui/overlays/Overlays.h @@ -46,6 +46,7 @@ private: unsigned int _nextOverlayID; QGLWidget* _parent; QReadWriteLock _lock; + QReadWriteLock _deleteLock; };