From 0185884ca8fb4f36eef91eced541c19d5f6717bc Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 16 Oct 2018 09:56:56 +0100 Subject: [PATCH 1/5] Catalogs: allow migration to alternate IDs --- simgear/package/Catalog.cxx | 88 +++++++++--- simgear/package/Catalog.hxx | 2 + simgear/package/CatalogTest.cxx | 126 +++++++++++++++++ simgear/package/Root.cxx | 13 +- simgear/package/Root.hxx | 2 + simgear/package/catalogTest1/catalog-alt.xml | 141 +++++++++++++++++++ simgear/package/catalogTest1/catalog-v2.xml | 6 + 7 files changed, 358 insertions(+), 20 deletions(-) create mode 100644 simgear/package/catalogTest1/catalog-alt.xml diff --git a/simgear/package/Catalog.cxx b/simgear/package/Catalog.cxx index de74d19d..5092b888 100644 --- a/simgear/package/Catalog.cxx +++ b/simgear/package/Catalog.cxx @@ -81,17 +81,27 @@ bool checkVersion(const std::string& aVersion, SGPropertyNode_ptr props) } return false; } + +SGPropertyNode_ptr alternateForVersion(const std::string& aVersion, SGPropertyNode_ptr props) +{ + for (auto v : props->getChildren("alternate-version")) { + for (auto versionSpec : v->getChildren("version")) { + if (checkVersionString(aVersion, versionSpec->getStringValue())) { + return v; + } + } + } + + return {}; +} std::string redirectUrlForVersion(const std::string& aVersion, SGPropertyNode_ptr props) { - for (SGPropertyNode* v : props->getChildren("alternate-version")) { - std::string s(v->getStringValue("version")); - if (checkVersionString(aVersion, s)) { - return v->getStringValue("url");; - } - } + auto node = alternateForVersion(aVersion, props); + if (node) + return node->getStringValue("url");; - return std::string(); + return {}; } ////////////////////////////////////////////////////////////////////////////// @@ -139,19 +149,15 @@ protected: std::string ver(m_owner->root()->applicationVersion()); if (!checkVersion(ver, props)) { - SG_LOG(SG_GENERAL, SG_WARN, "downloaded catalog " << m_owner->url() << ", but version required " << ver); - // check for a version redirect entry - std::string url = redirectUrlForVersion(ver, props); - if (!url.empty()) { - SG_LOG(SG_GENERAL, SG_WARN, "redirecting from " << m_owner->url() << - " to \n\t" << url); - - // update the URL and kick off a new request - m_owner->setUrl(url); - Downloader* dl = new Downloader(m_owner, url); - m_owner->root()->makeHTTPRequest(dl); + auto alt = alternateForVersion(ver, props); + if (alt) { + SG_LOG(SG_GENERAL, SG_WARN, "have alternate version of package at:" + << alt->getStringValue("url")); + m_owner->processAlternate(alt); } else { + SG_LOG(SG_GENERAL, SG_WARN, "downloaded catalog " << m_owner->url() + << ", but app version " << ver << " is not comaptible"); m_owner->refreshComplete(Delegate::FAIL_VERSION); } @@ -228,7 +234,7 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath) bool versionCheckOk = checkVersion(aRoot->applicationVersion(), props); if (!versionCheckOk) { - SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: need version: " + SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: app version: " << aRoot->applicationVersion()); // keep the catalog but mark it as needing an update } else { @@ -598,6 +604,50 @@ bool Catalog::isEnabled() const } } +void Catalog::processAlternate(SGPropertyNode_ptr alt) +{ + std::string altId; + const auto idPtr = alt->getStringValue("id"); + if (idPtr) { + altId = std::string(idPtr); + } + + std::string altUrl; + if (alt->getStringValue("url")) { + altUrl = std::string(alt->getStringValue("url")); + } + + CatalogRef existing; + if (!altId.empty()) { + existing = root()->getCatalogById(altId); + } else { + existing = root()->getCatalogByUrl(altUrl); + } + + if (existing && (existing != this)) { + // we already have the alternate, so just go quiet here + changeStatus(Delegate::FAIL_VERSION); + return; + } + + // we have an alternate ID, and it's differnt from our ID, so let's + // define a new catalog + if (!altId.empty()) { + SG_LOG(SG_GENERAL, SG_INFO, "Adding new catalog:" << altId << " as version alternate for " << id()); + // new catalog being added + createFromUrl(root(), altUrl); + + // and we can go idle now + changeStatus(Delegate::FAIL_VERSION); + return; + } + + SG_LOG(SG_GENERAL, SG_INFO, "Migrating catalog " << id() << " to new URL:" << altUrl); + setUrl(altUrl); + Downloader* dl = new Downloader(this, altUrl); + root()->makeHTTPRequest(dl); +} + } // of namespace pkg } // of namespace simgear diff --git a/simgear/package/Catalog.hxx b/simgear/package/Catalog.hxx index c0a6edd8..f300690a 100644 --- a/simgear/package/Catalog.hxx +++ b/simgear/package/Catalog.hxx @@ -179,6 +179,8 @@ private: void changeStatus(Delegate::StatusCode newStatus); + void processAlternate(SGPropertyNode_ptr alt); + Root* m_root; SGPropertyNode_ptr m_props; SGPath m_installRoot; diff --git a/simgear/package/CatalogTest.cxx b/simgear/package/CatalogTest.cxx index d0d1d93a..8b9f3435 100644 --- a/simgear/package/CatalogTest.cxx +++ b/simgear/package/CatalogTest.cxx @@ -741,6 +741,130 @@ void testVersionMigrate(HTTP::Client* cl) } } +void testVersionMigrateToId(HTTP::Client* cl) +{ + global_catalogVersion = 2; // version which has migration info + SGPath rootPath(simgear::Dir::current().path()); + rootPath.append("cat_migrate_version_id"); + simgear::Dir pd(rootPath); + pd.removeChildren(); + + { + pkg::RootRef root(new pkg::Root(rootPath, "8.1.2")); + root->setHTTPClient(cl); + + pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml"); + waitForUpdateComplete(cl, root); + SG_VERIFY(c->isEnabled()); + + // install a package + pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG"); + SG_CHECK_EQUAL(p1->id(), "b737-NG"); + pkg::InstallRef ins = p1->install(); + SG_VERIFY(ins->isQueued()); + waitForUpdateComplete(cl, root); + SG_VERIFY(p1->isInstalled()); + } + + // change version to an alternate one + { + pkg::RootRef root(new pkg::Root(rootPath, "7.5")); + root->setHTTPClient(cl); + + // this should cause the alternate package to be loaded + root->refresh(true); + waitForUpdateComplete(cl, root); + + pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1"); + SG_VERIFY(!cat->isEnabled()); + SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION); + SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1"); + SG_CHECK_EQUAL(cat->url(), "http://localhost:2000/catalogTest1/catalog.xml"); + + auto enabledCats = root->catalogs(); + auto it = std::find(enabledCats.begin(), enabledCats.end(), cat); + SG_VERIFY(it == enabledCats.end()); + + // ensure existing package is still installed + pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG"); + SG_VERIFY(p1 != pkg::PackageRef()); + SG_CHECK_EQUAL(p1->id(), "b737-NG"); + + // but not listed + auto packs = root->allPackages(); + auto k = std::find(packs.begin(), packs.end(), p1); + SG_VERIFY(k == packs.end()); + + // check the new catalog + auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt"); + SG_VERIFY(altCat->isEnabled()); + SG_CHECK_EQUAL(altCat->status(), pkg::Delegate::STATUS_REFRESHED); + SG_CHECK_EQUAL(altCat->id(), "org.flightgear.test.catalog-alt"); + SG_CHECK_EQUAL(altCat->url(), "http://localhost:2000/catalogTest1/catalog-alt.xml"); + + it = std::find(enabledCats.begin(), enabledCats.end(), altCat); + SG_VERIFY(it != enabledCats.end()); + + // install a parallel package from the new catalog + pkg::PackageRef p2 = root->getPackageById("org.flightgear.test.catalog-alt.b737-NG"); + SG_CHECK_EQUAL(p2->id(), "b737-NG"); + pkg::InstallRef ins = p2->install(); + SG_VERIFY(ins->isQueued()); + waitForUpdateComplete(cl, root); + SG_VERIFY(p2->isInstalled()); + + // do a non-scoped lookup, we should get the new one + pkg::PackageRef p3 = root->getPackageById("b737-NG"); + SG_CHECK_EQUAL(p2, p3); + } + + // test that re-init-ing doesn't mirgate again + { + pkg::RootRef root(new pkg::Root(rootPath, "7.5")); + root->setHTTPClient(cl); + + root->refresh(true); + waitForUpdateComplete(cl, root); + + pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1"); + SG_VERIFY(!cat->isEnabled()); + SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION); + + auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt"); + SG_VERIFY(altCat->isEnabled()); + + auto packs = root->allPackages(); + SG_CHECK_EQUAL(packs.size(), 4); + } + + // and now switch back to the older version + { + pkg::RootRef root(new pkg::Root(rootPath, "8.1.0")); + root->setHTTPClient(cl); + root->refresh(true); + waitForUpdateComplete(cl, root); + + pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1"); + SG_VERIFY(cat->isEnabled()); + SG_CHECK_EQUAL(cat->status(), pkg::Delegate::STATUS_REFRESHED); + + auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt"); + SG_VERIFY(!altCat->isEnabled()); + + // verify the original aircraft is still installed and available + pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG"); + SG_VERIFY(p1 != pkg::PackageRef()); + SG_CHECK_EQUAL(p1->id(), "b737-NG"); + SG_VERIFY(p1->isInstalled()); + + // verify the alt package is still installed, + pkg::PackageRef p2 = root->getPackageById("org.flightgear.test.catalog-alt.b737-NG"); + SG_VERIFY(p2 != pkg::PackageRef()); + SG_CHECK_EQUAL(p2->id(), "b737-NG"); + SG_VERIFY(p2->isInstalled()); + } +} + void testOfflineMode(HTTP::Client* cl) { global_catalogVersion = 0; @@ -990,6 +1114,8 @@ int main(int argc, char* argv[]) removeInvalidCatalog(&cl); + testVersionMigrateToId(&cl); + SG_LOG(SG_GENERAL, SG_INFO, "Successfully passed all tests!"); return EXIT_SUCCESS; } diff --git a/simgear/package/Root.cxx b/simgear/package/Root.cxx index b207af2c..ac19af07 100644 --- a/simgear/package/Root.cxx +++ b/simgear/package/Root.cxx @@ -414,7 +414,7 @@ Root::Root(const SGPath& aPath, const std::string& aVersion) : if (cat && cat->isEnabled()) { d->catalogs.insert({cat->id(), cat}); } else if (cat) { - SG_LOG(SG_GENERAL, SG_WARN, "Package-Root init: catalog is disabled: " << cat->id()); + SG_LOG(SG_GENERAL, SG_DEBUG, "Package-Root init: catalog is disabled: " << cat->id()); } } // of child directories iteration } @@ -450,6 +450,17 @@ CatalogRef Root::getCatalogById(const std::string& aId) const return it->second; } +CatalogRef Root::getCatalogByUrl(const std::string& aUrl) const +{ + auto it = std::find_if(d->catalogs.begin(), d->catalogs.end(), + [aUrl](const CatalogDict::value_type& v) + { return (v.second->url() == aUrl); }); + if (it == d->catalogs.end()) + return {}; + + return it->second; +} + PackageRef Root::getPackageById(const std::string& aName) const { size_t lastDot = aName.rfind('.'); diff --git a/simgear/package/Root.hxx b/simgear/package/Root.hxx index 70461915..2c598166 100644 --- a/simgear/package/Root.hxx +++ b/simgear/package/Root.hxx @@ -132,6 +132,8 @@ public: CatalogRef getCatalogById(const std::string& aId) const; + CatalogRef getCatalogByUrl(const std::string& aUrl) const; + void scheduleToUpdate(InstallRef aInstall); /** diff --git a/simgear/package/catalogTest1/catalog-alt.xml b/simgear/package/catalogTest1/catalog-alt.xml new file mode 100644 index 00000000..54d2edc2 --- /dev/null +++ b/simgear/package/catalogTest1/catalog-alt.xml @@ -0,0 +1,141 @@ + + + + org.flightgear.test.catalog-alt + Alternate test catalog + http://localhost:2000/catalogTest1/catalog-alt.xml + 4 + + 7.* + + + + alpha + Alpha package + 9 + 593 + + a469c4b837f0521db48616cfe65ac1ea + http://localhost:2000/catalogTest1/alpha.zip + + alpha + + + + c172p + Cessna 172-P + c172p + A plane made by Cessna + 42 + 860 + + cessna + ga + piston + ifr + + + 3 + 4 + 5 + 4 + + + + + exterior + thumb-exterior.png + http://foo.bar.com/thumb-exterior.png + + + + panel + thumb-panel.png + http://foo.bar.com/thumb-panel.png + + + + thumb-something.png + http://foo.bar.com/thumb-something.png + + + + c172p-2d-panel + C172 with 2d panel only + + + + c172p-floats + C172 with floats + + + exterior + thumb-exterior-floats.png + http://foo.bar.com/thumb-exterior-floats.png + + + + panel + thumb-panel.png + http://foo.bar.com/thumb-panel.png + + + + + c172p-skis + C172 with skis + + + exterior + thumb-exterior-skis.png + http://foo.bar.com/thumb-exterior-skis.png + + + + panel + thumb-panel.png + http://foo.bar.com/thumb-panel.png + + + + ec0e2ffdf98d6a5c05c77445e5447ff5 + http://localhost:2000/catalogTest1/c172p.zip + + + + + b737-NG + Boeing 737 NG + b737NG + A popular twin-engined narrow body jet + 112 + 860 + + boeing + jet + ifr + + + 5 + 5 + 4 + 4 + + + a94ca5704f305b90767f40617d194ed6 + http://localhost:2000/catalogTest1/b737.tar.gz + + + + + dc3 + DC-3 + 9 + 593 + + a469c4b837f0521db48616cfe65ac1ea + http://localhost:2000/catalogTest1/alpha.zip + + dc3 + + diff --git a/simgear/package/catalogTest1/catalog-v2.xml b/simgear/package/catalogTest1/catalog-v2.xml index 75a19c57..fa416115 100644 --- a/simgear/package/catalogTest1/catalog-v2.xml +++ b/simgear/package/catalogTest1/catalog-v2.xml @@ -15,6 +15,12 @@ http://localhost:2000/catalogTest1/catalog-v10.xml + + 7.* + org.flightgear.test.catalog-alt + http://localhost:2000/catalogTest1/catalog-alt.xml + + alpha Alpha package From 14354090f99ec603947c10767bf1d48c268e52e3 Mon Sep 17 00:00:00 2001 From: Stuart Buchanan Date: Thu, 18 Oct 2018 22:28:39 +0100 Subject: [PATCH 2/5] Don't check for cloud movement every frame Under Basic Weather, the cloudfield is finite size and clouds are shifted as the viewpoint changes. Previously each cloud was checked every frame to determine if it should be shifted. Not this only occurs if the viewpoint has moved a non-trivial distance. Note that this is separate from the clouds moving due to the wind. --- simgear/scene/sky/cloudfield.cxx | 108 +++++++++++++++---------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/simgear/scene/sky/cloudfield.cxx b/simgear/scene/sky/cloudfield.cxx index 13ee3030..0b313920 100644 --- a/simgear/scene/sky/cloudfield.cxx +++ b/simgear/scene/sky/cloudfield.cxx @@ -1,4 +1,4 @@ -// a layer of 3d clouds + // a layer of 3d clouds // // Written by Harald JOHNSEN, started April 2005. // @@ -78,56 +78,57 @@ bool SGCloudField::reposition( const SGVec3f& p, const SGVec3f& up, double lon, SGVec3 cart; SGGeod new_pos = SGGeod::fromRadFt(lon, lat, 0.0f); - + SGGeodesy::SGGeodToCart(new_pos, cart); osg::Vec3f osg_pos = toOsg(cart); osg::Quat orient = toOsg(SGQuatd::fromLonLatRad(lon, lat) * SGQuatd::fromRealImag(0, SGVec3d(0, 1, 0))); - + // Always update the altitude transform, as this allows // the clouds to rise and fall smoothly depending on environment updates. altitude_transform->setPosition(osg::Vec3f(0.0f, 0.0f, (float) asl)); - + // Similarly, always determine the effects of the wind osg::Vec3f wind = osg::Vec3f(-cos((direction + 180)* SGD_DEGREES_TO_RADIANS) * speed * dt, sin((direction + 180)* SGD_DEGREES_TO_RADIANS) * speed * dt, 0.0f); osg::Vec3f windosg = field_transform->getAttitude() * wind; field_transform->setPosition(field_transform->getPosition() + windosg); - + if (!wrap) { // If we're not wrapping the cloudfield, then we make no effort to reposition return false; } - + if ((old_pos - osg_pos).length() > fieldSize*2) { // Big movement - reposition centered to current location. field_transform->setPosition(osg_pos); field_transform->setAttitude(orient); old_pos = osg_pos; - } else { + } else if ((old_pos - osg_pos).length() > fieldSize*0.1) { + // Smaller, but non-trivial movement - check if any clouds need to be moved osg::Quat fta = field_transform->getAttitude(); osg::Quat ftainv = field_transform->getAttitude().inverse(); - + // delta is the vector from the old position to the new position in cloud-coords // osg::Vec3f delta = ftainv * (osg_pos - old_pos); old_pos = osg_pos; - + // Check if any of the clouds should be moved. for(CloudHash::const_iterator itr = cloud_hash.begin(), end = cloud_hash.end(); itr != end; ++itr) { - + osg::ref_ptr pat = itr->second; - + if (pat == 0) { continue; } - + osg::Vec3f currpos = field_transform->getPosition() + fta * pat->getPosition(); - + // Determine the vector from the new position to the cloud in cloud-space. - osg::Vec3f w = ftainv * (currpos - toOsg(cart)); - + osg::Vec3f w = ftainv * (currpos - toOsg(cart)); + // Determine a course if required. Note that this involves some axis translation. float x = 0.0; float y = 0.0; @@ -135,16 +136,16 @@ bool SGCloudField::reposition( const SGVec3f& p, const SGVec3f& up, double lon, if (w.x() < -0.6*fieldSize) { y = -fieldSize; } if (w.y() > 0.6*fieldSize) { x = -fieldSize; } if (w.y() < -0.6*fieldSize) { x = fieldSize; } - + if ((x != 0.0) || (y != 0.0)) { removeCloudFromTree(pat); - SGGeod p = SGGeod::fromCart(toSG(field_transform->getPosition() + + SGGeod p = SGGeod::fromCart(toSG(field_transform->getPosition() + fta * pat->getPosition())); - addCloudToTree(pat, p, x, y); + addCloudToTree(pat, p, x, y); } - } + } } - + // Render the clouds in order from farthest away layer to nearest one. field_root->getStateSet()->setRenderBinDetails(CLOUDS_BIN, "DepthSortedBin"); return true; @@ -161,7 +162,7 @@ SGCloudField::SGCloudField() : osg::StateSet *rootSet = field_root->getOrCreateStateSet(); rootSet->setRenderBinDetails(CLOUDS_BIN, "DepthSortedBin"); rootSet->setAttributeAndModes(getFog()); - + field_transform->addChild(altitude_transform.get()); placed_root = new osg::Group(); altitude_transform->addChild(placed_root); @@ -169,7 +170,7 @@ SGCloudField::SGCloudField() : lodcount = 0; cloudcount = 0; } - + SGCloudField::~SGCloudField() { } @@ -181,7 +182,7 @@ void SGCloudField::clear(void) { ++itr) { removeCloudFromTree(itr->second); } - + cloud_hash.clear(); } @@ -198,7 +199,7 @@ void SGCloudField::applyVisAndLoDRange(void) } } } - + bool SGCloudField::addCloud(float lon, float lat, float alt, int index, osg::ref_ptr geode) { return addCloud(lon, lat, alt, 0.0f, 0.0f, index, geode); } @@ -244,34 +245,34 @@ void SGCloudField::removeCloudFromTree(osg::ref_ptr transform, float lon, float lat, float alt, float x, float y) { - + // Get the base position SGGeod loc = SGGeod::fromDegFt(lon, lat, alt); - addCloudToTree(transform, loc, x, y); + addCloudToTree(transform, loc, x, y); } void SGCloudField::addCloudToTree(osg::ref_ptr transform, SGGeod loc, float x, float y) { - + float alt = loc.getElevationFt(); // Determine any shift by x/y if ((x != 0.0f) || (y != 0.0f)) { - double crs = 90.0 - SG_RADIANS_TO_DEGREES * atan2(y, x); + double crs = 90.0 - SG_RADIANS_TO_DEGREES * atan2(y, x); double dst = sqrt(x*x + y*y); double endcrs; - - SGGeod base_pos = SGGeod::fromGeodFt(loc, 0.0f); + + SGGeod base_pos = SGGeod::fromGeodFt(loc, 0.0f); SGGeodesy::direct(base_pos, crs, dst, loc, endcrs); } - - // The direct call provides the position at 0 alt, so adjust as required. - loc.setElevationFt(alt); - addCloudToTree(transform, loc); + + // The direct call provides the position at 0 alt, so adjust as required. + loc.setElevationFt(alt); + addCloudToTree(transform, loc); } - - -void SGCloudField::addCloudToTree(osg::ref_ptr transform, SGGeod loc) { + + +void SGCloudField::addCloudToTree(osg::ref_ptr transform, SGGeod loc) { // Work out where this cloud should go in OSG coordinates. SGVec3 cart; SGGeodesy::SGGeodToCart(loc, cart); @@ -285,17 +286,17 @@ void SGCloudField::addCloudToTree(osg::ref_ptr t // Convert to the scenegraph orientation where we just rotate around // the y axis 180 degrees. osg::Quat orient = toOsg(SGQuatd::fromLonLatDeg(loc.getLongitudeDeg(), loc.getLatitudeDeg()) * SGQuatd::fromRealImag(0, SGVec3d(0, 1, 0))); - - osg::Vec3f osg_pos = toOsg(fieldcenter); - + + osg::Vec3f osg_pos = toOsg(fieldcenter); + field_transform->setPosition(osg_pos); field_transform->setAttitude(orient); old_pos = osg_pos; } - + // Convert position to cloud-coordinates pos = pos - field_transform->getPosition(); - pos = field_transform->getAttitude().inverse() * pos; + pos = field_transform->getAttitude().inverse() * pos; // We have a two level dynamic quad tree which the cloud will be added // to. If there are no appropriate nodes in the quad tree, they are @@ -310,7 +311,7 @@ void SGCloudField::addCloudToTree(osg::ref_ptr t if ((lodnode1->getCenter() - pos).length2() < lod1_range*lod1_range) { // New cloud is within RADIUS_LEVEL_1 of the center of the LOD node. found = true; - } + } } if (!found) { @@ -318,7 +319,7 @@ void SGCloudField::addCloudToTree(osg::ref_ptr t impostornode = new osgSim::Impostor(); impostornode->setImpostorThreshold(impostor_distance); //impostornode->setImpostorThresholdToBound(); - //impostornode->setCenter(pos); + //impostornode->setCenter(pos); placed_root->addChild(impostornode.get()); lodnode1 = (osg::ref_ptr) impostornode; } else { @@ -330,8 +331,8 @@ void SGCloudField::addCloudToTree(osg::ref_ptr t // Now check if there is a second level LOD node at an appropriate distance found = false; - - for (unsigned int j = 0; (!found) && (j < lodnode1->getNumChildren()); j++) { + + for (unsigned int j = 0; (!found) && (j < lodnode1->getNumChildren()); j++) { lodnode = (osg::LOD*) lodnode1->getChild(j); if ((lodnode->getCenter() - pos).length2() < lod2_range*lod2_range) { // We've found the right leaf LOD node @@ -344,37 +345,37 @@ void SGCloudField::addCloudToTree(osg::ref_ptr t lodnode = new osg::LOD(); lodnode1->addChild(lodnode, 0.0f, lod1_range + lod2_range + view_distance + MAX_CLOUD_DEPTH); lodcount++; - } - + } + transform->setPosition(pos); lodnode->addChild(transform.get(), 0.0f, view_distance + MAX_CLOUD_DEPTH); cloudcount++; SG_LOG(SG_ENVIRONMENT, SG_DEBUG, "Impostors: " << impostorcount << - " LoD: " << lodcount << + " LoD: " << lodcount << " Clouds: " << cloudcount); lodnode->dirtyBound(); lodnode1->dirtyBound(); field_root->dirtyBound(); } - + bool SGCloudField::deleteCloud(int identifier) { osg::ref_ptr transform = cloud_hash[identifier]; if (transform == 0) return false; - + removeCloudFromTree(transform); cloud_hash.erase(identifier); return true; } - + bool SGCloudField::repositionCloud(int identifier, float lon, float lat, float alt) { return repositionCloud(identifier, lon, lat, alt, 0.0f, 0.0f); } bool SGCloudField::repositionCloud(int identifier, float lon, float lat, float alt, float x, float y) { osg::ref_ptr transform = cloud_hash[identifier]; - + if (transform == NULL) return false; removeCloudFromTree(transform); @@ -398,4 +399,3 @@ void SGCloudField::updateFog(double visibility, const osg::Vec4f& color) { fog->setColor(color); fog->setDensity(sqrt_m_log01 / visibility); } - From f05ff37560dd4ec4824554e1291f74876890736d Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 23 Oct 2018 15:29:31 +0100 Subject: [PATCH 3/5] Fix for assert with empty systems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty subsystem groups didn’t set their init state correctly, leading to an assert on post-init. Fix this and add a test for it. https://sourceforge.net/p/flightgear/codetickets/2043/ --- simgear/structure/subsystem_mgr.cxx | 1 + simgear/structure/subsystem_test.cxx | 29 +++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index 537649e2..a7294f5f 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -253,6 +253,7 @@ SGSubsystemGroup::incrementalInit() { // special case this, simplifies the logic below if (_members.empty()) { + _state = State::INIT; return INIT_DONE; } diff --git a/simgear/structure/subsystem_test.cxx b/simgear/structure/subsystem_test.cxx index a1136a78..219a3244 100644 --- a/simgear/structure/subsystem_test.cxx +++ b/simgear/structure/subsystem_test.cxx @@ -314,6 +314,7 @@ void testIncrementalInit() instruments->set_subsystem(radio1); instruments->set_subsystem(radio2); + manager->bind(); for ( ; ; ) { auto status = manager->incrementalInit(); @@ -338,9 +339,34 @@ void testIncrementalInit() SG_VERIFY(d->hasEvent("fake-radio.nav2-will-init")); SG_VERIFY(d->hasEvent("fake-radio.nav2-did-init")); +} +void testEmptyGroup() +{ + // testing the assert described here: + // https://sourceforge.net/p/flightgear/codetickets/2043/ + // when an empty group is inited, we skipped setting the state + + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + auto mySub = manager->add(SGSubsystemMgr::POST_FDM); + auto anotherSub = manager->add(SGSubsystemMgr::POST_FDM); + auto instruments = manager->add(SGSubsystemMgr::POST_FDM); + + manager->bind(); + for ( ; ; ) { + auto status = manager->incrementalInit(); + if (status == SGSubsystemMgr::INIT_DONE) + break; + } + manager->postinit(); + + SG_VERIFY(mySub->wasInited); - + SG_VERIFY(d->hasEvent("instruments-will-init")); + SG_VERIFY(d->hasEvent("instruments-did-init")); } void testSuspendResume() @@ -520,6 +546,7 @@ int main(int argc, char* argv[]) testSuspendResume(); testPropertyRoot(); testAddRemoveAfterInit(); + testEmptyGroup(); cout << __FILE__ << ": All tests passed" << endl; return EXIT_SUCCESS; From 758ab5e5812d6ee14f6361d6882cb6867b6cc13c Mon Sep 17 00:00:00 2001 From: Richard Harrison Date: Tue, 30 Oct 2018 20:12:12 +0100 Subject: [PATCH 4/5] LOD ranges The scenery ranges for bare and rough are now deltas, to avoid overlapping values by user error. --- simgear/scene/tgdb/ReaderWriterSTG.cxx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/simgear/scene/tgdb/ReaderWriterSTG.cxx b/simgear/scene/tgdb/ReaderWriterSTG.cxx index dbd1f991..0418d710 100644 --- a/simgear/scene/tgdb/ReaderWriterSTG.cxx +++ b/simgear/scene/tgdb/ReaderWriterSTG.cxx @@ -313,10 +313,17 @@ struct ReaderWriterSTG::_ModelBin { return false; } + // starting with 2018.3 we will use deltas rather than absolutes as it is more intuitive for the user + // and somewhat easier to visualise + + double detailedRange = atof(options->getPluginStringData("SimGear::LOD_RANGE_DETAILED").c_str()); + double bareRangeDelta = atof(options->getPluginStringData("SimGear::LOD_RANGE_BARE").c_str()); + double roughRangeDelta = atof(options->getPluginStringData("SimGear::LOD_RANGE_ROUGH").c_str()); + // Determine object ranges. Mesh size of 2000mx2000m needs to be accounted for. - _object_range_bare = 1414.0f + atof(options->getPluginStringData("SimGear::LOD_RANGE_BARE").c_str()); - _object_range_rough = 1414.0f + atof(options->getPluginStringData("SimGear::LOD_RANGE_ROUGH").c_str()); - _object_range_detailed = 1414.0f + atof(options->getPluginStringData("SimGear::LOD_RANGE_DETAILED").c_str()); + _object_range_detailed = 1414.0f + detailedRange; + _object_range_bare = _object_range_detailed + bareRangeDelta; + _object_range_rough = _object_range_detailed + roughRangeDelta; SG_LOG(SG_TERRAIN, SG_INFO, "Loading stg file " << absoluteFileName); From 61cc8d6c506ca2b9b59bb04a31568dcd3ae9bc7b Mon Sep 17 00:00:00 2001 From: Richard Harrison Date: Tue, 30 Oct 2018 20:13:32 +0100 Subject: [PATCH 5/5] Fix exception when number of primitive sets not what assumed. This was something that happened when random vegetation was off, but tree shadows was on. Adding random vegetation would then reliably cause an exception. --- simgear/scene/tgdb/TreeBin.cxx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/simgear/scene/tgdb/TreeBin.cxx b/simgear/scene/tgdb/TreeBin.cxx index ee8f4b39..194a15be 100644 --- a/simgear/scene/tgdb/TreeBin.cxx +++ b/simgear/scene/tgdb/TreeBin.cxx @@ -233,9 +233,11 @@ void addTreeToLeafGeode(Geode* geode, const SGVec3f& p, const SGVec3f& t) int imax = 2; if (use_tree_shadows) { imax = 3; } for (int i = 0; i < imax; ++i) { - DrawArrays* primSet = static_cast(geom->getPrimitiveSet(i)); - if(primSet != nullptr) - primSet->setCount(numVerts); + if (i < geom->getNumPrimitiveSets()) { + DrawArrays* primSet = static_cast(geom->getPrimitiveSet(i)); + if (primSet != nullptr) + primSet->setCount(numVerts); + } } } }