Optimize MapCropper (#5701)

* Ignore read cropping on JSON based outputs
* Bulk remove ways in OsmMap
* Member function argument description comment
* Copyright

Author: bmarchant

hoot-core/src/main/cpp/hoot/core/elements/OsmMap.cpp

@@ -651,6 +651,38 @@ void OsmMap::replaceNodes(const std::map<long, long>& replacements)
   }
 }
 
+void OsmMap::bulkRemoveWays(const std::vector<long>& 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<ElementToRelationMap> 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<long>& 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<OGRSpatialReference>& srs)
 {
   _srs = srs;

hoot-core/src/main/cpp/hoot/core/elements/OsmMap.h

@@ -213,6 +213,15 @@ class OsmMap : public std::enable_shared_from_this<OsmMap>, 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<long>& way_ids, bool removeFully);
+
   ////////////////////////////////////////RELATION/////////////////////////////////////////////////
 
   ConstRelationPtr getRelation(long id) const override;

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());

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())

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
    *

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));

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<long> waysToRemove;
+  vector<long> waysToRemoveFully;
+  vector<long> 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");
 

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))
   {

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]]]]}}
 ]
 }

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]]]}}
 ]
 }