From f77724a646e2d2dbf8c01c3f1737a085fdf57cd1 Mon Sep 17 00:00:00 2001 From: James Turner Date: Thu, 19 Jul 2018 07:40:23 +0100 Subject: [PATCH 01/13] Fix terrasync hash persistence --- simgear/io/HTTPRepository.cxx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/simgear/io/HTTPRepository.cxx b/simgear/io/HTTPRepository.cxx index fb2cd9fe..41be821a 100644 --- a/simgear/io/HTTPRepository.cxx +++ b/simgear/io/HTTPRepository.cxx @@ -1006,11 +1006,11 @@ HTTPRepository::failure() const SGPath cachePath = basePath; cachePath.append(".hashes"); - sg_ofstream stream(cachePath, std::ios::out | std::ios::trunc); + sg_ofstream stream(cachePath, std::ios::out | std::ios::trunc | std::ios::binary); HashCache::const_iterator it; for (it = hashes.begin(); it != hashes.end(); ++it) { - stream << it->filePath << ":" << it->modTime << ":" - << it->lengthBytes << ":" << it->hashHex << "\n"; + stream << it->filePath << "*" << it->modTime << "*" + << it->lengthBytes << "*" << it->hashHex << "\n"; } stream.close(); hashCacheDirty = false; @@ -1034,7 +1034,7 @@ HTTPRepository::failure() const if( line.empty() || line[0] == '#' ) continue; - string_list tokens = simgear::strutils::split( line, ":" ); + string_list tokens = simgear::strutils::split(line, "*"); if( tokens.size() < 4 ) { SG_LOG(SG_TERRASYNC, SG_WARN, "invalid entry in '" << cachePath << "': '" << line << "' (ignoring line)"); continue; @@ -1056,6 +1056,9 @@ HTTPRepository::failure() const entry.lengthBytes = strtol(sizeData.c_str(), NULL, 10); hashes.push_back(entry); } + + SG_LOG(SG_TERRASYNC, SG_INFO, "restored hashes:" << hashes.size()); + } class DirectoryWithPath From f1e1bf11b6105aedfb6c898055a60f23e75557a5 Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 24 Jun 2018 21:55:20 +0100 Subject: [PATCH 02/13] Cache x/y nodes in Canvas::Map::GeoPair This removes a bad CPU hit when Canvas map is used continuously, since this is two named property lookups per map coord per frame --- simgear/canvas/elements/map/geo_node_pair.hxx | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/simgear/canvas/elements/map/geo_node_pair.hxx b/simgear/canvas/elements/map/geo_node_pair.hxx index deb4c373..4733238b 100644 --- a/simgear/canvas/elements/map/geo_node_pair.hxx +++ b/simgear/canvas/elements/map/geo_node_pair.hxx @@ -67,7 +67,8 @@ namespace canvas { _node_lat = node; _status &= ~LAT_MISSING; - + _xNode.reset(); + if( node == _node_lon ) { _node_lon = 0; @@ -79,7 +80,8 @@ namespace canvas { _node_lon = node; _status &= ~LON_MISSING; - + _yNode.reset(); + if( node == _node_lat ) { _node_lat = 0; @@ -100,16 +102,25 @@ namespace canvas void setTargetName(const std::string& name) { _target_name = name; + _xNode.reset(); + _yNode.reset(); } void setScreenPos(float x, float y) { assert( isComplete() ); - SGPropertyNode *parent = _node_lat->getParent(); - parent->getChild(_target_name, _node_lat->getIndex(), true) - ->setDoubleValue(x); - parent->getChild(_target_name, _node_lon->getIndex(), true) - ->setDoubleValue(y); + if (!_xNode) { + SGPropertyNode *parent = _node_lat->getParent(); + _xNode = parent->getChild(_target_name, _node_lat->getIndex(), true); + } + + if (!_yNode) { + SGPropertyNode *parent = _node_lat->getParent(); + _yNode = parent->getChild(_target_name, _node_lon->getIndex(), true); + } + + _xNode->setDoubleValue(x); + _yNode->setDoubleValue(y); } void print() @@ -125,6 +136,8 @@ namespace canvas SGPropertyNode *_node_lat, *_node_lon; std::string _target_name; + SGPropertyNode_ptr _xNode, + _yNode; }; From 9553a604ac929f13e97e89f04c050e3d54b05cec Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 26 Jun 2018 08:25:59 +0100 Subject: [PATCH 03/13] Cache parsed lat/lon values in Canvas::Map This avoids re-parsing each lat/lon in the map from a string, each time the project changes --- simgear/canvas/elements/CanvasMap.cxx | 31 ++++++++++++------- simgear/canvas/elements/map/geo_node_pair.hxx | 7 +++++ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/simgear/canvas/elements/CanvasMap.cxx b/simgear/canvas/elements/CanvasMap.cxx index a7c43a51..ca97c4f7 100644 --- a/simgear/canvas/elements/CanvasMap.cxx +++ b/simgear/canvas/elements/CanvasMap.cxx @@ -97,17 +97,26 @@ namespace canvas || (!geo_node->isDirty() && !_projection_dirty) ) continue; - GeoCoord lat = parseGeoCoord(geo_node->getLat()); - if( lat.type != GeoCoord::LATITUDE ) - continue; - - GeoCoord lon = parseGeoCoord(geo_node->getLon()); - if( lon.type != GeoCoord::LONGITUDE ) - continue; - - Projection::ScreenPosition pos = - _projection->worldToScreen(lat.value, lon.value); - + double latD = -9999.0, lonD = -9999.0; + if (geo_node->isDirty()) { + GeoCoord lat = parseGeoCoord(geo_node->getLat()); + if( lat.type != GeoCoord::LATITUDE ) + continue; + + GeoCoord lon = parseGeoCoord(geo_node->getLon()); + if( lon.type != GeoCoord::LONGITUDE ) + continue; + + // save the parsed values so we can re-use them if only projection + // is changed (very common case for moving vehicle) + latD = lat.value; + lonD = lon.value; + geo_node->setCachedLatLon(std::make_pair(latD, lonD)); + } else { + std::tie(latD, lonD) = geo_node->getCachedLatLon(); + } + + Projection::ScreenPosition pos = _projection->worldToScreen(latD, lonD); geo_node->setScreenPos(pos.x, pos.y); // geo_node->print(); diff --git a/simgear/canvas/elements/map/geo_node_pair.hxx b/simgear/canvas/elements/map/geo_node_pair.hxx index 4733238b..aa01aa91 100644 --- a/simgear/canvas/elements/map/geo_node_pair.hxx +++ b/simgear/canvas/elements/map/geo_node_pair.hxx @@ -99,6 +99,12 @@ namespace canvas return _node_lon ? _node_lon->getStringValue() : ""; } + void setCachedLatLon(const std::pair& latLon) + { _cachedLatLon = latLon; } + + std::pair getCachedLatLon() + { return _cachedLatLon; } + void setTargetName(const std::string& name) { _target_name = name; @@ -138,6 +144,7 @@ namespace canvas std::string _target_name; SGPropertyNode_ptr _xNode, _yNode; + std::pair _cachedLatLon; }; From 2e3cace7f96315377af6145cd823aba3aee3358f Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 26 Jun 2018 08:26:57 +0100 Subject: [PATCH 04/13] Canvas::path: early return for descendant changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ‘rect’ attributes of Path were before the early-return when the change is for a descendant node. --- simgear/canvas/elements/CanvasPath.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/simgear/canvas/elements/CanvasPath.cxx b/simgear/canvas/elements/CanvasPath.cxx index 5fd8b775..ae083e84 100644 --- a/simgear/canvas/elements/CanvasPath.cxx +++ b/simgear/canvas/elements/CanvasPath.cxx @@ -863,6 +863,9 @@ namespace canvas //---------------------------------------------------------------------------- void Path::childChanged(SGPropertyNode* child) { + if( child->getParent() != _node ) + return; + const std::string& name = child->getNameString(); const std::string &prName = child->getParent()->getNameString(); @@ -892,9 +895,6 @@ namespace canvas return; } - if( child->getParent() != _node ) - return; - if( name == "cmd" ) _attributes_dirty |= CMDS; else if( name == "coord" ) From 2c9420d9bc02da7b05b0b9c52921ff89e3e4358b Mon Sep 17 00:00:00 2001 From: Edward d'Auvergne Date: Fri, 8 Jun 2018 21:42:47 +0200 Subject: [PATCH 05/13] Logstream: Added a logging class for outputting messages in headless mode. --- simgear/debug/debug_types.h | 3 ++- simgear/debug/logstream.cxx | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/simgear/debug/debug_types.h b/simgear/debug/debug_types.h index 2443276b..cc856556 100644 --- a/simgear/debug/debug_types.h +++ b/simgear/debug/debug_types.h @@ -34,7 +34,8 @@ typedef enum { SG_GUI = 0x00800000, SG_TERRASYNC = 0x01000000, SG_PARTICLES = 0x02000000, - SG_UNDEFD = 0x04000000, // For range checking + SG_HEADLESS = 0x04000000, + SG_UNDEFD = 0x08000000, // For range checking SG_ALL = 0xFFFFFFFF } sgDebugClass; diff --git a/simgear/debug/logstream.cxx b/simgear/debug/logstream.cxx index b8b4d312..4e2f6ee4 100644 --- a/simgear/debug/logstream.cxx +++ b/simgear/debug/logstream.cxx @@ -100,6 +100,7 @@ const char* LogCallback::debugClassToString(sgDebugClass c) case SG_GUI: return "gui"; case SG_TERRASYNC: return "terrasync"; case SG_PARTICLES: return "particles"; + case SG_HEADLESS: return "headless"; default: return "unknown"; } } From 3817bdd6025f1cd7868b7eaaa4da9bb0fc49d114 Mon Sep 17 00:00:00 2001 From: Edward d'Auvergne Date: Wed, 18 Jul 2018 10:28:56 +0200 Subject: [PATCH 06/13] NotifyLogger: Shifted the class from flightgear to simgear. This is to allow the code to be reused in the flightgear test suite. --- simgear/debug/CMakeLists.txt | 4 +-- simgear/debug/OsgIoCapture.hxx | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 simgear/debug/OsgIoCapture.hxx diff --git a/simgear/debug/CMakeLists.txt b/simgear/debug/CMakeLists.txt index 07690068..e494eb92 100644 --- a/simgear/debug/CMakeLists.txt +++ b/simgear/debug/CMakeLists.txt @@ -1,7 +1,7 @@ include (SimGearComponent) -set(HEADERS debug_types.h logstream.hxx BufferedLogCallback.hxx) +set(HEADERS debug_types.h logstream.hxx BufferedLogCallback.hxx OsgIoCapture.hxx) set(SOURCES logstream.cxx BufferedLogCallback.cxx) -simgear_component(debug debug "${SOURCES}" "${HEADERS}") \ No newline at end of file +simgear_component(debug debug "${SOURCES}" "${HEADERS}") diff --git a/simgear/debug/OsgIoCapture.hxx b/simgear/debug/OsgIoCapture.hxx new file mode 100644 index 00000000..7804dda2 --- /dev/null +++ b/simgear/debug/OsgIoCapture.hxx @@ -0,0 +1,45 @@ +#include + +#include + +using namespace osg; + + +/** + * merge OSG output into our logging system, so it gets recorded to file, + * and so we can display a GUI console with renderer issues, especially + * shader compilation warnings and errors. + */ +class NotifyLogger : public osg::NotifyHandler +{ +public: + // note this callback will be invoked by OSG from multiple threads. + // fortunately our Simgear logging implementation already handles + // that internally, so we simply pass the message on. + virtual void notify(osg::NotifySeverity severity, const char* message) { + // Detect whether a osg::Reference derived object is deleted with a non-zero + // reference count. In this case trigger a segfault to get a stack trace. + if (strstr(message, "the final reference count was")) { + // as this is going to segfault ignore the translation of severity and always output the message. + SG_LOG(SG_GL, SG_ALERT, message); + int* trigger_segfault = 0; + *trigger_segfault = 0; + return; + } + SG_LOG(SG_GL, translateSeverity(severity), message); + } + +private: + sgDebugPriority translateSeverity(osg::NotifySeverity severity) { + switch (severity) { + case osg::ALWAYS: + case osg::FATAL: return SG_ALERT; + case osg::WARN: return SG_WARN; + case osg::NOTICE: + case osg::INFO: return SG_INFO; + case osg::DEBUG_FP: + case osg::DEBUG_INFO: return SG_DEBUG; + default: return SG_ALERT; + } + } +}; From 6f505f3a76e80c43c518730cea188b944ae60bee Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 29 Jul 2018 09:19:17 +0100 Subject: [PATCH 07/13] formatGeodAsString: add ICAO route format --- simgear/misc/strutils.cxx | 35 ++++++++++++++++++++++++++++++++++ simgear/misc/strutils.hxx | 3 ++- simgear/misc/strutils_test.cxx | 10 ++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/simgear/misc/strutils.cxx b/simgear/misc/strutils.cxx index 1de78be5..b15a73e7 100644 --- a/simgear/misc/strutils.cxx +++ b/simgear/misc/strutils.cxx @@ -1379,6 +1379,35 @@ std::string formatLatLonValueAsString(double deg, LatLonFormat format, case LatLonFormat::DECIMAL_DEGREES_SYMBOL: ::snprintf(buf, sizeof(buf), "%3.6f%s%c", deg, degSym, c); break; + + case LatLonFormat::ICAO_ROUTE_DEGREES: + { + min = (deg - int(deg)) * 60.0; + if (min >= 59.9995) { + min -= 60.0; + deg += 1.0; + } + + if (static_cast(min) == 0) { + // 7-digit mode + if (c == 'N' || c == 'S') { + snprintf(buf, sizeof(buf), "%02d%c", int(deg), c); + } else { + snprintf(buf, sizeof(buf), "%03d%c", int(deg), c); + } + } else { + // 11-digit mode + if (c == 'N' || c == 'S') { + snprintf(buf, sizeof(buf), "%02d%02d%c", int(deg), int(min), c); + } else { + snprintf(buf, sizeof(buf), "%03d%02d%c", int(deg), int(min), c); + } + } + break; + } + + default: + break; } return std::string(buf); @@ -1390,6 +1419,12 @@ std::string formatGeodAsString(const SGGeod& geod, LatLonFormat format, const char ns = (geod.getLatitudeDeg() > 0.0) ? 'N' : 'S'; const char ew = (geod.getLongitudeDeg() > 0.0) ? 'E' : 'W'; + // no comma seperator + if (format == LatLonFormat::ICAO_ROUTE_DEGREES) { + return formatLatLonValueAsString(geod.getLatitudeDeg(), format, ns, degreeSymbol) + + formatLatLonValueAsString(geod.getLongitudeDeg(), format, ew, degreeSymbol); + } + return formatLatLonValueAsString(geod.getLatitudeDeg(), format, ns, degreeSymbol) + "," + formatLatLonValueAsString(geod.getLongitudeDeg(), format, ew, degreeSymbol); } diff --git a/simgear/misc/strutils.hxx b/simgear/misc/strutils.hxx index f4b14558..74fb7f39 100644 --- a/simgear/misc/strutils.hxx +++ b/simgear/misc/strutils.hxx @@ -396,7 +396,8 @@ namespace simgear { ZERO_PAD_DEGREES_MINUTES, ZERO_PAD_DEGREES_MINUTES_SECONDS, TRINITY_HOUSE, ///< dd* mm'.mmm X, ddd* mm'.mmm X (Trinity House Navigation standard). - DECIMAL_DEGREES_SYMBOL ///< 88.4*N,4.54*W + DECIMAL_DEGREES_SYMBOL, ///< 88.4*N,4.54*W + ICAO_ROUTE_DEGREES, ///< 52N045W or 5212N04512W - precision auto-selected }; enum class DegreeSymbol diff --git a/simgear/misc/strutils_test.cxx b/simgear/misc/strutils_test.cxx index 07ff8c1c..35ac5b79 100644 --- a/simgear/misc/strutils_test.cxx +++ b/simgear/misc/strutils_test.cxx @@ -684,6 +684,14 @@ void test_formatGeod() SG_CHECK_EQUAL(strutils::formatGeodAsString(a, strutils::LatLonFormat::SIGNED_DECIMAL_DEGREES), "55.450000,-3.460000"); SG_CHECK_EQUAL(strutils::formatGeodAsString(a, strutils::LatLonFormat::DEGREES_MINUTES_SECONDS), "55*27'00.0\"N,3*27'36.0\"W"); + + + SG_CHECK_EQUAL(strutils::formatGeodAsString(a, strutils::LatLonFormat::ICAO_ROUTE_DEGREES), + "5527N00327W"); + SGGeod shortA = SGGeod::fromDeg(106, -34); + SG_CHECK_EQUAL(strutils::formatGeodAsString(shortA, strutils::LatLonFormat::ICAO_ROUTE_DEGREES), + "34S106E"); + const auto s = strutils::formatGeodAsString(a, strutils::LatLonFormat::ZERO_PAD_DEGREES_MINUTES, @@ -696,6 +704,8 @@ void test_formatGeod() strutils::LatLonFormat::DECIMAL_DEGREES_SYMBOL, strutils::DegreeSymbol::UTF8_DEGREE); SG_CHECK_EQUAL(s2, "6.156800\xC2\xB0S,106.827800\xC2\xB0" "E"); + + } int main(int argc, char* argv[]) From 264779e1d36d35716b6dd997b34ba0fbfe6bce77 Mon Sep 17 00:00:00 2001 From: James Turner Date: Wed, 8 Aug 2018 11:12:53 +0200 Subject: [PATCH 08/13] Packages: fix download progress (Broke it when improving zip/tar support) --- simgear/package/Install.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/simgear/package/Install.cxx b/simgear/package/Install.cxx index 5754b485..85f8b708 100644 --- a/simgear/package/Install.cxx +++ b/simgear/package/Install.cxx @@ -45,8 +45,7 @@ class Install::PackageArchiveDownloader : public HTTP::Request public: PackageArchiveDownloader(InstallRef aOwner) : HTTP::Request("" /* dummy URL */), - m_owner(aOwner), - m_downloaded(0) + m_owner(aOwner) { m_urls = m_owner->package()->downloadUrls(); if (m_urls.empty()) { @@ -115,6 +114,7 @@ protected: { const uint8_t* ubytes = (uint8_t*) s; SG_MD5Update(&m_md5, ubytes, n); + m_downloaded += n; m_owner->installProgress(m_downloaded, responseLength()); m_extractor->extractBytes(ubytes, n); } @@ -221,7 +221,7 @@ private: string_list m_urls; SG_MD5_CTX m_md5; SGPath m_extractPath; - size_t m_downloaded; + size_t m_downloaded = 0; std::unique_ptr m_extractor; }; From 8e8ce04e181bf744d9a6ec25fb852e6a92cfd2ea Mon Sep 17 00:00:00 2001 From: James Turner Date: Sat, 11 Aug 2018 18:16:33 +0200 Subject: [PATCH 09/13] Subsystems: default name to group name when adding --- simgear/structure/subsystem_mgr.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index 80c7ef0a..644741a7 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -440,6 +440,8 @@ SGSubsystemGroup::set_subsystem (const string &name, SGSubsystem * subsystem, if (name.empty()) { SG_LOG(SG_GENERAL, SG_DEV_WARN, "adding subsystem to group without a name"); // TODO, make this an exception in the future + } else if (subsystem->name().empty()) { + subsystem->set_name(name); } else if (name != subsystem->name()) { SG_LOG(SG_GENERAL, SG_DEV_WARN, "adding subsystem to group with name '" << name << "', but name() returns '" << subsystem->name() << "'"); From 1fa77078fcf571accbb649ebe9a6e7a851bff8ec Mon Sep 17 00:00:00 2001 From: James Turner Date: Sat, 11 Aug 2018 18:41:04 +0200 Subject: [PATCH 10/13] Subsystems: bind/init/unbind on add/remove When adding/remove subsystems to a group which is already bound or inited, transition the subsystem automatically. This removes the need to manually transition the subsystem in various places in Flightgear. --- simgear/structure/subsystem_mgr.cxx | 46 ++++++++++++++++++---------- simgear/structure/subsystem_test.cxx | 39 +++++++++++++++++++++++ 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index 644741a7..474ea3dd 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -456,13 +456,24 @@ SGSubsystemGroup::set_subsystem (const string &name, SGSubsystem * subsystem, notifyDidChange(subsystem, State::ADD); if (_state != State::INVALID) { - SG_LOG(SG_GENERAL, SG_DEV_WARN, "TODO: implement SGSubsystemGroup transition when adding after init"); // transition to the correct state - // bind + if (_state >= State::BIND) { + notifyWillChange(subsystem, State::BIND); + subsystem->bind(); + notifyDidChange(subsystem, State::BIND); + } - // init + if (_state >= State::INIT) { + notifyWillChange(subsystem, State::INIT); + subsystem->init(); + notifyDidChange(subsystem, State::INIT); + } - // post-init + if (_state >= State::POSTINIT) { + notifyWillChange(subsystem, State::POSTINIT); + subsystem->postinit(); + notifyDidChange(subsystem, State::POSTINIT); + } } } @@ -504,11 +515,17 @@ SGSubsystemGroup::remove_subsystem(const string &name) if (_state != State::INVALID) { // transition out correctly - SG_LOG(SG_GENERAL, SG_DEV_WARN, "TODO: implement SGSubsystemGroup transition when removing before shutdown"); - - // shutdown + if (_state >= State::INIT) { + notifyWillChange(sub, State::SHUTDOWN); + sub->shutdown(); + notifyDidChange(sub, State::SHUTDOWN); + } - // unbind + if (_state >= State::BIND) { + notifyWillChange(sub, State::UNBIND); + sub->unbind(); + notifyDidChange(sub, State::UNBIND); + } } notifyWillChange(sub, State::REMOVE); @@ -1152,10 +1169,9 @@ namespace { group, minTime); - if (arg->getBoolValue("do-bind-init", false)) { - sub->bind(); - sub->init(); - } + // we no longer check for the do-bind-init flag here, since set_subsystem + // tracks the group state and will transition the added subsystem + // automatically return true; } @@ -1170,11 +1186,7 @@ namespace { SG_LOG(SG_GENERAL, SG_ALERT, "do_remove_subsystem: unknown subsytem:" << name); return false; } - - // is it safe to always call these? let's assume so! - instance->shutdown(); - instance->unbind(); - + // unplug from the manager (this also deletes the instance!) manager->remove(name.c_str()); return true; diff --git a/simgear/structure/subsystem_test.cxx b/simgear/structure/subsystem_test.cxx index dfc816d0..ed399351 100644 --- a/simgear/structure/subsystem_test.cxx +++ b/simgear/structure/subsystem_test.cxx @@ -439,6 +439,44 @@ void testPropertyRoot() SG_CHECK_EQUAL(props->getIntValue("anothersub/bar"), 172); } +void testAddRemoveAfterInit() +{ + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + auto group = manager->add(); + SG_VERIFY(group); + + auto radio1 = manager->createInstance("nav1"); + group->set_subsystem(radio1); + + manager->bind(); + manager->init(); + + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-init")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-bind")); + + auto radio2 = manager->createInstance("nav2"); + group->set_subsystem(radio2); + + SG_VERIFY(d->hasEvent("fake-radio.nav2-will-init")); + SG_VERIFY(d->hasEvent("fake-radio.nav2-did-init")); + SG_VERIFY(d->hasEvent("fake-radio.nav2-did-bind")); + + bool ok = manager->remove("fake-radio.nav1"); + SG_VERIFY(ok); + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-remove")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-remove")); + + + +} + /////////////////////////////////////////////////////////////////////////////// @@ -450,6 +488,7 @@ int main(int argc, char* argv[]) testIncrementalInit(); testSuspendResume(); testPropertyRoot(); + testAddRemoveAfterInit(); cout << __FILE__ << ": All tests passed" << endl; return EXIT_SUCCESS; From 7853a5329d036e79fed954aea823532ccf0fe778 Mon Sep 17 00:00:00 2001 From: James Turner Date: Sat, 11 Aug 2018 18:41:29 +0200 Subject: [PATCH 11/13] Extend propsfwd.hxx so it can be used in more places --- simgear/props/props.hxx | 2 +- simgear/props/propsfwd.hxx | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/simgear/props/props.hxx b/simgear/props/props.hxx index 18512f18..c94186cb 100644 --- a/simgear/props/props.hxx +++ b/simgear/props/props.hxx @@ -751,7 +751,7 @@ typedef SGSharedPtr SGConstPropertyNode_ptr; namespace simgear { -typedef std::vector PropertyList; + using PropertyList = std::vector; } /** diff --git a/simgear/props/propsfwd.hxx b/simgear/props/propsfwd.hxx index 87998b59..d62e19a7 100644 --- a/simgear/props/propsfwd.hxx +++ b/simgear/props/propsfwd.hxx @@ -1,3 +1,20 @@ +// Copyright (C) 2018 James Turner - +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Library General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// + /** \file * * Forward declarations for properties (and related structures) @@ -7,12 +24,18 @@ #define SG_PROPS_FWD_HXX #include +#include class SGPropertyNode; typedef SGSharedPtr SGPropertyNode_ptr; typedef SGSharedPtr SGConstPropertyNode_ptr; +namespace simgear +{ + using PropertyList = std::vector; +} + class SGCondition; #endif // of SG_PROPS_FWD_HXX From 306eaeaba909d0650f9645b81cef3b0da786b987 Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 19 Aug 2018 12:32:30 +0100 Subject: [PATCH 12/13] Fix subsystem-remove bug --- simgear/structure/subsystem_mgr.cxx | 8 +++---- simgear/structure/subsystem_mgr.hxx | 6 ++++- simgear/structure/subsystem_test.cxx | 33 +++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index 474ea3dd..537649e2 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -455,7 +455,7 @@ SGSubsystemGroup::set_subsystem (const string &name, SGSubsystem * subsystem, subsystem->set_group(this); notifyDidChange(subsystem, State::ADD); - if (_state != State::INVALID) { + if (_state != State::INVALID && (_state <= State::POSTINIT)) { // transition to the correct state if (_state >= State::BIND) { notifyWillChange(subsystem, State::BIND); @@ -469,7 +469,7 @@ SGSubsystemGroup::set_subsystem (const string &name, SGSubsystem * subsystem, notifyDidChange(subsystem, State::INIT); } - if (_state >= State::POSTINIT) { + if (_state == State::POSTINIT) { notifyWillChange(subsystem, State::POSTINIT); subsystem->postinit(); notifyDidChange(subsystem, State::POSTINIT); @@ -515,13 +515,13 @@ SGSubsystemGroup::remove_subsystem(const string &name) if (_state != State::INVALID) { // transition out correctly - if (_state >= State::INIT) { + if ((_state >= State::INIT) && (_state < State::SHUTDOWN)) { notifyWillChange(sub, State::SHUTDOWN); sub->shutdown(); notifyDidChange(sub, State::SHUTDOWN); } - if (_state >= State::BIND) { + if ((_state >= State::BIND) && (_state < State::UNBIND)) { notifyWillChange(sub, State::UNBIND); sub->unbind(); notifyDidChange(sub, State::UNBIND); diff --git a/simgear/structure/subsystem_mgr.hxx b/simgear/structure/subsystem_mgr.hxx index b3cd71f9..95cd5a46 100644 --- a/simgear/structure/subsystem_mgr.hxx +++ b/simgear/structure/subsystem_mgr.hxx @@ -301,6 +301,10 @@ public: /// get the parent group of this subsystem SGSubsystemGroup* get_group() const; + // ordering here is exceptionally important, due to + // liveness of ranges. If you're extending this consider + // carefully where the new state lies and position it correctly. + // Also extend the tests to ensure you handle all cases. enum class State { INVALID = -1, ADD = 0, @@ -310,8 +314,8 @@ public: REINIT, SUSPEND, RESUME, - UNBIND, SHUTDOWN, + UNBIND, REMOVE }; diff --git a/simgear/structure/subsystem_test.cxx b/simgear/structure/subsystem_test.cxx index ed399351..a1136a78 100644 --- a/simgear/structure/subsystem_test.cxx +++ b/simgear/structure/subsystem_test.cxx @@ -451,6 +451,11 @@ void testAddRemoveAfterInit() auto radio1 = manager->createInstance("nav1"); group->set_subsystem(radio1); + auto com1 = manager->createInstance("com1"); + group->set_subsystem(com1); + auto com2 = manager->createInstance("com2"); + group->set_subsystem(com2); + manager->bind(); manager->init(); @@ -473,8 +478,34 @@ void testAddRemoveAfterInit() SG_VERIFY(d->hasEvent("fake-radio.nav1-will-remove")); SG_VERIFY(d->hasEvent("fake-radio.nav1-did-remove")); - + manager->shutdown(); + SG_VERIFY(d->hasEvent("fake-radio.nav2-will-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.nav2-did-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.com1-will-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.com1-did-shutdown")); + + d->events.clear(); + + ok = manager->remove("fake-radio.com1"); + SG_VERIFY(d->hasEvent("fake-radio.com1-will-remove")); + SG_VERIFY(d->hasEvent("fake-radio.com1-did-remove")); + SG_VERIFY(d->hasEvent("fake-radio.com1-will-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.com1-did-unbind")); + SG_VERIFY(!d->hasEvent("fake-radio.com1-will-shutdown")); + SG_VERIFY(!d->hasEvent("fake-radio.com1-did-shutdown")); + + manager->unbind(); + + SG_VERIFY(d->hasEvent("fake-radio.com2-will-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.com2-did-unbind")); + + d->events.clear(); + manager->remove("fake-radio.com2"); + SG_VERIFY(!d->hasEvent("fake-radio.com2-will-unbind")); + SG_VERIFY(!d->hasEvent("fake-radio.com2-did-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.com2-will-remove")); + SG_VERIFY(d->hasEvent("fake-radio.com2-did-remove")); } /////////////////////////////////////////////////////////////////////////////// From 8dd08339931466fd2ef5b12adeefeb0ec9e5946b Mon Sep 17 00:00:00 2001 From: James Turner Date: Thu, 23 Aug 2018 19:11:50 +0100 Subject: [PATCH 13/13] Fix setting OPENAL_DIR on Windows We need to set this in all cases, since AeonWave is requested by default, but may not be available. --- CMakeLists.txt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 02c7cd2d..1e4116a0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -202,11 +202,8 @@ if (MSVC AND MSVC_3RDPARTY_ROOT) message(STATUS "BOOST_INCLUDEDIR is ${BOOST_INCLUDEDIR}") endif() - if (NOT USE_AEONWAVE) - set (OPENAL_INCLUDE_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/include) - set (OPENAL_LIBRARY_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/lib) - message(STATUS "OPENAL_INCLUDE_DIR is ${OPENAL_INCLUDE_DIR}") - endif() + set (OPENAL_INCLUDE_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/include) + set (OPENAL_LIBRARY_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/lib) endif (MSVC AND MSVC_3RDPARTY_ROOT) if(APPLE)