v0.2.54..v0.2.55 changeset HighwaySnapMerger.cpp
Garret Voltz edited this page Aug 14, 2020
·
1 revision
diff --git a/hoot-core/src/main/cpp/hoot/core/conflate/highway/HighwaySnapMerger.cpp b/hoot-core/src/main/cpp/hoot/core/conflate/highway/HighwaySnapMerger.cpp
index d4d92a9..ad58440 100644
--- a/hoot-core/src/main/cpp/hoot/core/conflate/highway/HighwaySnapMerger.cpp
+++ b/hoot-core/src/main/cpp/hoot/core/conflate/highway/HighwaySnapMerger.cpp
@@ -58,6 +58,11 @@
#include <hoot/core/util/Validate.h>
#include <hoot/core/visitors/ElementOsmMapVisitor.h>
#include <hoot/core/visitors/WaysVisitor.h>
+#include <hoot/core/conflate/merging/WayNodeCopier.h>
+#include <hoot/core/criterion/NotCriterion.h>
+#include <hoot/core/criterion/NoInformationCriterion.h>
+#include <hoot/core/util/ConfigOptions.h>
+#include <hoot/core/util/Settings.h>
// Qt
#include <QSet>
@@ -179,8 +184,16 @@ bool HighwaySnapMerger::_directConnect(const ConstOsmMapPtr& map, WayPtr w) cons
bool HighwaySnapMerger::_doesWayConnect(long node1, long node2, const ConstWayPtr& w) const
{
- return (w->getNodeId(0) == node1 && w->getLastNodeId() == node2) ||
- (w->getNodeId(0) == node2 && w->getLastNodeId() == node1);
+ return
+ (w->getNodeId(0) == node1 && w->getLastNodeId() == node2) ||
+ (w->getNodeId(0) == node2 && w->getLastNodeId() == node1);
+}
+
+WaySublineMatchString HighwaySnapMerger::_matchSubline(OsmMapPtr map, ElementPtr e1, ElementPtr e2)
+{
+ // Some attempts were made to use cached subline matches pased in from HighwaySnapMergerJs for
+ // performance reasons, but the results were unstable. See branch 3969b.
+ return _sublineMatcher->findMatch(map, e1, e2);
}
bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, ElementId eid2,
@@ -188,10 +201,14 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
{
// TODO: This monster method needs to be refactored into smaller parts where possible.
+ // ENABLE THE OsmMapWriterFactory::writeDebugMap CALLS FOR SMALL DATASETS DURING DEBUGGING ONLY.
+ // writes a map file for each road merge
+
LOG_VART(eid1);
- LOG_VART(map->getElement(eid1));
+ //LOG_VART(map->getElement(eid1));
LOG_VART(eid2);
- LOG_VART(map->getElement(eid2));
+ //LOG_VART(map->getElement(eid2));
+ //const QString eidLogString = "-" + eid1.toString() + "-" + eid2.toString();
if (HighwayMergerAbstract::_mergePair(map, eid1, eid2, replaced))
{
@@ -229,6 +246,9 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
// remove the second element and any reviews that contain the element
RemoveReviewsByEidOp(remove->getElementId(), true).apply(result);
+ //OsmMapWriterFactory::writeDebugMap(
+ //map, "HighwaySnapMerger-merged-identical-elements" + eidLogString);
+
return false;
}
@@ -236,7 +256,7 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
WaySublineMatchString match;
try
{
- match = _sublineMatcher->findMatch(result, e1, e2);
+ match = _matchSubline(result, e1, e2);
}
catch (const NeedsReviewException& e)
{
@@ -264,9 +284,11 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
// split the first element and don't reverse any of the geometries.
_splitElement(map, match.getSublineString1(), match.getReverseVector1(), replaced, e1, e1Match,
scraps1);
+ //OsmMapWriterFactory::writeDebugMap(map, "HighwaySnapMerger-after-split-1" + eidLogString);
// split the second element and reverse any geometries to make the matches work.
_splitElement(map, match.getSublineString2(), match.getReverseVector2(), replaced, e2, e2Match,
scraps2);
+ //OsmMapWriterFactory::writeDebugMap(map, "HighwaySnapMerger-after-split-2" + eidLogString);
LOG_VART(e1Match->getElementId());
if (scraps1)
@@ -289,7 +311,9 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
// remove any ways that directly connect from e1Match to e2Match
_removeSpans(result, e1Match, e2Match);
+ //OsmMapWriterFactory::writeDebugMap(map, "HighwaySnapMerger-after-remove-spans" + eidLogString);
_snapEnds(map, e2Match, e1Match);
+ //OsmMapWriterFactory::writeDebugMap(map, "HighwaySnapMerger-after-snap-ends" + eidLogString);
if (e1Match)
{
@@ -323,6 +347,7 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
LOG_VART(e1Match->getElementType());
LOG_VART(e1->getElementId());
LOG_VART(e2->getElementId());
+ //OsmMapWriterFactory::writeDebugMap(map, "HighwaySnapMerger-after-tag-merging" + eidLogString);
bool swapWayIds = false;
@@ -471,7 +496,7 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
if (swapWayIds)
{
ElementId eidm1 = e1Match->getElementId();
- LOG_TRACE("Swapping way IDs: " << eid1 << " and " << eidm1 << "...");
+ LOG_TRACE("Swapping e1 match ID: " << eidm1 << " with e1 ID: " << eid1 << "...");
// Swap the old way ID back into the match element
IdSwapOp(eid1, eidm1).apply(result);
// Remove the old way with a new swapped out ID
@@ -489,42 +514,71 @@ bool HighwaySnapMerger::_mergePair(const OsmMapPtr& map, ElementId eid1, Element
}
else if (scraps1)
{
- LOG_TRACE("Replacing: " << eid1 << " with scraps1: " << scraps1->getElementId() << "...");
+ LOG_TRACE("Replacing e1: " << eid1 << " with scraps1: " << scraps1->getElementId() << "...");
ReplaceElementOp(eid1, scraps1->getElementId(), true).apply(result);
}
}
else
{
// remove any reviews that contain this element.
- LOG_TRACE("Removing: " << eid1 << "...");
+ LOG_TRACE("Removing e1: " << eid1 << "...");
RemoveReviewsByEidOp(eid1, true).apply(result);
}
+ //OsmMapWriterFactory::writeDebugMap(
+ //map, "HighwaySnapMerger-after-old-way-removal-1" + eidLogString);
// If there is something left to review against,
if (scraps2)
{
// swap the elements with the scraps.
LOG_TRACE(
- "Replacing: " << e2Match->getElementId() << " and : " << eid2 << " with scraps2: " <<
- scraps2->getElementId() << "...");
+ "Replacing e2 match: " << e2Match->getElementId() << " and e2: " << eid2 <<
+ " with scraps2: " << scraps2->getElementId() << "...");
map->addElement(scraps2);
ReplaceElementOp(e2Match->getElementId(), scraps2->getElementId(), true).apply(result);
ReplaceElementOp(eid2, scraps2->getElementId(), true).apply(result);
-// _updateScrapParent(result, e2Match->getId(), scraps2);
}
- // Otherwise, drop the reviews and the element.
+ // Otherwise, drop the element and any reviews its in.
else
{
- LOG_TRACE("Removing: " << e2Match->getElementId() << " and : " << eid2 << "...");
+ LOG_TRACE("Removing e2 match: " << e2Match->getElementId() << " and e2: " << eid2 << "...");
+
+ // add any informational nodes from the ways being replaced to the merged output before deleting
+ // them
+ WayNodeCopier nodeCopier;
+ nodeCopier.setOsmMap(map.get());
+ nodeCopier.addCriterion(NotCriterionPtr(new NotCriterion(new NoInformationCriterion())));
+
+ if (e2Match->getElementId().getType() == ElementType::Way && eid1.getType() == ElementType::Way)
+ {
+ LOG_TRACE(
+ "Copying information nodes from e2 match: " << e2Match->getElementId() << " to e1: " <<
+ eid1 << "...");
+ nodeCopier.copy(e2Match->getElementId(), eid1);
+ }
+
+ if (e2Match->getElementId().getType() == ElementType::Way &&
+ e1Match->getElementId().getType() == ElementType::Way)
+ {
+ LOG_TRACE(
+ "Copying information nodes from e2 match: " << e2Match->getElementId() <<
+ " to e1 match: " << e1Match->getElementId() << "...");
+ nodeCopier.copy(e2Match->getElementId(), e1Match->getElementId());
+ }
+ // remove reviews e2Match is involved in
RemoveReviewsByEidOp(e2Match->getElementId(), true).apply(result);
- // Make the way that we're keeping have membership in whatever relations the way we're removing
+ // Make the way that we're keeping has membership in whatever relations the way we're removing
// was in. I *think* this makes sense. This logic may also need to be replicated elsewhere
// during merging. TODO: we may be able to combine the following two removals into a single one
RelationMemberSwapper::swap(eid2, eid1, map, false);
+
+ // remove reviews e2 is involved in
RemoveReviewsByEidOp(eid2, true).apply(result);
}
+ //OsmMapWriterFactory::writeDebugMap(
+ //map, "HighwaySnapMerger-after-old-way-removal-2" + eidLogString);
if (e1Match)
{
@@ -656,7 +710,7 @@ void HighwaySnapMerger::_removeSpans(OsmMapPtr map, const WayPtr& w1, const WayP
void HighwaySnapMerger::_snapEnds(const OsmMapPtr& map, ElementPtr snapee, ElementPtr snapTo) const
{
- // TODO: get rid of this?
+ // TODO: get rid of this and replace with visitors/WaysVisitor
class WaysVisitor : public ElementOsmMapVisitor
{
public:
@@ -697,23 +751,25 @@ void HighwaySnapMerger::_snapEnds(const OsmMapPtr& map, ElementPtr snapee, Eleme
vector<WayPtr>& _w;
};
- LOG_TRACE("Snapping ends...");
+ LOG_TRACE(
+ "Snapping end of " << snapee->getElementId() << " to " << snapTo->getElementId() << "...");
//LOG_VART(snapee);
//LOG_VART(snapTo);
- // convert all the elements into arrays of ways. If it is a way already then it creates a vector
- // of size 1 with that way, if they're relations w/ complex multilinestrings then you'll get all
+ // Convert all the elements into arrays of ways. If it is a way already, then it creates a vector
+ // of size 1 with that way. If they're relations w/ complex multilinestrings, then you'll get all
// the component ways.
vector<WayPtr> snapeeWays = WaysVisitor::getWays(map, snapee);
vector<WayPtr> snapToWays = WaysVisitor::getWays(map, snapTo);
-
+ LOG_VART(snapeeWays.size());
+ LOG_VART(snapToWays.size());
assert(snapToWays.size() == snapeeWays.size());
std::shared_ptr<NodeToWayMap> n2w = map->getIndex().getNodeToWayMap();
for (size_t i = 0; i < snapeeWays.size(); i++)
{
- // find all the ways that connect to the beginning or end of w1. There shouldn't be any that
+ // Find all the ways that connect to the beginning or end of w1. There shouldn't be any that
// connect in the middle.
set<long> wids = n2w->getWaysByNode(snapeeWays[i]->getNodeIds()[0]);
const set<long>& wids2 = n2w->getWaysByNode(snapeeWays[i]->getLastNodeId());
@@ -726,21 +782,55 @@ void HighwaySnapMerger::_snapEnds(const OsmMapPtr& map, ElementPtr snapee, Eleme
WayPtr w = map->getWay(*it);
if (w->getStatus() == Status::Unknown2)
{
- _snapEnds(map->getWay(*it), snapeeWays[i], snapToWays[i]);
+ _snapEnds(map, map->getWay(*it), snapeeWays[i], snapToWays[i]);
}
}
}
- _snapEnds(snapeeWays[i], snapeeWays[i], snapToWays[i]);
+ _snapEnds(map, snapeeWays[i], snapeeWays[i], snapToWays[i]);
}
//LOG_VART(snapee);
//LOG_VART(snapTo);
}
-void HighwaySnapMerger::_snapEnds(WayPtr snapee, WayPtr middle, WayPtr snapTo) const
+void HighwaySnapMerger::_snapEnds(const OsmMapPtr& map, WayPtr snapee, WayPtr middle,
+ WayPtr snapTo) const
{
- snapee->replaceNode(middle->getNodeId(0), snapTo->getNodeId(0));
- snapee->replaceNode(middle->getLastNodeId(), snapTo->getLastNodeId());
+ NodePtr replacedNode = map->getNode(middle->getNodeId(0));
+ NodePtr replacementNode = map->getNode(snapTo->getNodeId(0));
+ _snapEnd(map, snapee, replacedNode, replacementNode);
+
+ replacedNode = map->getNode(middle->getLastNodeId());
+ replacementNode = map->getNode(snapTo->getLastNodeId());
+ _snapEnd(map, snapee, replacedNode, replacementNode);
+}
+
+void HighwaySnapMerger::_snapEnd(const OsmMapPtr& map, WayPtr snapee, NodePtr replacedNode,
+ NodePtr replacementNode) const
+{
+ LOG_TRACE(
+ "Replacing " << replacedNode->getElementId() << " with " << replacementNode->getElementId() <<
+ " on " << snapee->getElementId() << "...");
+ // replace the node
+ snapee->replaceNode(replacedNode->getId(), replacementNode->getId());
+
+ // TODO: This is a hack for now, as one single test (highway-search-radius-1) has highway=road
+ // which makes no sense on a node. Want to think about it a little more before deciding to modify
+ // the test's input data.
+ const QStringList nodeKvpExcludeList("highway=road");
+ // If the node we just replaced has info and the one we're replacing it with does not, let's copy
+ // that info over to the replacement.
+ if (replacedNode->getTags().hasInformationTag() &&
+ !replacementNode->getTags().hasInformationTag() &&
+ !replacedNode->getTags().hasAnyKvp(nodeKvpExcludeList))
+ {
+ replacementNode->setTags(
+ TagMergerFactory::mergeTags(
+ replacementNode->getTags(), replacedNode->getTags(), ElementType::Node));
+ }
+ // Let's also preserve relation membership.
+ RelationMemberSwapper::swap(
+ replacedNode->getElementId(), replacementNode->getElementId(), map, false);
}
void HighwaySnapMerger::_splitElement(const OsmMapPtr& map, const WaySublineCollection& s,
@@ -757,14 +847,14 @@ void HighwaySnapMerger::_splitElement(const OsmMapPtr& map, const WaySublineColl
set<ConstWayPtr, WayPtrCompare> ways;
ways.insert(waysV.begin(), waysV.end());
- // remove all the ways that are part of the subline. This leaves us with a list of ways that
+ // Remove all the ways that are part of the subline. This leaves us with a list of ways that
// aren't going to be modified.
for (size_t i = 0; i < s.getSublines().size(); i++)
{
ways.erase(s.getSublines()[i].getWay());
}
- // the subline string split should always result in a match section.
+ // The subline string split should always result in a match section.
assert(match);
// if there are ways that aren't part of the way subline string
@@ -774,8 +864,10 @@ void HighwaySnapMerger::_splitElement(const OsmMapPtr& map, const WaySublineColl
RelationPtr r;
if (!scrap || scrap->getElementType() == ElementType::Way)
{
- r.reset(new Relation(splitee->getStatus(), map->createNextRelationId(),
- splitee->getCircularError(), MetadataTags::RelationMultilineString()));
+ r.reset(
+ new Relation(
+ splitee->getStatus(), map->createNextRelationId(), splitee->getCircularError(),
+ MetadataTags::RelationMultilineString()));
if (scrap)
{
r->addElement("", scrap);
@@ -881,10 +973,10 @@ void HighwaySnapMerger::_splitElement(const OsmMapPtr& map, const WaySublineColl
multiLineStringAdded = true;
}
- // make sure the tags are still legit on the scrap.
+ // Make sure the tags are still legit on the scrap.
scrap->setTags(splitee->getTags());
// With the merging switching between split ways and relations, it gets a little hard to keep
- // track of where this tags is needed, so one final check here to make sure it gets added
+ // track of where this tag is needed, so one final check here to make sure it gets added
// correctly.
if (_markAddedMultilineStringRelations && multiLineStringAdded &&
scrap->getElementType() == ElementType::Relation)