v0.2.47..v0.2.48 changeset BuildingOutlineUpdateOp.cpp

diff --git a/hoot-core/src/main/cpp/hoot/core/ops/BuildingOutlineUpdateOp.cpp b/hoot-core/src/main/cpp/hoot/core/ops/BuildingOutlineUpdateOp.cpp
index 7772ba5..c1b8cef 100644
--- a/hoot-core/src/main/cpp/hoot/core/ops/BuildingOutlineUpdateOp.cpp
+++ b/hoot-core/src/main/cpp/hoot/core/ops/BuildingOutlineUpdateOp.cpp
@@ -170,17 +170,17 @@ void BuildingOutlineUpdateOp::_deleteBuildingRelations()
   }
 }
 
-void BuildingOutlineUpdateOp::_unionOutline(const RelationPtr& pBuilding, const ElementPtr& pElement, std::shared_ptr<Geometry>& pOutline)
+void BuildingOutlineUpdateOp::_unionOutline(const RelationPtr& pBuilding,
+                                            const ElementPtr& pElement,
+                                            std::shared_ptr<Geometry>& pOutline)
 {
   ElementConverter elementConverter = ElementConverter(_map);
   std::shared_ptr<Geometry> pGeometry;
-
   try
   {
     if (pElement->getElementType() == ElementType::Way)
     {
       const WayPtr& pWay = std::dynamic_pointer_cast<Way>(pElement);
-
       if (pWay->isClosedArea())
       {
         pGeometry = elementConverter.convertToPolygon(pWay);
@@ -242,8 +242,7 @@ void BuildingOutlineUpdateOp::_createOutline(const RelationPtr& pBuilding)
       pBuilding->removeElement(entries[i].role, entries[i].getElementId());
     }
     else if (entries[i].role == MetadataTags::RolePart() ||
-             (considerOuterRoleAsPart && entries[i].role == MetadataTags::RoleOuter())
-            )
+             (considerOuterRoleAsPart && entries[i].role == MetadataTags::RoleOuter()))
     {
       if (entries[i].getElementId().getType() == ElementType::Way)
       {
@@ -285,70 +284,63 @@ void BuildingOutlineUpdateOp::_createOutline(const RelationPtr& pBuilding)
 
   if (!outline->isEmpty())
   {
-    LOG_TRACE("Adding building outline element...");
+    LOG_TRACE("Creating building outline element...");
 
     const std::shared_ptr<Element> pOutlineElement =
       GeometryConverter(_map).convertGeometryToElement(
         outline.get(), pBuilding->getStatus(), pBuilding->getCircularError());
-
     _mergeNodes(pOutlineElement, pBuilding);
 
-    LOG_VART(pOutlineElement);
-
     if (_removeBuildingRelations)
     {      
-      // only copy tags to the outline element if we are removing the building relations
-      pOutlineElement->setTags(pBuilding->getTags());;
-
+      // Only copy tags to the outline element if we are removing the building relations.
+      pOutlineElement->setTags(pBuilding->getTags());
       // We don't need the relation "type" tag.
       pOutlineElement->getTags().remove("type");
 
       LOG_TRACE("Marking building: " << pBuilding->getElementId() << " for deletion...");
       _buildingRelationIds.insert(pBuilding->getElementId());
     }
-    else
+    // Never add outlines to multipoly relations, as it triggers JOSM errors.
+    else if (pBuilding->getType() != MetadataTags::RelationMultiPolygon())
     {
       Tags buildingTags = pBuilding->getTags();
-
-      // to preserve naming of relation buildings in JOSM we copy the building's "building" and "name" tags
+      // To preserve naming of relation buildings in JOSM we copy the building's "building" and
+      // "name" tags.
       if (buildingTags.contains("name") && buildingTags.contains("building") )
       {
-        pOutlineElement->setTag( "name", buildingTags["name"] );
-        pOutlineElement->setTag( "building", buildingTags["building"] );
+        pOutlineElement->setTag("name", buildingTags["name"]);
+        pOutlineElement->setTag("building", buildingTags["building"]);
       }
-
+      LOG_TRACE("Adding building outline element " << pOutlineElement->getElementId());
       pBuilding->addElement(MetadataTags::RoleOutline(), pOutlineElement);
     }
 
-    // find outline ways that are exact duplicates of the original building ways
-    vector<long> removeWayIds;
-
-    if (pOutlineElement->getElementType() == ElementType::Way)
+    // If we are removing the building relations and only keeping the multipoly relations, find
+    // multipoly ways that are the same as the original building ways and copy the tags from the
+    // building ways over to them. Don't do this if we are keeping the both the building and
+    // multipoly relations, as we'll end up with duplicated way errors in JOSM.
+    if (_removeBuildingRelations)
     {
-      ConstWayPtr pOutlineWay = std::dynamic_pointer_cast<const Way>(pOutlineElement);
-      _findOutlineDuplicate(pOutlineWay, buildingWayLookup, removeWayIds, pBuilding);
-    }
-    else if (pOutlineElement->getElementType() == ElementType::Relation)
-    {  
-      const RelationPtr pOutlineRelation = std::dynamic_pointer_cast<Relation>(pOutlineElement);
-
-      foreach (RelationData::Entry outlineEntry, pOutlineRelation->getMembers())
+      if (pOutlineElement->getElementType() == ElementType::Way)
       {
-        ElementPtr pOutlineMember = _map->getElement(outlineEntry.getElementId());
-
-        if (pOutlineMember->getElementType() == ElementType::Way)
+        WayPtr pOutlineWay = std::dynamic_pointer_cast<Way>(pOutlineElement);
+        _updateMultipolyWayMembers(pOutlineWay, buildingWayLookup);
+      }
+      else if (pOutlineElement->getElementType() == ElementType::Relation)
+      {
+        const RelationPtr pOutlineRelation = std::dynamic_pointer_cast<Relation>(pOutlineElement);
+        foreach (RelationData::Entry outlineEntry, pOutlineRelation->getMembers())
         {
-          ConstWayPtr pOutlineWay = std::dynamic_pointer_cast<const Way>(pOutlineMember);
-          _findOutlineDuplicate(pOutlineWay, buildingWayLookup, removeWayIds, pOutlineRelation);
+          ElementPtr pOutlineMember = _map->getElement(outlineEntry.getElementId());
+          if (pOutlineMember->getElementType() == ElementType::Way)
+          {
+            WayPtr pOutlineWay = std::dynamic_pointer_cast<Way>(pOutlineMember);
+            _updateMultipolyWayMembers(pOutlineWay, buildingWayLookup);
+          }
         }
       }
     }
-
-    // remove replaced outline ways
-    foreach (long id, removeWayIds)
-    {
-      RemoveWayByEid::removeWayFully(_map, id);
-    }
   }
   else
   {
@@ -358,11 +350,8 @@ void BuildingOutlineUpdateOp::_createOutline(const RelationPtr& pBuilding)
   LOG_TRACE("Output building: " << pBuilding);
 }
 
-void BuildingOutlineUpdateOp::_findOutlineDuplicate( ConstWayPtr& pOutlineWay,
-                                                     QHash<RelationData::Entry,WayPtr>& buildingWayLookup,
-                                                     vector<long>& removeWayIds,
-                                                     const RelationPtr& pOutlineHost
-                                                   )
+void BuildingOutlineUpdateOp::_updateMultipolyWayMembers(
+  WayPtr& pOutlineWay, QHash<RelationData::Entry, WayPtr>& buildingWayLookup)
 {
   // see if it's a duplicate of any building
   foreach (WayPtr pBuildingWay, buildingWayLookup)
@@ -371,18 +360,19 @@ void BuildingOutlineUpdateOp::_findOutlineDuplicate( ConstWayPtr& pOutlineWay,
     {
       vector<long> sourceNodes = pBuildingWay->getNodeIds();
       vector<long> wayNodes = pOutlineWay->getNodeIds();
-
       if (sourceNodes == wayNodes)
       {
-        // replace the outline way with the building way and mark the outline way for removal
-        removeWayIds.push_back(pOutlineWay->getId());
-        pOutlineHost->replaceElement(pOutlineWay, pBuildingWay);
+        // Copy all the tags from the building part ways we already updated to the multipoly way
+        // members.
+        pOutlineWay->setTags(pBuildingWay->getTags());
+        // Not certain whether the building part tag should get copied here or not...copying it
+        // for now.
+        //pOutlineWay->getTags().remove(MetadataTags::BuildingPart());
       }
     }
   }
 }
 
-
 void BuildingOutlineUpdateOp::_mergeNodes(const std::shared_ptr<Element>& changed,
   const RelationPtr& reference)
 {