diff --git a/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.cpp b/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.cpp index d02934ac89..42db5b381b 100644 --- a/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.cpp +++ b/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.cpp @@ -651,6 +651,38 @@ void OsmMap::replaceNodes(const std::map& replacements) } } +void OsmMap::bulkRemoveWays(const std::vector& way_ids, bool removeFully) +{ + // Get a pointer to the element to relation map and then clear the index before working with the index again, + // this will keep the index from being rebuilt with each removal of a way. Rebuild it once at the end of the + // function + std::shared_ptr relationMap = _index->getElementToRelationMap(); + if (removeFully) + _index->clearRelationIndex(); + + // Remove all of the ways + for (auto& id : way_ids) + { + ElementId eid = ElementId::way(id); + if (removeFully) + { + // Update all relations to remove references to this way + const set& relations = relationMap->getRelationByElement(eid); + for (const auto& r : _relations) + { + if (relations.find(r.first) != relations.end()) + r.second->removeElement(eid); + } + } + // Actually remove the way from the map and the index + _index->removeWay(getWay(id)); + _ways.erase(id); + } + // Rebuild the relation map only one time + if (removeFully) + _index->getElementToRelationMap(); +} + void OsmMap::setProjection(const std::shared_ptr& srs) { _srs = srs; diff --git a/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.h b/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.h index 4c8cc51bd2..c1db2a8e83 100644 --- a/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.h +++ b/hoot-core/src/main/cpp/hoot/core/elements/OsmMap.h @@ -213,6 +213,15 @@ class OsmMap : public std::enable_shared_from_this, public ElementProvid int numWaysAppended() const { return _numWaysAppended; } int numWaysSkippedForAppending() const { return _numWaysSkippedForAppending; } + /** Removing multiple ways using other methods will trigger the indices in this object + * to be rebuilt between each delete operation which is expensive, this method will delete + * all of the ways in one shot and rebuild the indices afterwards. + * @param way_ids Vector of way IDs that are to be removed + * @param removeFully When set to true, way IDs are removed from relations in the map too + * when false, the way is removed from the map only, relations still reference the way ID + */ + void bulkRemoveWays(const std::vector& way_ids, bool removeFully); + ////////////////////////////////////////RELATION///////////////////////////////////////////////// ConstRelationPtr getRelation(long id) const override; diff --git a/hoot-core/src/main/cpp/hoot/core/io/DataConverter.cpp b/hoot-core/src/main/cpp/hoot/core/io/DataConverter.cpp index 6806fe9864..6b517b4506 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/DataConverter.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/DataConverter.cpp @@ -109,7 +109,11 @@ void DataConverter::convert(const QStringList& inputs, const QString& output) { _validateInput(inputs, output); - if (!IoUtils::areSupportedOgrFormats(inputs, true) && IoUtils::isSupportedOgrFormat(output)) + bool ogrInputs = IoUtils::areSupportedOgrFormats(inputs, true); + bool ogrOutput = IoUtils::isSupportedOgrFormat(output); + bool jsonOutput = IoUtils::isSupportedJsonFormat(output); + + if (!ogrInputs && (ogrOutput || jsonOutput)) _cropReadIfBounded = false; _progress.setJobId(ConfigOptions().getJobId()); diff --git a/hoot-core/src/main/cpp/hoot/core/io/IoUtils.cpp b/hoot-core/src/main/cpp/hoot/core/io/IoUtils.cpp index a330eccca5..7af8f21055 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/IoUtils.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/IoUtils.cpp @@ -106,6 +106,12 @@ bool IoUtils::isSupportedOgrFormat(const QString& input, const bool allowDir) } } +bool IoUtils::isSupportedJsonFormat(const QString& input) +{ + const QString inputLower = input.toLower(); + return inputLower.endsWith(".json") || inputLower.endsWith(".geojson"); +} + bool IoUtils::anyAreSupportedOgrFormats(const QStringList& inputs, const bool allowDir) { if (inputs.empty()) diff --git a/hoot-core/src/main/cpp/hoot/core/io/IoUtils.h b/hoot-core/src/main/cpp/hoot/core/io/IoUtils.h index 9a8991f6c8..71c063df39 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/IoUtils.h +++ b/hoot-core/src/main/cpp/hoot/core/io/IoUtils.h @@ -78,6 +78,13 @@ class IoUtils * @return true if the input is supported by OGR; false otherwise */ static bool isSupportedOgrFormat(const QString& input, const bool allowDir = false); + /** + * Returns true if the input format is a Hootenanny supported JSON format + * + * @param input input path + * @return true if the input is a type of JSON; false otherwise + */ + static bool isSupportedJsonFormat(const QString& input); /** * Determines if a set of inputs paths are all OGR supported formats * diff --git a/hoot-core/src/main/cpp/hoot/core/io/OsmGeoJsonWriter.cpp b/hoot-core/src/main/cpp/hoot/core/io/OsmGeoJsonWriter.cpp index d320c3416c..5dd3f9f54f 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/OsmGeoJsonWriter.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/OsmGeoJsonWriter.cpp @@ -314,6 +314,8 @@ void OsmGeoJsonWriter::_writePartial(const ElementProviderPtr& provider, const C QString OsmGeoJsonWriter::_getBbox() const { Envelope bounds = CalculateMapBoundsVisitor::getGeosBounds(_map); + if (_bounds) + bounds = *_bounds->getEnvelopeInternal(); return QString("[%1, %2, %3, %4]") .arg(QString::number(bounds.getMinX(), 'g', 5), QString::number(bounds.getMinY(), 'g', 5), QString::number(bounds.getMaxX(), 'g', 5), QString::number(bounds.getMaxY(), 'g', 5)); diff --git a/hoot-core/src/main/cpp/hoot/core/ops/MapCropper.cpp b/hoot-core/src/main/cpp/hoot/core/ops/MapCropper.cpp index 199bd7c994..f3a8a78102 100644 --- a/hoot-core/src/main/cpp/hoot/core/ops/MapCropper.cpp +++ b/hoot-core/src/main/cpp/hoot/core/ops/MapCropper.cpp @@ -22,7 +22,7 @@ * This will properly maintain the copyright information. Maxar * copyrights will be updated automatically. * - * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Maxar (http://www.maxar.com/) + * @copyright Copyright (C) 2015-2023 Maxar (http://www.maxar.com/) */ #include "MapCropper.h" @@ -190,6 +190,10 @@ void MapCropper::apply(OsmMapPtr& map) LOG_VARD(_bounds->toString()); LOG_VARD(_inclusionCrit.get()); + // First iteration finds the elements to delete and crop + vector waysToRemove; + vector waysToRemoveFully; + vector waysToCrop; // go through all the ways long wayCtr = 0; // Make a copy because the map is modified below @@ -253,9 +257,9 @@ void MapCropper::apply(OsmMapPtr& map) LOG_TRACE("Dropping wholly outside way: " << w->getElementId() << "..."); // Removal is based on the parent setting, either remove it fully or leave it in the relation if (_removeFromParentRelation) - RemoveWayByEid::removeWayFully(map, w->getId()); + waysToRemoveFully.emplace_back(w->getId()); else - RemoveWayByEid::removeWay(map, w->getId()); + waysToRemove.emplace_back(w->getId()); _numWaysOutOfBounds++; _numAffected++; } @@ -271,7 +275,7 @@ void MapCropper::apply(OsmMapPtr& map) { // Way isn't wholly inside and the configuration requires it to be, so remove the way. LOG_TRACE("Dropping due to _keepOnlyFeaturesInsideBounds=true: " << w->getElementId() << "..."); - RemoveWayByEid::removeWayFully(map, w->getId()); + waysToRemoveFully.emplace_back(w->getId()); _numWaysOutOfBounds++; _numAffected++; } @@ -280,7 +284,7 @@ void MapCropper::apply(OsmMapPtr& map) // Way crosses the boundary and we're not configured to keep ways that cross the bounds, so // do an expensive operation to decide how much to keep, if any. LOG_TRACE("Cropping due to _keepEntireFeaturesCrossingBounds=false: " << w->getElementId() << "..."); - _cropWay(map, w->getId()); + waysToCrop.emplace_back(w->getId()); _numWaysCrossingThreshold++; } else @@ -299,6 +303,17 @@ void MapCropper::apply(OsmMapPtr& map) StringUtils::formatLargeNumber(ways.size()) << " ways."); } } + + // Bulk remove ways from map and relations too + map->bulkRemoveWays(waysToRemoveFully, true); + + // Bulk remove ways from map only + map->bulkRemoveWays(waysToRemove, false); + + // Iterate the ways that cross the bounds and crop + for (auto id : waysToCrop) + _cropWay(map, id); + LOG_VARD(map->size()); OsmMapWriterFactory::writeDebugMap(map, className(), "after-way-removal"); diff --git a/hoot-core/src/main/cpp/hoot/core/visitors/SchemaTranslationVisitor.cpp b/hoot-core/src/main/cpp/hoot/core/visitors/SchemaTranslationVisitor.cpp index c0efd280f3..ad3c10fe26 100644 --- a/hoot-core/src/main/cpp/hoot/core/visitors/SchemaTranslationVisitor.cpp +++ b/hoot-core/src/main/cpp/hoot/core/visitors/SchemaTranslationVisitor.cpp @@ -22,7 +22,7 @@ * This will properly maintain the copyright information. Maxar * copyrights will be updated automatically. * - * @copyright Copyright (C) 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/) + * @copyright Copyright (C) 2015-2023 Maxar (http://www.maxar.com/) */ #include "SchemaTranslationVisitor.h" @@ -95,6 +95,10 @@ void SchemaTranslationVisitor::setTranslationScript(QString path) void SchemaTranslationVisitor::visit(const ElementPtr& e) { + // Don't attempt translation without a translator + if (!_translator) + return; + // Filter the elements if (e.get() && e->getTags().getNonDebugCount() > 0 && (_elementStatusFilter == Status::Invalid || e->getStatus() == _elementStatusFilter)) { diff --git a/test-files/io/GeoJson/CroppingTest/BUILDING_S.geojson b/test-files/io/GeoJson/CroppingTest/BUILDING_S.geojson index 2dde0b4c18..bb37c21ef5 100644 --- a/test-files/io/GeoJson/CroppingTest/BUILDING_S.geojson +++ b/test-files/io/GeoJson/CroppingTest/BUILDING_S.geojson @@ -1,4 +1,4 @@ -{"generator":"Hootenanny","type":"FeatureCollection","bbox":[-76.621, 39.284, -76.614, 39.287],"features": [ +{"generator":"Hootenanny","type":"FeatureCollection","bbox":[-76.62, 39.286, -76.615, 39.287],"features": [ {"type":"Feature","properties":{"ARA":"41183.0","FCSUBTYPE":"100083","F_CODE":"AL013","ZI005_FNA":"Building Test","ZI026_CTUC":"5"},"geometry": {"type":"MultiPolygon","coordinates": [[[[-76.6190305379, 39.2860467208],[-76.61874622374999, 39.28605710088],[-76.61875695258, 39.28620449776],[-76.61806762486999, 39.28623148588],[-76.61805421382, 39.28609446913],[-76.61762506038001, 39.28611107723],[-76.61760888352043, 39.28582831499],[-76.61901334434501, 39.28582831499],[-76.6190305379, 39.2860467208]]],[[[-76.61730051309, 39.28602388464],[-76.61600232393, 39.28606955696],[-76.61602914602, 39.28634151429],[-76.61542296678, 39.28636435036],[-76.61538129093391, 39.28582831499],[-76.61728816009655, 39.28582831499],[-76.61730051309, 39.28602388464]]]]}} ] } diff --git a/test-files/io/GeoJson/CroppingTest/ROAD_C.geojson b/test-files/io/GeoJson/CroppingTest/ROAD_C.geojson index ce46836952..dfcf58545f 100644 --- a/test-files/io/GeoJson/CroppingTest/ROAD_C.geojson +++ b/test-files/io/GeoJson/CroppingTest/ROAD_C.geojson @@ -1,4 +1,4 @@ -{"generator":"Hootenanny","type":"FeatureCollection","bbox":[-76.621, 39.284, -76.614, 39.287],"features": [ +{"generator":"Hootenanny","type":"FeatureCollection","bbox":[-76.62, 39.286, -76.615, 39.287],"features": [ {"type":"Feature","properties":{"FCSUBTYPE":"100152","F_CODE":"AP030","LZN":"1118.6","OSMTAGS":{"highway":"primary"},"RIN_ROI":"3","RMWC":"5","RTY":"3","SGCC":"5","ZI005_FNA":"Highway Test","ZI016_WTC":"1","ZI026_CTUC":"5"},"geometry": {"type":"MultiLineString","coordinates": [[[-76.61972935029, 39.28622613963066],[-76.61925316929999, 39.28625017619],[-76.61922038510578, 39.28582831499]],[[-76.6148605218813, 39.28582831499],[-76.61492944837001, 39.28646608078],[-76.61460649762, 39.28648516172767]]]}} ] }