From 867cf6b60ae7d97fb3858116634d9d16f3b3aa0f Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 19 Jan 2020 06:30:10 -0800 Subject: [PATCH 01/10] test_Http: Better completion checking --- simgear/io/test_HTTP.cxx | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/simgear/io/test_HTTP.cxx b/simgear/io/test_HTTP.cxx index ccef3f9e..52034a3b 100644 --- a/simgear/io/test_HTTP.cxx +++ b/simgear/io/test_HTTP.cxx @@ -381,6 +381,26 @@ void waitForFailed(HTTP::Client* cl, TestRequest* tr) cerr << "timed out waiting for failure" << endl; } +using CompletionCheck = std::function; + +bool waitFor(HTTP::Client* cl, CompletionCheck ccheck) +{ + SGTimeStamp start(SGTimeStamp::now()); + while (start.elapsedMSec() < 10000) { + cl->update(); + testServer.poll(); + + if (ccheck()) { + return true; + } + SGTimeStamp::sleepForMSec(15); + } + + cerr << "timed out" << endl; + return false; +} + + int main(int argc, char* argv[]) { sglog().setLogLevels( SG_ALL, SG_INFO ); @@ -456,6 +476,8 @@ int main(int argc, char* argv[]) // larger get request for (unsigned int i=0; i> 2); } @@ -600,9 +622,10 @@ cout << "testing proxy close" << endl; HTTP::Request_ptr own3(tr3); cl.makeRequest(tr3); - waitForComplete(&cl, tr3); - SG_VERIFY(tr->complete); - SG_VERIFY(tr2->complete); + SG_VERIFY(waitFor(&cl, [tr, tr2, tr3]() { + return tr->complete && tr2->complete &&tr3->complete; + })); + SG_CHECK_EQUAL(tr->bodyData, string(BODY1)); SG_CHECK_EQUAL(tr2->responseLength(), strlen(BODY3)); @@ -631,9 +654,9 @@ cout << "testing proxy close" << endl; HTTP::Request_ptr own3(tr3); cl.makeRequest(tr3); - waitForComplete(&cl, tr3); - SG_VERIFY(tr->complete); - SG_VERIFY(tr2->complete); + SG_VERIFY(waitFor(&cl, [tr, tr2, tr3]() { + return tr->complete && tr2->complete &&tr3->complete; + })); SG_CHECK_EQUAL(tr->responseLength(), strlen(BODY1)); SG_CHECK_EQUAL(tr->responseBytesReceived(), strlen(BODY1)); From aab88b411ed35e4ea8918af6e8ab04865b7e58a1 Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Wed, 22 Jan 2020 13:44:47 +0100 Subject: [PATCH 02/10] Switch to C++11 threads, mutexes and lock_guards. Switching to C++11 condition_variables requires some more thought. --- simgear/bvh/BVHPager.cxx | 14 +- simgear/debug/BufferedLogCallback.cxx | 8 +- simgear/debug/logstream.cxx | 18 +- simgear/debug/logstream.hxx | 2 + simgear/emesary/Transmitter.hxx | 4 +- simgear/emesary/test_emesary.cxx | 1 + simgear/hla/RTIFederateFactoryRegistry.cxx | 8 +- simgear/io/HTTPClient.cxx | 7 +- simgear/io/raw_socket.cxx | 3 +- simgear/misc/lru_cache.hxx | 15 +- simgear/scene/material/Effect.cxx | 11 +- simgear/scene/material/Effect.hxx | 4 +- simgear/scene/material/mat.cxx | 6 +- simgear/scene/material/mat.hxx | 3 +- simgear/scene/material/matlib.cxx | 5 +- simgear/scene/model/ModelRegistry.cxx | 1 - simgear/scene/tsync/terrasync.cxx | 35 ++-- simgear/structure/SGAtomic.cxx | 10 +- simgear/structure/SGAtomic.hxx | 2 +- simgear/structure/StringTable.cxx | 3 +- simgear/structure/StringTable.hxx | 4 +- simgear/structure/commands.cxx | 8 +- simgear/threads/CMakeLists.txt | 1 - simgear/threads/SGGuard.hxx | 35 ---- simgear/threads/SGQueue.hxx | 46 ++--- simgear/threads/SGThread.cxx | 210 +++++---------------- simgear/threads/SGThread.hxx | 47 +---- 27 files changed, 160 insertions(+), 351 deletions(-) delete mode 100644 simgear/threads/SGGuard.hxx diff --git a/simgear/bvh/BVHPager.cxx b/simgear/bvh/BVHPager.cxx index 829b297c..6f533f40 100644 --- a/simgear/bvh/BVHPager.cxx +++ b/simgear/bvh/BVHPager.cxx @@ -20,9 +20,9 @@ #include "BVHPager.hxx" #include +#include #include -#include #include "BVHPageNode.hxx" #include "BVHPageRequest.hxx" @@ -37,12 +37,12 @@ struct BVHPager::_PrivateData : protected SGThread { struct _LockedQueue { void _push(const _Request& request) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); _requestList.push_back(request); } _Request _pop() { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); if (_requestList.empty()) return _Request(); _Request request; @@ -51,7 +51,7 @@ struct BVHPager::_PrivateData : protected SGThread { return request; } private: - SGMutex _mutex; + std::mutex _mutex; _RequestList _requestList; }; @@ -62,7 +62,7 @@ struct BVHPager::_PrivateData : protected SGThread { } void _push(const _Request& request) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); bool needSignal = _requestList.empty(); _requestList.push_back(request); if (needSignal) @@ -70,7 +70,7 @@ struct BVHPager::_PrivateData : protected SGThread { } _Request _pop() { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); while (_requestList.empty()) _waitCondition.wait(_mutex); _Request request; @@ -79,7 +79,7 @@ struct BVHPager::_PrivateData : protected SGThread { return request; } private: - SGMutex _mutex; + std::mutex _mutex; SGWaitCondition _waitCondition; _RequestList _requestList; }; diff --git a/simgear/debug/BufferedLogCallback.cxx b/simgear/debug/BufferedLogCallback.cxx index a0e63976..12b9065c 100644 --- a/simgear/debug/BufferedLogCallback.cxx +++ b/simgear/debug/BufferedLogCallback.cxx @@ -26,9 +26,9 @@ #include #include -#include #include // for malloc +#include namespace simgear { @@ -36,7 +36,7 @@ namespace simgear class BufferedLogCallback::BufferedLogCallbackPrivate { public: - SGMutex m_mutex; + std::mutex m_mutex; vector_cstring m_buffer; unsigned int m_stamp; unsigned int m_maxLength; @@ -74,7 +74,7 @@ void BufferedLogCallback::operator()(sgDebugClass c, sgDebugPriority p, msg = (vector_cstring::value_type) strdup(aMessage.c_str()); } - SGGuard g(d->m_mutex); + std::lock_guard g(d->m_mutex); d->m_buffer.push_back(msg); d->m_stamp++; } @@ -86,7 +86,7 @@ unsigned int BufferedLogCallback::stamp() const unsigned int BufferedLogCallback::threadsafeCopy(vector_cstring& aOutput) { - SGGuard g(d->m_mutex); + std::lock_guard g(d->m_mutex); size_t sz = d->m_buffer.size(); aOutput.resize(sz); memcpy(aOutput.data(), d->m_buffer.data(), sz * sizeof(vector_cstring::value_type)); diff --git a/simgear/debug/logstream.cxx b/simgear/debug/logstream.cxx index fe4dac42..d99b0dc2 100644 --- a/simgear/debug/logstream.cxx +++ b/simgear/debug/logstream.cxx @@ -28,13 +28,13 @@ #include #include #include +#include #include #include #include #include -#include #include #include @@ -398,7 +398,7 @@ public: removeCallbacks(); } - SGMutex m_lock; + std::mutex m_lock; SGBlockingQueue m_entries; // log entries posted during startup @@ -427,7 +427,7 @@ public: void startLog() { - SGGuard g(m_lock); + std::lock_guard g(m_lock); if (m_isRunning) return; m_isRunning = true; start(); @@ -440,7 +440,7 @@ public: } { - SGGuard g(m_lock); + std::lock_guard g(m_lock); m_startupLogging = on; m_startupEntries.clear(); } @@ -456,7 +456,7 @@ public: return; } { - SGGuard g(m_lock); + std::lock_guard g(m_lock); if (m_startupLogging) { // save to the startup list for not-yet-added callbacks to // pull down on startup @@ -474,7 +474,7 @@ public: bool stop() { { - SGGuard g(m_lock); + std::lock_guard g(m_lock); if (!m_isRunning) { return false; } @@ -574,7 +574,7 @@ public: ///////////////////////////////////////////////////////////////////////////// static std::unique_ptr global_logstream; -static SGMutex global_logStreamLock; +static std::mutex global_logStreamLock; logstream::logstream() { @@ -742,7 +742,7 @@ sglog() // http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf // in the absence of portable memory barrier ops in Simgear, // let's keep this correct & safe - SGGuard g(global_logStreamLock); + std::lock_guard g(global_logStreamLock); if( !global_logstream ) global_logstream.reset(new logstream); @@ -823,7 +823,7 @@ void requestConsole() void shutdownLogging() { - SGGuard g(global_logStreamLock); + std::lock_guard g(global_logStreamLock); global_logstream.reset(); } diff --git a/simgear/debug/logstream.hxx b/simgear/debug/logstream.hxx index ec101d44..201d2160 100644 --- a/simgear/debug/logstream.hxx +++ b/simgear/debug/logstream.hxx @@ -210,9 +210,11 @@ logstream& sglog(); } } while(0) #ifdef FG_NDEBUG # define SG_LOG(C,P,M) do { if((P) == SG_POPUP) SG_LOGX(C,P,M) } while(0) +# define SG_LOG_NAN(C,P,M) SG_LOG(C,P,M) # define SG_HEXDUMP(C,P,MEM,LEN) #else # define SG_LOG(C,P,M) SG_LOGX(C,P,M) +# define SG_LOG_NAN(C,P,M) do { SG_LOGX(C,P,M); throw std::overflow_error(M); } while(0) # define SG_LOG_HEXDUMP(C,P,MEM,LEN) if(sglog().would_log(C,P)) sglog().hexdump(C, P, __FILE__, __LINE__, MEM, LEN) #endif diff --git a/simgear/emesary/Transmitter.hxx b/simgear/emesary/Transmitter.hxx index 339a5488..ba71b63f 100644 --- a/simgear/emesary/Transmitter.hxx +++ b/simgear/emesary/Transmitter.hxx @@ -27,7 +27,7 @@ #include #include #include -#include +#include namespace simgear { @@ -41,7 +41,7 @@ namespace simgear RecipientList recipient_list; RecipientList deleted_recipients; int CurrentRecipientIndex = 0; - SGMutex _lock; + std::mutex _lock; std::atomic receiveDepth; std::atomic sentMessageCount; diff --git a/simgear/emesary/test_emesary.cxx b/simgear/emesary/test_emesary.cxx index 55acd622..bb854059 100644 --- a/simgear/emesary/test_emesary.cxx +++ b/simgear/emesary/test_emesary.cxx @@ -7,6 +7,7 @@ #include +#include #include using std::cout; diff --git a/simgear/hla/RTIFederateFactoryRegistry.cxx b/simgear/hla/RTIFederateFactoryRegistry.cxx index 8584a429..d70e89d2 100644 --- a/simgear/hla/RTIFederateFactoryRegistry.cxx +++ b/simgear/hla/RTIFederateFactoryRegistry.cxx @@ -19,11 +19,13 @@ # include #endif +#include + #include #include "RTIFederateFactoryRegistry.hxx" -#include "simgear/threads/SGGuard.hxx" +#include "simgear/threads/std::lock_guard.hxx" #include "RTIFederate.hxx" namespace simgear { @@ -39,7 +41,7 @@ RTIFederateFactoryRegistry::~RTIFederateFactoryRegistry() SGSharedPtr RTIFederateFactoryRegistry::create(const std::string& name, const std::list& stringList) const { - SGGuard guard(_mutex); + std::lock_guard guard(_mutex); for (FederateFactoryList::const_iterator i = _federateFactoryList.begin(); i != _federateFactoryList.end(); ++i) { SGSharedPtr federate = (*i)->create(name, stringList); if (!federate.valid()) @@ -52,7 +54,7 @@ RTIFederateFactoryRegistry::create(const std::string& name, const std::list guard(_mutex); + std::lock_guard guard(_mutex); _federateFactoryList.push_back(factory); } diff --git a/simgear/io/HTTPClient.cxx b/simgear/io/HTTPClient.cxx index d5c32af7..f6533161 100644 --- a/simgear/io/HTTPClient.cxx +++ b/simgear/io/HTTPClient.cxx @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -48,8 +49,6 @@ #include #include #include -#include -#include #if defined( HAVE_VERSION_H ) && HAVE_VERSION_H #include "version.h" @@ -125,9 +124,9 @@ Client::Client() : setUserAgent("SimGear-" SG_STRINGIZE(SIMGEAR_VERSION)); static bool didInitCurlGlobal = false; - static SGMutex initMutex; + static std::mutex initMutex; - SGGuard g(initMutex); + std::lock_guard g(initMutex); if (!didInitCurlGlobal) { curl_global_init(CURL_GLOBAL_ALL); didInitCurlGlobal = true; diff --git a/simgear/io/raw_socket.cxx b/simgear/io/raw_socket.cxx index 1eae0c4c..94b8a441 100644 --- a/simgear/io/raw_socket.cxx +++ b/simgear/io/raw_socket.cxx @@ -36,6 +36,7 @@ #include #include #include // for snprintf +#include #include #if defined(WINSOCK) @@ -213,7 +214,7 @@ private: return ok; } - SGMutex _lock; + std::mutex _lock; SGWaitCondition _wait; typedef std::map AddressCache; diff --git a/simgear/misc/lru_cache.hxx b/simgear/misc/lru_cache.hxx index 92f85b57..13d74fb5 100644 --- a/simgear/misc/lru_cache.hxx +++ b/simgear/misc/lru_cache.hxx @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -48,7 +49,7 @@ namespace simgear class lru_cache { public: - SGMutex _mutex; + std::mutex _mutex; typedef Key key_type; typedef Value value_type; @@ -85,13 +86,13 @@ namespace simgear bool contains(const key_type &key) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); return m_map.find(key) != m_map.end(); } void insert(const key_type &key, const value_type &value) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); typename map_type::iterator i = m_map.find(key); if (i == m_map.end()) { // insert item into the cache, but first check if it is full @@ -107,7 +108,7 @@ namespace simgear } boost::optional findValue(const std::string &requiredValue) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); for (typename map_type::iterator it = m_map.begin(); it != m_map.end(); ++it) if (it->second.first == requiredValue) return it->first; @@ -115,7 +116,7 @@ namespace simgear } boost::optional get(const key_type &key) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); // lookup value in the cache typename map_type::iterator i = m_map.find(key); if (i == m_map.end()) { @@ -148,7 +149,7 @@ namespace simgear void clear() { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); m_map.clear(); m_list.clear(); } @@ -156,7 +157,7 @@ namespace simgear private: void evict() { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); // evict item from the end of most recently used list typename list_type::iterator i = --m_list.end(); m_map.erase(*i); diff --git a/simgear/scene/material/Effect.cxx b/simgear/scene/material/Effect.cxx index 89b33a75..44f15a7f 100644 --- a/simgear/scene/material/Effect.cxx +++ b/simgear/scene/material/Effect.cxx @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -79,8 +80,6 @@ #include #include #include -#include -#include namespace simgear { @@ -104,7 +103,7 @@ ref_ptr UniformFactoryImpl::getUniform( Effect * effect, SGConstPropertyNode_ptr valProp, const SGReaderWriterOptions* options ) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); std::string val = "0"; if (valProp->nChildren() == 0) { @@ -191,7 +190,7 @@ void UniformFactoryImpl::addListener(DeferredPropertyListener* listener) void UniformFactoryImpl::updateListeners( SGPropertyNode* propRoot ) { - SGGuard scopeLock(_mutex); + std::lock_guard scopeLock(_mutex); if (deferredListenerList.empty()) return; @@ -1475,10 +1474,10 @@ void mergeSchemesFallbacks(Effect *effect, const SGReaderWriterOptions *options) // Walk the techniques property tree, building techniques and // passes. -static SGMutex realizeTechniques_lock; +static std::mutex realizeTechniques_lock; bool Effect::realizeTechniques(const SGReaderWriterOptions* options) { - SGGuard g(realizeTechniques_lock); + std::lock_guard g(realizeTechniques_lock); if (getPropertyRoot()->getBoolValue("/sim/version/compositor-support", false)) mergeSchemesFallbacks(this, options); diff --git a/simgear/scene/material/Effect.hxx b/simgear/scene/material/Effect.hxx index ba255d0a..02a83285 100644 --- a/simgear/scene/material/Effect.hxx +++ b/simgear/scene/material/Effect.hxx @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -32,7 +33,6 @@ #include #include #include -#include #include namespace osg @@ -189,7 +189,7 @@ private: static const char* vec3Names[]; static const char* vec4Names[]; - SGMutex _mutex; + std::mutex _mutex; typedef boost::tuple UniformCacheKey; typedef boost::tuple, SGPropertyChangeListener*> UniformCacheValue; diff --git a/simgear/scene/material/mat.cxx b/simgear/scene/material/mat.cxx index c8d856d7..1ebb86b1 100644 --- a/simgear/scene/material/mat.cxx +++ b/simgear/scene/material/mat.cxx @@ -31,6 +31,7 @@ #include #include #include +#include #include #include "mat.hxx" @@ -50,7 +51,6 @@ #include #include #include -#include #include #include @@ -463,7 +463,7 @@ Effect* SGMaterial::get_effect(int i) Effect* SGMaterial::get_one_effect(int texIndex) { - SGGuard g(_lock); + std::lock_guard g(_lock); if (_status.empty()) { SG_LOG( SG_GENERAL, SG_WARN, "No effect available."); return 0; @@ -475,7 +475,7 @@ Effect* SGMaterial::get_one_effect(int texIndex) Effect* SGMaterial::get_effect() { - SGGuard g(_lock); + std::lock_guard g(_lock); return get_effect(0); } diff --git a/simgear/scene/material/mat.hxx b/simgear/scene/material/mat.hxx index 8e4647d2..c48caf9b 100644 --- a/simgear/scene/material/mat.hxx +++ b/simgear/scene/material/mat.hxx @@ -31,6 +31,7 @@ #include // Standard C++ string library #include #include +#include #include "Effect.hxx" @@ -517,7 +518,7 @@ private: const SGPropertyNode* parameters; // per-material lock for entrypoints called from multiple threads - SGMutex _lock; + std::mutex _lock; //////////////////////////////////////////////////////////////////// // Internal constructors and methods. diff --git a/simgear/scene/material/matlib.cxx b/simgear/scene/material/matlib.cxx index 6d4ad5ab..0b40869d 100644 --- a/simgear/scene/material/matlib.cxx +++ b/simgear/scene/material/matlib.cxx @@ -31,6 +31,7 @@ #include #include +#include #include @@ -41,8 +42,6 @@ #include #include #include -#include -#include #include "mat.hxx" @@ -56,7 +55,7 @@ using std::string; class SGMaterialLib::MatLibPrivate { public: - SGMutex mutex; + std::mutex mutex; }; // Constructor diff --git a/simgear/scene/model/ModelRegistry.cxx b/simgear/scene/model/ModelRegistry.cxx index 0087cbbf..a1e6d32b 100644 --- a/simgear/scene/model/ModelRegistry.cxx +++ b/simgear/scene/model/ModelRegistry.cxx @@ -61,7 +61,6 @@ #include #include #include -#include #include #include "BoundingVolumeBuildVisitor.hxx" diff --git a/simgear/scene/tsync/terrasync.cxx b/simgear/scene/tsync/terrasync.cxx index a1715097..e0daf980 100644 --- a/simgear/scene/tsync/terrasync.cxx +++ b/simgear/scene/tsync/terrasync.cxx @@ -57,7 +57,6 @@ #include #include #include -#include #include #include @@ -242,31 +241,31 @@ public: bool isIdle() { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); return !_state._busy; } bool isRunning() { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); return _running; } bool isStalled() { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); return _state._stalled; } bool hasServer() { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); return _state._hasServer; } bool hasServer( bool flag ) { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); return (_state._hasServer = flag); } @@ -309,14 +308,14 @@ public: void setAllowedErrorCount(int errors) { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _state._allowed_errors = errors; } void setCachePath(const SGPath& p) {_persistentCachePath = p;} void setCacheHits(unsigned int hits) { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _state._cache_hits = hits; } @@ -324,7 +323,7 @@ public: { TerrasyncThreadState st; { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); st = _state; } return st; @@ -332,7 +331,7 @@ public: private: void incrementCacheHits() { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _state._cache_hits++; } @@ -371,7 +370,7 @@ private: string _dnsdn; TerrasyncThreadState _state; - SGMutex _stateLock; + std::mutex _stateLock; }; SGTerraSync::WorkerThread::WorkerThread() : @@ -392,7 +391,7 @@ void SGTerraSync::WorkerThread::stop() // set stop flag and wake up the thread with an empty request { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _stop = true; } @@ -520,7 +519,7 @@ bool SGTerraSync::WorkerThread::findServer() void SGTerraSync::WorkerThread::run() { { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _running = true; } @@ -529,7 +528,7 @@ void SGTerraSync::WorkerThread::run() runInternal(); { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _running = false; } } @@ -640,7 +639,7 @@ void SGTerraSync::WorkerThread::runInternal() } { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _state._transfer_rate = _http.transferRateBytesPerSec(); // convert from bytes to kbytes _state._total_kb_downloaded = static_cast(_http.totalBytesDownloaded() / 1024); @@ -676,7 +675,7 @@ void SGTerraSync::WorkerThread::runInternal() } { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); _state._totalKbPending = newPendingCount; // approximately atomic update _state._busy = anySlotBusy; } @@ -714,7 +713,7 @@ SyncItem::Status SGTerraSync::WorkerThread::isPathCached(const SyncItem& next) c void SGTerraSync::WorkerThread::fail(SyncItem failedItem) { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); time_t now = time(0); _state._consecutive_errors++; _state._fail_count++; @@ -742,7 +741,7 @@ void SGTerraSync::WorkerThread::notFound(SyncItem item) void SGTerraSync::WorkerThread::updated(SyncItem item, bool isNewDirectory) { { - SGGuard g(_stateLock); + std::lock_guard g(_stateLock); time_t now = time(0); _state._consecutive_errors = 0; _state._success_count++; diff --git a/simgear/structure/SGAtomic.cxx b/simgear/structure/SGAtomic.cxx index 71f24ed9..6d069df1 100644 --- a/simgear/structure/SGAtomic.cxx +++ b/simgear/structure/SGAtomic.cxx @@ -31,7 +31,7 @@ #elif defined(GCC_ATOMIC_BUILTINS_FOUND) #elif defined(__GNUC__) && defined(__i386__) #elif defined(SGATOMIC_USE_MUTEX) -# include +# include #else # error #endif @@ -52,7 +52,7 @@ SGAtomic::operator++() : "memory"); return result + 1; #else - SGGuard lock(mMutex); + std::lock_guard lock(mMutex); return ++mValue; #endif } @@ -73,7 +73,7 @@ SGAtomic::operator--() : "memory"); return result - 1; #else - SGGuard lock(mMutex); + std::lock_guard lock(mMutex); return --mValue; #endif } @@ -89,7 +89,7 @@ SGAtomic::operator unsigned() const __asm__ __volatile__("": : : "memory"); return mValue; #else - SGGuard lock(mMutex); + std::lock_guard lock(mMutex); return mValue; #endif } @@ -111,7 +111,7 @@ SGAtomic::compareAndExchange(unsigned oldValue, unsigned newValue) : "memory"); return before == oldValue; #else - SGGuard lock(mMutex); + std::lock_guard lock(mMutex); if (mValue != oldValue) return false; mValue = newValue; diff --git a/simgear/structure/SGAtomic.hxx b/simgear/structure/SGAtomic.hxx index 493c8d25..64f5e3a5 100644 --- a/simgear/structure/SGAtomic.hxx +++ b/simgear/structure/SGAtomic.hxx @@ -111,7 +111,7 @@ private: SGAtomic& operator=(const SGAtomic&); #if defined(SGATOMIC_USE_MUTEX) - mutable SGMutex mMutex; + mutable std::mutex mMutex; #endif unsigned mValue; }; diff --git a/simgear/structure/StringTable.cxx b/simgear/structure/StringTable.cxx index 5dba1848..0a2da859 100644 --- a/simgear/structure/StringTable.cxx +++ b/simgear/structure/StringTable.cxx @@ -1,6 +1,5 @@ #include "StringTable.hxx" -#include namespace simgear { @@ -8,7 +7,7 @@ using namespace std; const string* StringTable::insert(const string& str) { - SGGuard lock(_mutex); + std::lock_guard lock(_mutex); StringContainer::iterator it = _strings.insert(str).first; return &*it; } diff --git a/simgear/structure/StringTable.hxx b/simgear/structure/StringTable.hxx index 8f5d010a..7517766d 100644 --- a/simgear/structure/StringTable.hxx +++ b/simgear/structure/StringTable.hxx @@ -2,8 +2,8 @@ #define SIMGEAR_STRINGTABLE_HXX 1 #include +#include -#include #include #include #include @@ -21,7 +21,7 @@ class StringTable { const std::string* insert(const std::string& str); private: - SGMutex _mutex; + std::mutex _mutex; StringContainer _strings; }; } diff --git a/simgear/structure/commands.cxx b/simgear/structure/commands.cxx index 5621c8d8..056b9950 100644 --- a/simgear/structure/commands.cxx +++ b/simgear/structure/commands.cxx @@ -8,6 +8,7 @@ #include #include +#include #include @@ -15,7 +16,6 @@ #include #include -#include #include struct Invocation @@ -32,7 +32,7 @@ public: using InvocactionVec = std::vector; InvocactionVec _queue; - SGMutex _mutex; + std::mutex _mutex; using command_map = std::map ; command_map _commands; @@ -179,7 +179,7 @@ void SGCommandMgr::queuedExecute(const std::string &name, const SGPropertyNode* { Invocation invoke = {name, new SGPropertyNode}; copyProperties(arg, invoke.args); - SGGuard g(d->_mutex); + std::lock_guard g(d->_mutex); d->_queue.push_back(invoke); } @@ -192,7 +192,7 @@ void SGCommandMgr::executedQueuedCommands() // locked swap with the shared queue Private::InvocactionVec q; { - SGGuard g(d->_mutex); + std::lock_guard g(d->_mutex); d->_queue.swap(q); } diff --git a/simgear/threads/CMakeLists.txt b/simgear/threads/CMakeLists.txt index 5258be8f..54ec3a7c 100644 --- a/simgear/threads/CMakeLists.txt +++ b/simgear/threads/CMakeLists.txt @@ -1,7 +1,6 @@ include (SimGearComponent) set(HEADERS - SGGuard.hxx SGQueue.hxx SGThread.hxx) diff --git a/simgear/threads/SGGuard.hxx b/simgear/threads/SGGuard.hxx deleted file mode 100644 index 93d664a2..00000000 --- a/simgear/threads/SGGuard.hxx +++ /dev/null @@ -1,35 +0,0 @@ -#ifndef SGGUARD_HXX_INCLUDED -#define SGGUARD_HXX_INCLUDED 1 - -/** - * A scoped locking utility. - * An SGGuard object locks its synchronization object during creation and - * automatically unlocks it when it goes out of scope. - */ -template -class SGGuard -{ -public: - - /** - * Create an SGGuard object and lock the passed lockable object. - * @param l A lockable object. - */ - inline SGGuard( SGLOCK& l ) : lock(l) { lock.lock(); } - - /** - * Destroy this object and unlock the lockable object. - */ - inline ~SGGuard() { lock.unlock(); } - -private: - - SGLOCK& lock; //!< A lockable object - -private: - // Disable copying. - SGGuard(const SGLOCK&); - SGLOCK& operator= (const SGLOCK&); -}; - -#endif // SGGUARD_HXX_INCLUDED diff --git a/simgear/threads/SGQueue.hxx b/simgear/threads/SGQueue.hxx index 41c2cf30..54836623 100644 --- a/simgear/threads/SGQueue.hxx +++ b/simgear/threads/SGQueue.hxx @@ -5,7 +5,7 @@ #include #include -#include "SGGuard.hxx" +#include #include "SGThread.hxx" /** @@ -94,7 +94,7 @@ public: * @return True if queue is empty, otherwise false. */ virtual bool empty() { - SGGuard g(mutex); + std::lock_guard g(mutex); return this->fifo.empty(); } @@ -104,7 +104,7 @@ public: * @param item object to add. */ virtual void push( const T& item ) { - SGGuard g(mutex); + std::lock_guard g(mutex); this->fifo.push( item ); } @@ -114,7 +114,7 @@ public: * @return The next available object. */ virtual T front() { - SGGuard g(mutex); + std::lock_guard g(mutex); assert( ! this->fifo.empty() ); T item = this->fifo.front(); return item; @@ -126,7 +126,7 @@ public: * @return The next available object. */ virtual T pop() { - SGGuard g(mutex); + std::lock_guard g(mutex); if (this->fifo.empty()) return T(); // assumes T is default constructable // if (fifo.empty()) @@ -145,7 +145,7 @@ public: * @return Size of queue. */ virtual size_t size() { - SGGuard g(mutex); + std::lock_guard g(mutex); return this->fifo.size(); } @@ -154,7 +154,7 @@ private: /** * Mutex to serialise access. */ - SGMutex mutex; + std::mutex mutex; private: // Prevent copying. @@ -184,7 +184,7 @@ public: * */ virtual bool empty() { - SGGuard g(mutex); + std::lock_guard g(mutex); return this->fifo.empty(); } @@ -194,7 +194,7 @@ public: * @param item The object to add. */ virtual void push( const T& item ) { - SGGuard g(mutex); + std::lock_guard g(mutex); this->fifo.push( item ); not_empty.signal(); } @@ -206,7 +206,7 @@ public: * @return The next available object. */ virtual T front() { - SGGuard g(mutex); + std::lock_guard g(mutex); assert(this->fifo.empty() != true); //if (fifo.empty()) throw ?? @@ -222,7 +222,7 @@ public: * @return The next available object. */ virtual T pop() { - SGGuard g(mutex); + std::lock_guard g(mutex); while (this->fifo.empty()) not_empty.wait(mutex); @@ -241,7 +241,7 @@ public: * @return Size of queue. */ virtual size_t size() { - SGGuard g(mutex); + std::lock_guard g(mutex); return this->fifo.size(); } @@ -250,7 +250,7 @@ private: /** * Mutex to serialise access. */ - SGMutex mutex; + std::mutex mutex; /** * Condition to signal when queue not empty. @@ -286,7 +286,7 @@ public: * */ virtual void clear() { - SGGuard g(mutex); + std::lock_guard g(mutex); this->queue.clear(); } @@ -294,7 +294,7 @@ public: * */ virtual bool empty() { - SGGuard g(mutex); + std::lock_guard g(mutex); return this->queue.empty(); } @@ -304,7 +304,7 @@ public: * @param item The object to add. */ virtual void push_front( const T& item ) { - SGGuard g(mutex); + std::lock_guard g(mutex); this->queue.push_front( item ); not_empty.signal(); } @@ -315,7 +315,7 @@ public: * @param item The object to add. */ virtual void push_back( const T& item ) { - SGGuard g(mutex); + std::lock_guard g(mutex); this->queue.push_back( item ); not_empty.signal(); } @@ -327,7 +327,7 @@ public: * @return The next available object. */ virtual T front() { - SGGuard g(mutex); + std::lock_guard g(mutex); assert(this->queue.empty() != true); //if (queue.empty()) throw ?? @@ -343,7 +343,7 @@ public: * @return The next available object. */ virtual T pop_front() { - SGGuard g(mutex); + std::lock_guard g(mutex); while (this->queue.empty()) not_empty.wait(mutex); @@ -363,7 +363,7 @@ public: * @return The next available object. */ virtual T pop_back() { - SGGuard g(mutex); + std::lock_guard g(mutex); while (this->queue.empty()) not_empty.wait(mutex); @@ -382,12 +382,12 @@ public: * @return Size of queue. */ virtual size_t size() { - SGGuard g(mutex); + std::lock_guard g(mutex); return this->queue.size(); } void waitOnNotEmpty() { - SGGuard g(mutex); + std::lock_guard g(mutex); while (this->queue.empty()) not_empty.wait(mutex); } @@ -396,7 +396,7 @@ private: /** * Mutex to serialise access. */ - SGMutex mutex; + std::mutex mutex; /** * Condition to signal when queue not empty. diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index cdb16592..1a98569d 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -29,29 +29,26 @@ #include "SGThread.hxx" -#ifdef _WIN32 - -///////////////////////////////////////////////////////////////////////////// -/// win32 threads -///////////////////////////////////////////////////////////////////////////// - -#include -#include +#include +#include +#include +#include +#include struct SGThread::PrivateData { - PrivateData() : - _handle(INVALID_HANDLE_VALUE) + PrivateData() { } ~PrivateData() { - if (_handle == INVALID_HANDLE_VALUE) + // If we are still having a started thread and nobody waited, + // now detach ... + if (!_started) return; - CloseHandle(_handle); - _handle = INVALID_HANDLE_VALUE; + _thread.detach(); } - static DWORD WINAPI start_routine(LPVOID data) + static void *start_routine(void* data) { SGThread* thread = reinterpret_cast(data); thread->run(); @@ -60,55 +57,50 @@ struct SGThread::PrivateData { bool start(SGThread& thread) { - if (_handle != INVALID_HANDLE_VALUE) + if (_started) return false; - _handle = CreateThread(0, 0, start_routine, &thread, 0, 0); - if (_handle == INVALID_HANDLE_VALUE) + + try { + _thread = std::thread(start_routine, &thread); + } catch (std::runtime_error &ex) { return false; + } + + _started = true; return true; } void join() { - if (_handle == INVALID_HANDLE_VALUE) + if (!_started) return; - DWORD ret = WaitForSingleObject(_handle, INFINITE); - if (ret != WAIT_OBJECT_0) - return; - CloseHandle(_handle); - _handle = INVALID_HANDLE_VALUE; + + _thread.join(); + _started = false; } - HANDLE _handle; + std::thread _thread; + bool _started = false; }; -long SGThread::current( void ) { +long SGThread::current( void ) +{ +#ifdef _WIN32 return (long)GetCurrentThreadId(); +#else + return (long)pthread_self(); +#endif } -struct SGMutex::PrivateData { - PrivateData() - { - InitializeCriticalSection((LPCRITICAL_SECTION)&_criticalSection); - } - ~PrivateData() - { - DeleteCriticalSection((LPCRITICAL_SECTION)&_criticalSection); - } +#if _WIN32 - void lock(void) - { - EnterCriticalSection((LPCRITICAL_SECTION)&_criticalSection); - } +///////////////////////////////////////////////////////////////////////////// +/// win32 threads +///////////////////////////////////////////////////////////////////////////// - void unlock(void) - { - LeaveCriticalSection((LPCRITICAL_SECTION)&_criticalSection); - } - - CRITICAL_SECTION _criticalSection; -}; +#include +#include struct SGWaitCondition::PrivateData { ~PrivateData(void) @@ -138,7 +130,7 @@ struct SGWaitCondition::PrivateData { _mutex.unlock(); } - bool wait(SGMutex::PrivateData& externalMutex, DWORD msec) + bool wait(std::mutex& externalMutex, DWORD msec) { _mutex.lock(); if (_pool.empty()) @@ -163,13 +155,13 @@ struct SGWaitCondition::PrivateData { return result == WAIT_OBJECT_0; } - void wait(SGMutex::PrivateData& externalMutex) + void wait(std::mutex& externalMutex) { wait(externalMutex, INFINITE); } // Protect the list of waiters - SGMutex::PrivateData _mutex; + std::mutex _mutex; std::list _waiters; std::list _pool; @@ -185,89 +177,6 @@ struct SGWaitCondition::PrivateData { #include #include -struct SGThread::PrivateData { - PrivateData() : - _started(false) - { - } - ~PrivateData() - { - // If we are still having a started thread and nobody waited, - // now detach ... - if (!_started) - return; - pthread_detach(_thread); - } - - static void *start_routine(void* data) - { - SGThread* thread = reinterpret_cast(data); - thread->run(); - return 0; - } - - bool start(SGThread& thread) - { - if (_started) - return false; - - int ret = pthread_create(&_thread, 0, start_routine, &thread); - if (0 != ret) - return false; - - _started = true; - return true; - } - - void join() - { - if (!_started) - return; - - pthread_join(_thread, 0); - _started = false; - } - - pthread_t _thread; - bool _started; -}; - -long SGThread::current( void ) { - return (long)pthread_self(); -} - -struct SGMutex::PrivateData { - PrivateData() - { - int err = pthread_mutex_init(&_mutex, 0); - assert(err == 0); - (void)err; - } - - ~PrivateData() - { - int err = pthread_mutex_destroy(&_mutex); - assert(err == 0); - (void)err; - } - - void lock(void) - { - int err = pthread_mutex_lock(&_mutex); - assert(err == 0); - (void)err; - } - - void unlock(void) - { - int err = pthread_mutex_unlock(&_mutex); - assert(err == 0); - (void)err; - } - - pthread_mutex_t _mutex; -}; - struct SGWaitCondition::PrivateData { PrivateData(void) { @@ -296,14 +205,14 @@ struct SGWaitCondition::PrivateData { (void)err; } - void wait(SGMutex::PrivateData& mutex) + void wait(std::mutex& mutex) { - int err = pthread_cond_wait(&_condition, &mutex._mutex); + int err = pthread_cond_wait(&_condition, mutex.native_handle()); assert(err == 0); (void)err; } - bool wait(SGMutex::PrivateData& mutex, unsigned msec) + bool wait(std::mutex& mutex, unsigned msec) { struct timespec ts; #ifdef HAVE_CLOCK_GETTIME @@ -324,7 +233,7 @@ struct SGWaitCondition::PrivateData { } ts.tv_sec += msec / 1000; - int evalue = pthread_cond_timedwait(&_condition, &mutex._mutex, &ts); + int evalue = pthread_cond_timedwait(&_condition, mutex.native_handle(), &ts); if (evalue == 0) return true; @@ -360,29 +269,6 @@ SGThread::join() _privateData->join(); } -SGMutex::SGMutex() : - _privateData(new PrivateData) -{ -} - -SGMutex::~SGMutex() -{ - delete _privateData; - _privateData = 0; -} - -void -SGMutex::lock() -{ - _privateData->lock(); -} - -void -SGMutex::unlock() -{ - _privateData->unlock(); -} - SGWaitCondition::SGWaitCondition() : _privateData(new PrivateData) { @@ -395,15 +281,15 @@ SGWaitCondition::~SGWaitCondition() } void -SGWaitCondition::wait(SGMutex& mutex) +SGWaitCondition::wait(std::mutex& mutex) { - _privateData->wait(*mutex._privateData); + _privateData->wait(mutex); } bool -SGWaitCondition::wait(SGMutex& mutex, unsigned msec) +SGWaitCondition::wait(std::mutex& mutex, unsigned msec) { - return _privateData->wait(*mutex._privateData, msec); + return _privateData->wait(mutex, msec); } void diff --git a/simgear/threads/SGThread.hxx b/simgear/threads/SGThread.hxx index e5610f4b..ea1fa746 100644 --- a/simgear/threads/SGThread.hxx +++ b/simgear/threads/SGThread.hxx @@ -86,49 +86,6 @@ private: class SGWaitCondition; -/** - * A mutex is used to protect a section of code such that at any time - * only a single thread can execute the code. - */ -class SGMutex { -public: - /** - * Create a new mutex. - * Under Linux this is a 'fast' mutex. - */ - SGMutex(); - - /** - * Destroy a mutex object. - * Note: it is the responsibility of the caller to ensure the mutex is - * unlocked before destruction occurs. - */ - ~SGMutex(); - - /** - * Lock this mutex. - * If the mutex is currently unlocked, it becomes locked and owned by - * the calling thread. If the mutex is already locked by another thread, - * the calling thread is suspended until the mutex is unlocked. If the - * mutex is already locked and owned by the calling thread, the calling - * thread is suspended until the mutex is unlocked, effectively causing - * the calling thread to deadlock. - */ - void lock(); - - /** - * Unlock this mutex. - * It is assumed that the mutex is locked and owned by the calling thread. - */ - void unlock(); - -private: - struct PrivateData; - PrivateData* _privateData; - - friend class SGWaitCondition; -}; - /** * A condition variable is a synchronization device that allows threads to * suspend execution until some predicate on shared data is satisfied. @@ -152,7 +109,7 @@ public: * * @param mutex Reference to a locked mutex. */ - void wait(SGMutex& mutex); + void wait(std::mutex& mutex); /** * Wait for this condition variable to be signaled for at most \a 'msec' @@ -163,7 +120,7 @@ public: * * @return */ - bool wait(SGMutex& mutex, unsigned msec); + bool wait(std::mutex& mutex, unsigned msec); /** * Wake one thread waiting on this condition variable. From 9f8e6c06347d45a1a208651a81ca10445062af06 Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Thu, 23 Jan 2020 10:38:52 +0100 Subject: [PATCH 03/10] Make backwards compatible stubs so only a recompile without any modification is required. --- simgear/threads/CMakeLists.txt | 1 + simgear/threads/SGGuard.hxx | 5 +++++ simgear/threads/SGThread.hxx | 5 +++++ 3 files changed, 11 insertions(+) create mode 100644 simgear/threads/SGGuard.hxx diff --git a/simgear/threads/CMakeLists.txt b/simgear/threads/CMakeLists.txt index 54ec3a7c..5258be8f 100644 --- a/simgear/threads/CMakeLists.txt +++ b/simgear/threads/CMakeLists.txt @@ -1,6 +1,7 @@ include (SimGearComponent) set(HEADERS + SGGuard.hxx SGQueue.hxx SGThread.hxx) diff --git a/simgear/threads/SGGuard.hxx b/simgear/threads/SGGuard.hxx new file mode 100644 index 00000000..1dc4bd2d --- /dev/null +++ b/simgear/threads/SGGuard.hxx @@ -0,0 +1,5 @@ +#pragma once +#pragma message(": warning use std::lock_guard and std::mutex.") + +// backwards compatibility, just needs a recompile +#define SGGuard std::lock_guard diff --git a/simgear/threads/SGThread.hxx b/simgear/threads/SGThread.hxx index ea1fa746..ba9f52d2 100644 --- a/simgear/threads/SGThread.hxx +++ b/simgear/threads/SGThread.hxx @@ -29,6 +29,11 @@ #include #include + +// backwards compatibility, just needs a recompile +#define SGMutex std::mutex + + /** * Encapsulate generic threading methods. * Users derive a class from SGThread and implement the run() member function. From c3ae70461061a9546a430147a4aabc8960845d19 Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Thu, 23 Jan 2020 13:47:04 +0100 Subject: [PATCH 04/10] Move SGThread::current to after the system dependent code which includes the proper header files --- simgear/threads/SGThread.cxx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index 1a98569d..6123966d 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -83,16 +83,6 @@ struct SGThread::PrivateData { bool _started = false; }; -long SGThread::current( void ) -{ -#ifdef _WIN32 - return (long)GetCurrentThreadId(); -#else - return (long)pthread_self(); -#endif -} - - #if _WIN32 ///////////////////////////////////////////////////////////////////////////// @@ -269,6 +259,16 @@ SGThread::join() _privateData->join(); } +long SGThread::current( void ) +{ +#ifdef _WIN32 + return (long)GetCurrentThreadId(); +#else + return (long)pthread_self(); +#endif +} + + SGWaitCondition::SGWaitCondition() : _privateData(new PrivateData) { From 1c7fd67c392e01345185e8790a43bc6647aa7385 Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Fri, 24 Jan 2020 10:48:53 +0100 Subject: [PATCH 05/10] Convert condition variables to C++11 too. --- simgear/threads/SGThread.cxx | 232 ++++++++++++----------------------- 1 file changed, 79 insertions(+), 153 deletions(-) diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index 6123966d..7da62bb7 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -35,6 +35,85 @@ #include #include +#if _WIN32 +# include +# include +#else +# include +# include +# include +# include +#endif + +struct SGWaitCondition::PrivateData { + PrivateData(void) + { + } + ~PrivateData(void) + { + } + + void signal(void) + { + bool waiter; + { + std::lock_guard lock(_mutex); + waiter = !ready; + ready = true; + } + if (waiter) { + _condition.notify_one(); + } + } + + void broadcast(void) + { + bool waiter; + { + std::lock_guard lock(_mutex); + waiter = !ready; + ready = true; + } + if (waiter) { + _condition.notify_all(); + } + } + + void wait(std::mutex& mutex) + { + std::unique_lock lock(_mutex); + + mutex.unlock(); + _condition.wait(lock, [this]{ return ready; } ); + mutex.lock(); + + ready = false; + } + + bool wait(std::mutex& mutex, unsigned msec) + { + auto timeout = std::chrono::milliseconds(msec); + std::unique_lock lock(mutex); + + try { + mutex.unlock(); + _condition.wait_for(lock, timeout, [this]{ return ready; } ); + mutex.lock(); + + ready = false; + } catch (std::runtime_error &ex) { + return false; + } + return true; + } + + std::mutex _mutex; + std::condition_variable _condition; + +private: + bool ready = false; +}; + struct SGThread::PrivateData { PrivateData() { @@ -83,159 +162,6 @@ struct SGThread::PrivateData { bool _started = false; }; -#if _WIN32 - -///////////////////////////////////////////////////////////////////////////// -/// win32 threads -///////////////////////////////////////////////////////////////////////////// - -#include -#include - -struct SGWaitCondition::PrivateData { - ~PrivateData(void) - { - // The waiters list should be empty anyway - _mutex.lock(); - while (!_pool.empty()) { - CloseHandle(_pool.front()); - _pool.pop_front(); - } - _mutex.unlock(); - } - - void signal(void) - { - _mutex.lock(); - if (!_waiters.empty()) - SetEvent(_waiters.back()); - _mutex.unlock(); - } - - void broadcast(void) - { - _mutex.lock(); - for (std::list::iterator i = _waiters.begin(); i != _waiters.end(); ++i) - SetEvent(*i); - _mutex.unlock(); - } - - bool wait(std::mutex& externalMutex, DWORD msec) - { - _mutex.lock(); - if (_pool.empty()) - _waiters.push_front(CreateEvent(NULL, FALSE, FALSE, NULL)); - else - _waiters.splice(_waiters.begin(), _pool, _pool.begin()); - std::list::iterator i = _waiters.begin(); - _mutex.unlock(); - - externalMutex.unlock(); - - DWORD result = WaitForSingleObject(*i, msec); - - externalMutex.lock(); - - _mutex.lock(); - if (result != WAIT_OBJECT_0) - result = WaitForSingleObject(*i, 0); - _pool.splice(_pool.begin(), _waiters, i); - _mutex.unlock(); - - return result == WAIT_OBJECT_0; - } - - void wait(std::mutex& externalMutex) - { - wait(externalMutex, INFINITE); - } - - // Protect the list of waiters - std::mutex _mutex; - - std::list _waiters; - std::list _pool; -}; - -#else -///////////////////////////////////////////////////////////////////////////// -/// posix threads -///////////////////////////////////////////////////////////////////////////// - -#include -#include -#include -#include - -struct SGWaitCondition::PrivateData { - PrivateData(void) - { - int err = pthread_cond_init(&_condition, NULL); - assert(err == 0); - (void)err; - } - ~PrivateData(void) - { - int err = pthread_cond_destroy(&_condition); - assert(err == 0); - (void)err; - } - - void signal(void) - { - int err = pthread_cond_signal(&_condition); - assert(err == 0); - (void)err; - } - - void broadcast(void) - { - int err = pthread_cond_broadcast(&_condition); - assert(err == 0); - (void)err; - } - - void wait(std::mutex& mutex) - { - int err = pthread_cond_wait(&_condition, mutex.native_handle()); - assert(err == 0); - (void)err; - } - - bool wait(std::mutex& mutex, unsigned msec) - { - struct timespec ts; -#ifdef HAVE_CLOCK_GETTIME - if (0 != clock_gettime(CLOCK_REALTIME, &ts)) - return false; -#else - struct timeval tv; - if (0 != gettimeofday(&tv, NULL)) - return false; - ts.tv_sec = tv.tv_sec; - ts.tv_nsec = tv.tv_usec * 1000; -#endif - - ts.tv_nsec += 1000000*(msec % 1000); - if (1000000000 <= ts.tv_nsec) { - ts.tv_nsec -= 1000000000; - ts.tv_sec += 1; - } - ts.tv_sec += msec / 1000; - - int evalue = pthread_cond_timedwait(&_condition, mutex.native_handle(), &ts); - if (evalue == 0) - return true; - - assert(evalue == ETIMEDOUT); - return false; - } - - pthread_cond_t _condition; -}; - -#endif - SGThread::SGThread() : _privateData(new PrivateData) { From 308903dc9185223e015333c674b419b1ef45cb15 Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Fri, 24 Jan 2020 13:52:37 +0100 Subject: [PATCH 06/10] Revert to c3ae70461061a9546a430147a4aabc8960845d19 until the condition_variable problem is solved --- simgear/threads/SGThread.cxx | 232 +++++++++++++++++++++++------------ 1 file changed, 153 insertions(+), 79 deletions(-) diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index 7da62bb7..6123966d 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -35,85 +35,6 @@ #include #include -#if _WIN32 -# include -# include -#else -# include -# include -# include -# include -#endif - -struct SGWaitCondition::PrivateData { - PrivateData(void) - { - } - ~PrivateData(void) - { - } - - void signal(void) - { - bool waiter; - { - std::lock_guard lock(_mutex); - waiter = !ready; - ready = true; - } - if (waiter) { - _condition.notify_one(); - } - } - - void broadcast(void) - { - bool waiter; - { - std::lock_guard lock(_mutex); - waiter = !ready; - ready = true; - } - if (waiter) { - _condition.notify_all(); - } - } - - void wait(std::mutex& mutex) - { - std::unique_lock lock(_mutex); - - mutex.unlock(); - _condition.wait(lock, [this]{ return ready; } ); - mutex.lock(); - - ready = false; - } - - bool wait(std::mutex& mutex, unsigned msec) - { - auto timeout = std::chrono::milliseconds(msec); - std::unique_lock lock(mutex); - - try { - mutex.unlock(); - _condition.wait_for(lock, timeout, [this]{ return ready; } ); - mutex.lock(); - - ready = false; - } catch (std::runtime_error &ex) { - return false; - } - return true; - } - - std::mutex _mutex; - std::condition_variable _condition; - -private: - bool ready = false; -}; - struct SGThread::PrivateData { PrivateData() { @@ -162,6 +83,159 @@ struct SGThread::PrivateData { bool _started = false; }; +#if _WIN32 + +///////////////////////////////////////////////////////////////////////////// +/// win32 threads +///////////////////////////////////////////////////////////////////////////// + +#include +#include + +struct SGWaitCondition::PrivateData { + ~PrivateData(void) + { + // The waiters list should be empty anyway + _mutex.lock(); + while (!_pool.empty()) { + CloseHandle(_pool.front()); + _pool.pop_front(); + } + _mutex.unlock(); + } + + void signal(void) + { + _mutex.lock(); + if (!_waiters.empty()) + SetEvent(_waiters.back()); + _mutex.unlock(); + } + + void broadcast(void) + { + _mutex.lock(); + for (std::list::iterator i = _waiters.begin(); i != _waiters.end(); ++i) + SetEvent(*i); + _mutex.unlock(); + } + + bool wait(std::mutex& externalMutex, DWORD msec) + { + _mutex.lock(); + if (_pool.empty()) + _waiters.push_front(CreateEvent(NULL, FALSE, FALSE, NULL)); + else + _waiters.splice(_waiters.begin(), _pool, _pool.begin()); + std::list::iterator i = _waiters.begin(); + _mutex.unlock(); + + externalMutex.unlock(); + + DWORD result = WaitForSingleObject(*i, msec); + + externalMutex.lock(); + + _mutex.lock(); + if (result != WAIT_OBJECT_0) + result = WaitForSingleObject(*i, 0); + _pool.splice(_pool.begin(), _waiters, i); + _mutex.unlock(); + + return result == WAIT_OBJECT_0; + } + + void wait(std::mutex& externalMutex) + { + wait(externalMutex, INFINITE); + } + + // Protect the list of waiters + std::mutex _mutex; + + std::list _waiters; + std::list _pool; +}; + +#else +///////////////////////////////////////////////////////////////////////////// +/// posix threads +///////////////////////////////////////////////////////////////////////////// + +#include +#include +#include +#include + +struct SGWaitCondition::PrivateData { + PrivateData(void) + { + int err = pthread_cond_init(&_condition, NULL); + assert(err == 0); + (void)err; + } + ~PrivateData(void) + { + int err = pthread_cond_destroy(&_condition); + assert(err == 0); + (void)err; + } + + void signal(void) + { + int err = pthread_cond_signal(&_condition); + assert(err == 0); + (void)err; + } + + void broadcast(void) + { + int err = pthread_cond_broadcast(&_condition); + assert(err == 0); + (void)err; + } + + void wait(std::mutex& mutex) + { + int err = pthread_cond_wait(&_condition, mutex.native_handle()); + assert(err == 0); + (void)err; + } + + bool wait(std::mutex& mutex, unsigned msec) + { + struct timespec ts; +#ifdef HAVE_CLOCK_GETTIME + if (0 != clock_gettime(CLOCK_REALTIME, &ts)) + return false; +#else + struct timeval tv; + if (0 != gettimeofday(&tv, NULL)) + return false; + ts.tv_sec = tv.tv_sec; + ts.tv_nsec = tv.tv_usec * 1000; +#endif + + ts.tv_nsec += 1000000*(msec % 1000); + if (1000000000 <= ts.tv_nsec) { + ts.tv_nsec -= 1000000000; + ts.tv_sec += 1; + } + ts.tv_sec += msec / 1000; + + int evalue = pthread_cond_timedwait(&_condition, mutex.native_handle(), &ts); + if (evalue == 0) + return true; + + assert(evalue == ETIMEDOUT); + return false; + } + + pthread_cond_t _condition; +}; + +#endif + SGThread::SGThread() : _privateData(new PrivateData) { From b0580b2df5dabdc344150325d40431a99ebd73cd Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Fri, 24 Jan 2020 15:46:36 +0100 Subject: [PATCH 07/10] Re-order the mutex locking of the condition-mutex and the caller-mutex, add noxecept. Also add some debugging code to detect multiple locks of the same mutex. --- simgear/threads/SGThread.cxx | 257 ++++++++++++++--------------------- 1 file changed, 104 insertions(+), 153 deletions(-) diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index 6123966d..cbf1312e 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -35,6 +35,110 @@ #include #include +#if _WIN32 +# include +# include +#else +# include +# include +# include +# include +#endif + +struct SGWaitCondition::PrivateData { + PrivateData(void) + { + } + ~PrivateData(void) + { + } + + void signal(void) + { + bool waiting; + + { + std::lock_guard lock(_mtx); + waiting = !ready; + ready = true; + } + + if (waiting) { + _condition.notify_one(); + } + } + + void broadcast(void) + { + bool waiting; + + { + std::lock_guard lock(_mtx); + waiting = !ready; + ready = true; + } + + if (waiting) { + _condition.notify_all(); + } + } + + void wait(std::mutex& mutex) noexcept + { + mutex.unlock(); + + { + std::unique_lock lock(_mtx); + _condition.wait(lock, [this]{ return ready; } ); + ready = false; + } + +#ifndef NDEBUG +# if _WIN32 + native_handle_type *m = mutex.native_handle(); + assert(m->LockCount == 0); +# else + pthread_mutex_t *m = mutex.native_handle(); + assert(m->__data.__count == 0); +# endif +#endif + mutex.lock(); + } + + bool wait(std::mutex& mutex, unsigned msec) noexcept + { + auto timeout = std::chrono::milliseconds(msec); + + mutex.unlock(); + + { + std::unique_lock lock(_mtx); + _condition.wait_for(lock, timeout, [this]{ return ready; } ); + ready = false; + } + +#ifndef NDEBUG +# if _WIN32 + native_handle_type *m = mutex.native_handle(); + assert(m->LockCount == 0); +# else + pthread_mutex_t *m = mutex.native_handle(); + assert(m->__data.__count == 0); +# endif +#endif + mutex.lock(); + + return true; + } + + std::mutex _mtx; + std::condition_variable _condition; + std::atomic m_holder; + +private: + bool ready = false; +}; + struct SGThread::PrivateData { PrivateData() { @@ -83,159 +187,6 @@ struct SGThread::PrivateData { bool _started = false; }; -#if _WIN32 - -///////////////////////////////////////////////////////////////////////////// -/// win32 threads -///////////////////////////////////////////////////////////////////////////// - -#include -#include - -struct SGWaitCondition::PrivateData { - ~PrivateData(void) - { - // The waiters list should be empty anyway - _mutex.lock(); - while (!_pool.empty()) { - CloseHandle(_pool.front()); - _pool.pop_front(); - } - _mutex.unlock(); - } - - void signal(void) - { - _mutex.lock(); - if (!_waiters.empty()) - SetEvent(_waiters.back()); - _mutex.unlock(); - } - - void broadcast(void) - { - _mutex.lock(); - for (std::list::iterator i = _waiters.begin(); i != _waiters.end(); ++i) - SetEvent(*i); - _mutex.unlock(); - } - - bool wait(std::mutex& externalMutex, DWORD msec) - { - _mutex.lock(); - if (_pool.empty()) - _waiters.push_front(CreateEvent(NULL, FALSE, FALSE, NULL)); - else - _waiters.splice(_waiters.begin(), _pool, _pool.begin()); - std::list::iterator i = _waiters.begin(); - _mutex.unlock(); - - externalMutex.unlock(); - - DWORD result = WaitForSingleObject(*i, msec); - - externalMutex.lock(); - - _mutex.lock(); - if (result != WAIT_OBJECT_0) - result = WaitForSingleObject(*i, 0); - _pool.splice(_pool.begin(), _waiters, i); - _mutex.unlock(); - - return result == WAIT_OBJECT_0; - } - - void wait(std::mutex& externalMutex) - { - wait(externalMutex, INFINITE); - } - - // Protect the list of waiters - std::mutex _mutex; - - std::list _waiters; - std::list _pool; -}; - -#else -///////////////////////////////////////////////////////////////////////////// -/// posix threads -///////////////////////////////////////////////////////////////////////////// - -#include -#include -#include -#include - -struct SGWaitCondition::PrivateData { - PrivateData(void) - { - int err = pthread_cond_init(&_condition, NULL); - assert(err == 0); - (void)err; - } - ~PrivateData(void) - { - int err = pthread_cond_destroy(&_condition); - assert(err == 0); - (void)err; - } - - void signal(void) - { - int err = pthread_cond_signal(&_condition); - assert(err == 0); - (void)err; - } - - void broadcast(void) - { - int err = pthread_cond_broadcast(&_condition); - assert(err == 0); - (void)err; - } - - void wait(std::mutex& mutex) - { - int err = pthread_cond_wait(&_condition, mutex.native_handle()); - assert(err == 0); - (void)err; - } - - bool wait(std::mutex& mutex, unsigned msec) - { - struct timespec ts; -#ifdef HAVE_CLOCK_GETTIME - if (0 != clock_gettime(CLOCK_REALTIME, &ts)) - return false; -#else - struct timeval tv; - if (0 != gettimeofday(&tv, NULL)) - return false; - ts.tv_sec = tv.tv_sec; - ts.tv_nsec = tv.tv_usec * 1000; -#endif - - ts.tv_nsec += 1000000*(msec % 1000); - if (1000000000 <= ts.tv_nsec) { - ts.tv_nsec -= 1000000000; - ts.tv_sec += 1; - } - ts.tv_sec += msec / 1000; - - int evalue = pthread_cond_timedwait(&_condition, mutex.native_handle(), &ts); - if (evalue == 0) - return true; - - assert(evalue == ETIMEDOUT); - return false; - } - - pthread_cond_t _condition; -}; - -#endif - SGThread::SGThread() : _privateData(new PrivateData) { From faf43d2c4b6cb73669c9759c9d5a7e919e3af390 Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Fri, 24 Jan 2020 19:30:32 +0100 Subject: [PATCH 08/10] Comment out Windows debugging code, for now --- simgear/threads/SGThread.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index cbf1312e..4c618b99 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -95,8 +95,8 @@ struct SGWaitCondition::PrivateData { #ifndef NDEBUG # if _WIN32 - native_handle_type *m = mutex.native_handle(); - assert(m->LockCount == 0); +// native_handle_type *m = mutex.native_handle(); +// assert(m->LockCount == 0); # else pthread_mutex_t *m = mutex.native_handle(); assert(m->__data.__count == 0); @@ -119,8 +119,8 @@ struct SGWaitCondition::PrivateData { #ifndef NDEBUG # if _WIN32 - native_handle_type *m = mutex.native_handle(); - assert(m->LockCount == 0); +// native_handle_type *m = mutex.native_handle(); +// assert(m->LockCount == 0); # else pthread_mutex_t *m = mutex.native_handle(); assert(m->__data.__count == 0); From 9f862dcc0e614899eb7249f2961174bd36c92c3c Mon Sep 17 00:00:00 2001 From: Stuart Buchanan Date: Mon, 27 Jan 2020 09:28:25 +0000 Subject: [PATCH 09/10] Fix directional lights for AMD Some AMD drivers do not like triangles for point sprites, which breaks directional lights. This works around this by allowing users to set /rendering/triangle-directional-lights=false which falls back to the non-directional implementation of a point. --- simgear/scene/tgdb/pt_lights.cxx | 84 ++++++++++++++++---------- simgear/scene/tgdb/pt_lights.hxx | 6 +- simgear/scene/util/SGSceneFeatures.cxx | 8 +-- simgear/scene/util/SGSceneFeatures.hxx | 17 ++++++ 4 files changed, 77 insertions(+), 38 deletions(-) diff --git a/simgear/scene/tgdb/pt_lights.cxx b/simgear/scene/tgdb/pt_lights.cxx index 11ba4539..cad28567 100644 --- a/simgear/scene/tgdb/pt_lights.cxx +++ b/simgear/scene/tgdb/pt_lights.cxx @@ -153,41 +153,49 @@ SGLightFactory::getLightDrawable(const SGLightBin::Light& light) } osg::Drawable* -SGLightFactory::getLightDrawable(const SGDirectionalLightBin::Light& light) +SGLightFactory::getLightDrawable(const SGDirectionalLightBin::Light& light, bool useTriangles) { osg::Vec3Array* vertices = new osg::Vec3Array; osg::Vec4Array* colors = new osg::Vec4Array; - SGVec4f visibleColor(light.color); - SGVec4f invisibleColor(visibleColor[0], visibleColor[1], - visibleColor[2], 0); - SGVec3f normal = normalize(light.normal); - SGVec3f perp1 = perpendicular(normal); - SGVec3f perp2 = cross(normal, perp1); - SGVec3f position = light.position; - vertices->push_back(toOsg(position)); - vertices->push_back(toOsg(position + perp1)); - vertices->push_back(toOsg(position + perp2)); - colors->push_back(toOsg(visibleColor)); - colors->push_back(toOsg(invisibleColor)); - colors->push_back(toOsg(invisibleColor)); + if (useTriangles) { + SGVec4f visibleColor(light.color); + SGVec4f invisibleColor(visibleColor[0], visibleColor[1], + visibleColor[2], 0); + SGVec3f normal = normalize(light.normal); + SGVec3f perp1 = perpendicular(normal); + SGVec3f perp2 = cross(normal, perp1); + SGVec3f position = light.position; + vertices->push_back(toOsg(position)); + vertices->push_back(toOsg(position + perp1)); + vertices->push_back(toOsg(position + perp2)); + colors->push_back(toOsg(visibleColor)); + colors->push_back(toOsg(invisibleColor)); + colors->push_back(toOsg(invisibleColor)); - osg::Geometry* geometry = new osg::Geometry; - geometry->setDataVariance(osg::Object::STATIC); - geometry->setVertexArray(vertices); - geometry->setNormalBinding(osg::Geometry::BIND_OFF); - geometry->setColorArray(colors); - geometry->setColorBinding(osg::Geometry::BIND_PER_VERTEX); + osg::Geometry* geometry = new osg::Geometry; + geometry->setDataVariance(osg::Object::STATIC); + geometry->setVertexArray(vertices); + geometry->setNormalBinding(osg::Geometry::BIND_OFF); + geometry->setColorArray(colors); + geometry->setColorBinding(osg::Geometry::BIND_PER_VERTEX); - // Enlarge the bounding box to avoid such light nodes being victim to - // small feature culling. - geometry->setComputeBoundingBoxCallback(new SGEnlargeBoundingBox(1)); + // Enlarge the bounding box to avoid such light nodes being victim to + // small feature culling. + geometry->setComputeBoundingBoxCallback(new SGEnlargeBoundingBox(1)); - osg::DrawArrays* drawArrays; - drawArrays = new osg::DrawArrays(osg::PrimitiveSet::TRIANGLES, - 0, vertices->size()); - geometry->addPrimitiveSet(drawArrays); - return geometry; + osg::DrawArrays* drawArrays; + drawArrays = new osg::DrawArrays(osg::PrimitiveSet::TRIANGLES, + 0, vertices->size()); + geometry->addPrimitiveSet(drawArrays); + return geometry; + } else { + // Workaround for driver issue where point sprites cannot be triangles, + // (which we use to provide a sensible normal). So fall back to a simple + // non-directional point sprite. + const SGLightBin::Light nonDirectionalLight = SGLightBin::Light(light.position, light.color); + return getLightDrawable(nonDirectionalLight); + } } namespace @@ -367,6 +375,9 @@ SGLightFactory::getSequenced(const SGDirectionalLightBin& lights, const SGReader if (lights.getNumLights() <= 0) return 0; + static SGSceneFeatures* sceneFeatures = SGSceneFeatures::instance(); + bool useTriangles = sceneFeatures->getEnableTriangleDirectionalLights(); + // generate a repeatable random seed sg_srandom(unsigned(lights.getLight(0).position[0])); float flashTime = 0.065 + 0.003 * sg_random(); @@ -377,7 +388,7 @@ SGLightFactory::getSequenced(const SGDirectionalLightBin& lights, const SGReader for (int i = lights.getNumLights() - 1; 0 <= i; --i) { EffectGeode* egeode = new EffectGeode; egeode->setEffect(effect); - egeode->addDrawable(getLightDrawable(lights.getLight(i))); + egeode->addDrawable(getLightDrawable(lights.getLight(i), useTriangles)); sequence->addChild(egeode, flashTime); } sequence->addChild(new osg::Group, 1.9 + (0.1 * sg_random()) - (lights.getNumLights() * flashTime)); @@ -394,6 +405,9 @@ SGLightFactory::getReil(const SGDirectionalLightBin& lights, const SGReaderWrite if (lights.getNumLights() <= 0) return 0; + static SGSceneFeatures* sceneFeatures = SGSceneFeatures::instance(); + bool useTriangles = sceneFeatures->getEnableTriangleDirectionalLights(); + // generate a repeatable random seed sg_srandom(unsigned(lights.getLight(0).position[0])); float flashTime = 0.065 + 0.003 * sg_random(); @@ -405,7 +419,7 @@ SGLightFactory::getReil(const SGDirectionalLightBin& lights, const SGReaderWrite egeode->setEffect(effect); for (int i = lights.getNumLights() - 1; 0 <= i; --i) { - egeode->addDrawable(getLightDrawable(lights.getLight(i))); + egeode->addDrawable(getLightDrawable(lights.getLight(i), useTriangles)); } sequence->addChild(egeode, flashTime); sequence->addChild(new osg::Group, 1.9 + 0.1 * sg_random() - flashTime); @@ -463,6 +477,9 @@ SGLightFactory::getHoldShort(const SGDirectionalLightBin& lights, const SGReader if (lights.getNumLights() < 2) return 0; + static SGSceneFeatures* sceneFeatures = SGSceneFeatures::instance(); + bool useTriangles = sceneFeatures->getEnableTriangleDirectionalLights(); + sg_srandom(unsigned(lights.getLight(0).position[0])); float flashTime = 0.9 + 0.2 * sg_random(); osg::Sequence* sequence = new osg::Sequence; @@ -476,7 +493,7 @@ SGLightFactory::getHoldShort(const SGDirectionalLightBin& lights, const SGReader EffectGeode* egeode = new EffectGeode; egeode->setEffect(effect); for (unsigned int j = 0; j < lights.getNumLights(); ++j) { - egeode->addDrawable(getLightDrawable(lights.getLight(j))); + egeode->addDrawable(getLightDrawable(lights.getLight(j), useTriangles)); } sequence->addChild(egeode, (i==4) ? flashTime : 0.1); } @@ -494,6 +511,9 @@ SGLightFactory::getGuard(const SGDirectionalLightBin& lights, const SGReaderWrit if (lights.getNumLights() < 2) return 0; + static SGSceneFeatures* sceneFeatures = SGSceneFeatures::instance(); + bool useTriangles = sceneFeatures->getEnableTriangleDirectionalLights(); + // generate a repeatable random seed sg_srandom(unsigned(lights.getLight(0).position[0])); float flashTime = 0.9 + 0.2 * sg_random(); @@ -504,7 +524,7 @@ SGLightFactory::getGuard(const SGDirectionalLightBin& lights, const SGReaderWrit for (unsigned int i = 0; i < lights.getNumLights(); ++i) { EffectGeode* egeode = new EffectGeode; egeode->setEffect(effect); - egeode->addDrawable(getLightDrawable(lights.getLight(i))); + egeode->addDrawable(getLightDrawable(lights.getLight(i), useTriangles)); sequence->addChild(egeode, flashTime); } sequence->setInterval(osg::Sequence::LOOP, 0, -1); diff --git a/simgear/scene/tgdb/pt_lights.hxx b/simgear/scene/tgdb/pt_lights.hxx index 005aea1d..0b8d15d5 100644 --- a/simgear/scene/tgdb/pt_lights.hxx +++ b/simgear/scene/tgdb/pt_lights.hxx @@ -55,10 +55,12 @@ class Effect; // appropriate extensions are available.) inline void SGConfigureDirectionalLights( bool use_point_sprites, - bool distance_attenuation ) { + bool distance_attenuation, + bool use_triangles ) { static SGSceneFeatures* sceneFeatures = SGSceneFeatures::instance(); sceneFeatures->setEnablePointSpriteLights(use_point_sprites); sceneFeatures->setEnableDistanceAttenuationLights(distance_attenuation); + sceneFeatures->setEnableTriangleDirectionalLights(use_triangles); } class SGLightFactory { @@ -68,7 +70,7 @@ public: getLightDrawable(const SGLightBin::Light& light); static osg::Drawable* - getLightDrawable(const SGDirectionalLightBin::Light& light); + getLightDrawable(const SGDirectionalLightBin::Light& light, bool useTriangles); /** * Return a drawable for a very simple point light that isn't diff --git a/simgear/scene/util/SGSceneFeatures.cxx b/simgear/scene/util/SGSceneFeatures.cxx index 48aecf22..f17b52ac 100644 --- a/simgear/scene/util/SGSceneFeatures.cxx +++ b/simgear/scene/util/SGSceneFeatures.cxx @@ -1,6 +1,6 @@ /* -*-c++-*- * - * Copyright (C) 2006-2007 Mathias Froehlich + * Copyright (C) 2006-2007 Mathias Froehlich * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -46,6 +46,7 @@ SGSceneFeatures::SGSceneFeatures() : _TextureCacheActive(false), _shaderLights(true), _pointSpriteLights(true), + _triangleDirectionalLights(true), _distanceAttenuationLights(true), _textureFilter(1) { @@ -108,7 +109,7 @@ SGSceneFeatures::getHaveFragmentPrograms(unsigned contextId) const return false; if (!fpe->isFragmentProgramSupported()) return false; - + return true; #else const osg::GLExtensions* ex = osg::GLExtensions::Get(contextId, true); @@ -126,7 +127,7 @@ SGSceneFeatures::getHaveVertexPrograms(unsigned contextId) const return false; if (!vpe->isVertexProgramSupported()) return false; - + return true; #else const osg::GLExtensions* ex = osg::GLExtensions::Get(contextId, true); @@ -158,4 +159,3 @@ SGSceneFeatures::getHavePointParameters(unsigned contextId) const return ex && ex->isPointParametersSupported; #endif } - diff --git a/simgear/scene/util/SGSceneFeatures.hxx b/simgear/scene/util/SGSceneFeatures.hxx index 5b9f7f7c..cb9453d5 100644 --- a/simgear/scene/util/SGSceneFeatures.hxx +++ b/simgear/scene/util/SGSceneFeatures.hxx @@ -75,6 +75,21 @@ public: return getHavePointSprites(contextId); } + void setEnableTriangleDirectionalLights(bool enable) + { + _triangleDirectionalLights = enable; + } + bool getEnableTriangleDirectionalLights() const + { + return _triangleDirectionalLights; + } + bool getEnableTriangleDirectionalLights(unsigned contextId) const + { + if (!_triangleDirectionalLights) + return false; + return getHaveTriangleDirectionals(contextId); + } + void setEnableDistanceAttenuationLights(bool enable) { _distanceAttenuationLights = enable; @@ -108,6 +123,7 @@ public: protected: bool getHavePointSprites(unsigned contextId) const; + bool getHaveTriangleDirectionals(unsigned contextId) const; bool getHaveFragmentPrograms(unsigned contextId) const; bool getHaveVertexPrograms(unsigned contextId) const; bool getHaveShaderPrograms(unsigned contextId) const; @@ -126,6 +142,7 @@ private: bool _TextureCacheActive; bool _shaderLights; bool _pointSpriteLights; + bool _triangleDirectionalLights; bool _distanceAttenuationLights; int _textureFilter; }; From 11e243a2165b343633ba2d19208a972477bbf1bf Mon Sep 17 00:00:00 2001 From: Erik Hofman Date: Wed, 29 Jan 2020 11:33:54 +0100 Subject: [PATCH 10/10] Clean up the threading code: remove the private classes. --- simgear/threads/SGThread.cxx | 250 +++++++++++++---------------------- simgear/threads/SGThread.hxx | 19 ++- 2 files changed, 103 insertions(+), 166 deletions(-) diff --git a/simgear/threads/SGThread.cxx b/simgear/threads/SGThread.cxx index 4c618b99..0c1c5a30 100644 --- a/simgear/threads/SGThread.cxx +++ b/simgear/threads/SGThread.cxx @@ -4,6 +4,7 @@ // // Copyright (C) 2001 Bernard Bright - bbright@bigpond.net.au // Copyright (C) 2011 Mathias Froehlich +// Copyright (C) 2020 Erik Hofman // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -36,178 +37,49 @@ #include #if _WIN32 -# include # include #else # include -# include -# include -# include #endif -struct SGWaitCondition::PrivateData { - PrivateData(void) - { - } - ~PrivateData(void) - { - } - void signal(void) - { - bool waiting; - - { - std::lock_guard lock(_mtx); - waiting = !ready; - ready = true; - } - - if (waiting) { - _condition.notify_one(); - } - } - - void broadcast(void) - { - bool waiting; - - { - std::lock_guard lock(_mtx); - waiting = !ready; - ready = true; - } - - if (waiting) { - _condition.notify_all(); - } - } - - void wait(std::mutex& mutex) noexcept - { - mutex.unlock(); - - { - std::unique_lock lock(_mtx); - _condition.wait(lock, [this]{ return ready; } ); - ready = false; - } - -#ifndef NDEBUG -# if _WIN32 -// native_handle_type *m = mutex.native_handle(); -// assert(m->LockCount == 0); -# else - pthread_mutex_t *m = mutex.native_handle(); - assert(m->__data.__count == 0); -# endif -#endif - mutex.lock(); - } - - bool wait(std::mutex& mutex, unsigned msec) noexcept - { - auto timeout = std::chrono::milliseconds(msec); - - mutex.unlock(); - - { - std::unique_lock lock(_mtx); - _condition.wait_for(lock, timeout, [this]{ return ready; } ); - ready = false; - } - -#ifndef NDEBUG -# if _WIN32 -// native_handle_type *m = mutex.native_handle(); -// assert(m->LockCount == 0); -# else - pthread_mutex_t *m = mutex.native_handle(); - assert(m->__data.__count == 0); -# endif -#endif - mutex.lock(); - - return true; - } - - std::mutex _mtx; - std::condition_variable _condition; - std::atomic m_holder; - -private: - bool ready = false; -}; - -struct SGThread::PrivateData { - PrivateData() - { - } - ~PrivateData() - { - // If we are still having a started thread and nobody waited, - // now detach ... - if (!_started) - return; - _thread.detach(); - } - - static void *start_routine(void* data) - { - SGThread* thread = reinterpret_cast(data); - thread->run(); - return 0; - } - - bool start(SGThread& thread) - { - if (_started) - return false; - - try { - _thread = std::thread(start_routine, &thread); - } catch (std::runtime_error &ex) { - return false; - } - - _started = true; - return true; - } - - void join() - { - if (!_started) - return; - - _thread.join(); - _started = false; - } - - std::thread _thread; - bool _started = false; -}; - -SGThread::SGThread() : - _privateData(new PrivateData) +SGThread::SGThread() { } SGThread::~SGThread() { - delete _privateData; - _privateData = 0; + // If we are still having a started thread and nobody waited, + // now detach ... + if (!_started) + return; + _thread.detach(); } bool SGThread::start() { - return _privateData->start(*this); + if (_started) + return false; + + try { + _thread = std::thread(start_routine, this); + } catch (std::runtime_error &ex) { + return false; + } + + _started = true; + return true; } void SGThread::join() { - _privateData->join(); + if (!_started) + return; + + _thread.join(); + _started = false; } long SGThread::current( void ) @@ -219,40 +91,100 @@ long SGThread::current( void ) #endif } +void *SGThread::start_routine(void* data) +{ + SGThread* thread = reinterpret_cast(data); + thread->run(); + return 0; +} -SGWaitCondition::SGWaitCondition() : - _privateData(new PrivateData) + +SGWaitCondition::SGWaitCondition() { } SGWaitCondition::~SGWaitCondition() { - delete _privateData; - _privateData = 0; } void SGWaitCondition::wait(std::mutex& mutex) { - _privateData->wait(mutex); +#ifndef NDEBUG + try { + mutex.unlock(); + } catch(const std::system_error& e) { + SG_LOG(SG_GENERAL, SG_ALERT, "Unlocking error " << e.what() ); + } +#else + mutex.unlock(); +#endif + + { + std::unique_lock lock(_mtx); + _condition.wait(lock, [this]{ return ready; } ); + ready = false; + } + +#ifndef NDEBUG + try { + mutex.lock(); + } catch(const std::system_error& e) { + SG_LOG(SG_GENERAL, SG_ALERT, "Locking error " << e.what() ); + } +#else + mutex.lock(); +#endif } bool SGWaitCondition::wait(std::mutex& mutex, unsigned msec) { - return _privateData->wait(mutex, msec); + auto timeout = std::chrono::milliseconds(msec); + + mutex.unlock(); + + { + std::unique_lock lock(_mtx); + _condition.wait_for(lock, timeout, [this]{ return ready; } ); + ready = false; + } + + mutex.lock(); + + return true; } void SGWaitCondition::signal() { - _privateData->signal(); + bool waiting; + + { + std::lock_guard lock(_mtx); + waiting = !ready; + ready = true; + } + + if (waiting) { + _condition.notify_one(); + } } void SGWaitCondition::broadcast() { - _privateData->broadcast(); + bool waiting; + + { + std::lock_guard lock(_mtx); + waiting = !ready; + ready = true; + } + + if (waiting) { + _condition.notify_all(); + } } diff --git a/simgear/threads/SGThread.hxx b/simgear/threads/SGThread.hxx index ba9f52d2..738552e7 100644 --- a/simgear/threads/SGThread.hxx +++ b/simgear/threads/SGThread.hxx @@ -4,6 +4,7 @@ // // Copyright (C) 2001 Bernard Bright - bbright@bigpond.net.au // Copyright (C) 2011 Mathias Froehlich +// Copyright (C) 2020 Erik Hofman // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -23,6 +24,7 @@ #ifndef SGTHREAD_HXX_INCLUDED #define SGTHREAD_HXX_INCLUDED 1 +#include #include #include #include @@ -78,18 +80,20 @@ protected: */ virtual void run() = 0; + /** + * General thread starter routine. + */ + static void *start_routine(void* data); + private: // Disable copying. SGThread(const SGThread&); SGThread& operator=(const SGThread&); - struct PrivateData; - PrivateData* _privateData; - - friend struct PrivateData; + std::thread _thread; + bool _started = false; }; -class SGWaitCondition; /** * A condition variable is a synchronization device that allows threads to @@ -146,8 +150,9 @@ private: SGWaitCondition(const SGWaitCondition&); SGWaitCondition& operator=(const SGWaitCondition&); - struct PrivateData; - PrivateData* _privateData; + bool ready = false; + std::mutex _mtx; + std::condition_variable _condition; }; ///