Commit 75fe1f3d authored by Volker Krause's avatar Volker Krause
Browse files

Rewrite tracking of OSM data across clipping

The previous approach put coordinate pointers into the point structures of
the clipper lib. This worked in most cases, but as there are no guarantees
on point references remaining valid, the clipper lib ended up swapping
values of points in some cases, breaking our coordinate references and
subsequently losing the attached OSM data. While this is usually not a
problem for a random node, it could also happen on the closing node of a
polygon, causing it to no longer be closed and confusing the hell out of
the geometry reassembly code.

The new approach tracks this outside of the clipper lib with a map on the
integer coordinates (and thus avoiding floating point number comparison
issues). This also allows us to roll back a bunch of Marble-specific
changes to the clipper code.
parent 73c4289d
......@@ -139,31 +139,30 @@ ClipperLib::Path VectorClipper::clipPath(const GeoDataLatLonBox &box, int zoomLe
using namespace ClipperLib;
Path path;
int const steps = qMax(1, 22 - 2 * zoomLevel);
qreal const scale = IntPoint::scale;
double x = box.west() * scale;
double const horizontalStep = (box.east() * scale - x) / steps;
double y = box.north() * scale;
double const verticalStep = (box.south() * scale - y) / steps;
double x = box.west() * s_pointScale;
double const horizontalStep = (box.east() * s_pointScale - x) / steps;
double y = box.north() * s_pointScale;
double const verticalStep = (box.south() * s_pointScale - y) / steps;
for (int i=0; i<steps; ++i) {
path << IntPoint(qRound64(x), qRound64(y));
x += horizontalStep;
}
path << IntPoint(qRound64(box.east() * scale), qRound64(box.north() * scale));
path << IntPoint(qRound64(box.east() * s_pointScale), qRound64(box.north() * s_pointScale));
for (int i=0; i<steps; ++i) {
path << IntPoint(qRound64(x), qRound64(y));
y += verticalStep;
}
path << IntPoint(qRound64(box.east() * scale), qRound64(box.south() * scale));
path << IntPoint(qRound64(box.east() * s_pointScale), qRound64(box.south() * s_pointScale));
for (int i=0; i<steps; ++i) {
path << IntPoint(qRound64(x), qRound64(y));
x -= horizontalStep;
}
path << IntPoint(qRound64(box.west() * scale), qRound64(box.south() * scale));
path << IntPoint(qRound64(box.west() * s_pointScale), qRound64(box.south() * s_pointScale));
for (int i=0; i<steps; ++i) {
path << IntPoint(qRound64(x), qRound64(y));
y -= verticalStep;
}
path << IntPoint(qRound64(box.west() * scale), qRound64(box.north() * scale));
path << IntPoint(qRound64(box.west() * s_pointScale), qRound64(box.north() * s_pointScale));
return path;
}
......@@ -241,8 +240,11 @@ void VectorClipper::clipPolygon(const GeoDataPlacemark *placemark, const Clipper
}
using namespace ClipperLib;
Path path;
QHash<std::pair<cInt, cInt>, const GeoDataCoordinates*> coordMap;
for(auto const & node: qAsConst(polygon)->outerBoundary()) {
path << IntPoint(&node);
auto p = coordinateToPoint(node);
coordMap.insert(std::make_pair(p.X, p.Y), &node);
path.push_back(std::move(p));
}
cInt minX, maxX, minY, maxY;
......@@ -263,16 +265,7 @@ void VectorClipper::clipPolygon(const GeoDataPlacemark *placemark, const Clipper
int index = -1;
OsmPlacemarkData const & outerRingOsmData = placemarkOsmData.memberReference(index);
OsmPlacemarkData & newOuterRingOsmData = newPlacemarkOsmData.memberReference(index);
int nodeIndex = 0;
for(const auto &point: path) {
GeoDataCoordinates const coordinates = point.coordinates();
outerRing << coordinates;
auto const originalOsmData = outerRingOsmData.nodeReference(coordinates);
if (originalOsmData.id() > 0) {
newOuterRingOsmData.addNodeReference(coordinates, originalOsmData);
}
++nodeIndex;
}
pathToRing(path, &outerRing, outerRingOsmData, newOuterRingOsmData, coordMap);
GeoDataPolygon* newPolygon = new GeoDataPolygon;
newPolygon->setOuterBoundary(outerRing);
......@@ -305,8 +298,11 @@ void VectorClipper::clipPolygon(const GeoDataPlacemark *placemark, const Clipper
clipper.Clear();
clipper.AddPath(path, ptClip, true);
Path innerPath;
coordMap.clear();
for(auto const & node: innerBoundary) {
innerPath << IntPoint(&node);
auto p = coordinateToPoint(node);
coordMap.insert(std::make_pair(p.X, p.Y), &node);
innerPath.push_back(std::move(p));
}
clipper.AddPath(innerPath, ptSubject, true);
Paths innerPaths;
......@@ -315,16 +311,7 @@ void VectorClipper::clipPolygon(const GeoDataPlacemark *placemark, const Clipper
int const newIndex = newPolygon->innerBoundaries().size();
auto & newInnerRingOsmData = newPlacemarkOsmData.memberReference(newIndex);
GeoDataLinearRing innerRing;
nodeIndex = 0;
for(const auto &point: innerPath) {
GeoDataCoordinates const coordinates = point.coordinates();
innerRing << coordinates;
auto const originalOsmData = innerRingOsmData.nodeReference(coordinates);
if (originalOsmData.id() > 0) {
newInnerRingOsmData.addNodeReference(coordinates, originalOsmData);
}
++nodeIndex;
}
pathToRing(innerPath, &innerRing, innerRingOsmData, newInnerRingOsmData, coordMap);
newPolygon->appendInnerBoundary(innerRing);
if (innerRingOsmData.id() > 0) {
newInnerRingOsmData.addTag(QStringLiteral("mx:oid"), QString::number(innerRingOsmData.id()));
......
......@@ -50,6 +50,36 @@ private:
qreal area(const GeoDataLinearRing &ring);
void getBounds(const ClipperLib::Path &path, ClipperLib::cInt &minX, ClipperLib::cInt &maxX, ClipperLib::cInt &minY, ClipperLib::cInt &maxY) const;
// convert radian-based coordinates to 10^-7 degree (100 nanodegree) integer coordinates used by the clipper library
constexpr static qint64 const s_pointScale = 10000000 / M_PI * 180;
static inline ClipperLib::IntPoint coordinateToPoint(const GeoDataCoordinates &c)
{
return ClipperLib::IntPoint(qRound64(c.longitude() * s_pointScale), qRound64(c.latitude() * s_pointScale));
}
static inline GeoDataCoordinates pointToCoordinate(ClipperLib::IntPoint p)
{
return GeoDataCoordinates((double)p.X / s_pointScale, (double)p.Y / s_pointScale);
}
template<class T>
static void pathToRing(const ClipperLib::Path &path, T *ring, const OsmPlacemarkData &originalOsmData, OsmPlacemarkData &newOsmData, const QHash<std::pair<ClipperLib::cInt, ClipperLib::cInt>, const GeoDataCoordinates*> &coordMap)
{
int index = 0;
for(const auto &point: path) {
const auto it = coordMap.find(std::make_pair(point.X, point.Y));
if (it != coordMap.end()) {
*ring << *it.value();
auto const data = originalOsmData.nodeReference(*it.value());
if (data.id() > 0) {
newOsmData.addNodeReference(*it.value(), data);
}
} else {
*ring << pointToCoordinate(point);
}
++index;
}
}
template<class T>
void clipString(const GeoDataPlacemark *placemark, const ClipperLib::Path &tileBoundary, qreal minArea,
GeoDataDocument* document, QSet<qint64> &osmIds)
......@@ -71,8 +101,11 @@ private:
auto const & osmData = placemark->osmData();
using namespace ClipperLib;
Path subject;
QHash<std::pair<cInt, cInt>, const GeoDataCoordinates*> coordMap;
for(auto const & node: *ring) {
subject << IntPoint(&node);
auto p = coordinateToPoint(node);
coordMap.insert(std::make_pair(p.X, p.Y), &node);
subject.push_back(std::move(p));
}
cInt minX, maxX, minY, maxY;
getBounds(tileBoundary, minX, maxX, minY, maxY);
......@@ -94,16 +127,7 @@ private:
newPlacemark->setVisible(placemark->isVisible());
newPlacemark->setVisualCategory(placemark->visualCategory());
T* newRing = new T;
int index = 0;
for(const auto &point: path) {
GeoDataCoordinates const coordinates = point.coordinates();
*newRing << coordinates;
auto const originalOsmData = osmData.nodeReference(coordinates);
if (originalOsmData.id() > 0) {
newPlacemark->osmData().addNodeReference(coordinates, originalOsmData);
}
++index;
}
pathToRing(path, newRing, osmData, newPlacemark->osmData(), coordMap);
if (isBuilding) {
const auto building = geodata_cast<GeoDataBuilding>(placemark->geometry());
......
......@@ -48,8 +48,6 @@
#include <ostream>
#include <functional>
#include <MarbleMath.h>
namespace ClipperLib {
static double const pi = 3.141592653589793238;
......@@ -553,8 +551,8 @@ bool SlopesEqual(const TEdge &e1, const TEdge &e2, bool UseFullInt64Range)
}
//------------------------------------------------------------------------------
bool SlopesEqual(const IntPoint &pt1, const IntPoint &pt2,
const IntPoint &pt3, bool UseFullInt64Range)
bool SlopesEqual(const IntPoint pt1, const IntPoint pt2,
const IntPoint pt3, bool UseFullInt64Range)
{
#ifndef use_int32
if (UseFullInt64Range)
......@@ -565,8 +563,8 @@ bool SlopesEqual(const IntPoint &pt1, const IntPoint &pt2,
}
//------------------------------------------------------------------------------
bool SlopesEqual(const IntPoint &pt1, const IntPoint &pt2,
const IntPoint &pt3, const IntPoint &pt4, bool UseFullInt64Range)
bool SlopesEqual(const IntPoint pt1, const IntPoint pt2,
const IntPoint pt3, const IntPoint pt4, bool UseFullInt64Range)
{
#ifndef use_int32
if (UseFullInt64Range)
......@@ -583,7 +581,7 @@ inline bool IsHorizontal(TEdge &e)
}
//------------------------------------------------------------------------------
inline double GetDx(const IntPoint &pt1, const IntPoint &pt2)
inline double GetDx(const IntPoint pt1, const IntPoint pt2)
{
return (pt1.Y == pt2.Y) ?
HORIZONTAL : (double)(pt2.X - pt1.X) / (pt2.Y - pt1.Y);
......@@ -859,8 +857,8 @@ OutPt* GetBottomPt(OutPt *pp)
}
//------------------------------------------------------------------------------
bool Pt2IsBetweenPt1AndPt3(const IntPoint &pt1,
const IntPoint &pt2, const IntPoint &pt3)
bool Pt2IsBetweenPt1AndPt3(const IntPoint pt1,
const IntPoint pt2, const IntPoint pt3)
{
if ((pt1 == pt3) || (pt1 == pt2) || (pt3 == pt2))
return false;
......@@ -1941,7 +1939,7 @@ void Clipper::CopyAELToSEL()
}
//------------------------------------------------------------------------------
void Clipper::AddJoin(OutPt *op1, OutPt *op2, const IntPoint &OffPt)
void Clipper::AddJoin(OutPt *op1, OutPt *op2, const IntPoint OffPt)
{
Join* j = new Join;
j->OutPt1 = op1;
......@@ -1967,7 +1965,7 @@ void Clipper::ClearGhostJoins()
}
//------------------------------------------------------------------------------
void Clipper::AddGhostJoin(OutPt *op, const IntPoint &OffPt)
void Clipper::AddGhostJoin(OutPt *op, const IntPoint OffPt)
{
Join* j = new Join;
j->OutPt1 = op;
......@@ -3371,7 +3369,7 @@ OutPt* DupOutPt(OutPt* outPt, bool InsertAfter)
//------------------------------------------------------------------------------
bool JoinHorz(OutPt* op1, OutPt* op1b, OutPt* op2, OutPt* op2b,
const IntPoint &Pt, bool DiscardLeft)
const IntPoint Pt, bool DiscardLeft)
{
Direction Dir1 = (op1->Pt.X > op1b->Pt.X ? dRightToLeft : dLeftToRight);
Direction Dir2 = (op2->Pt.X > op2b->Pt.X ? dRightToLeft : dLeftToRight);
......@@ -4522,7 +4520,7 @@ void MinkowskiSum(const Path& pattern, const Path& path, Paths& solution, bool p
}
//------------------------------------------------------------------------------
void TranslatePath(const Path& input, Path& output, const IntPoint &delta)
void TranslatePath(const Path& input, Path& output, const IntPoint delta)
{
//precondition: input != output
output.resize(input.size());
......@@ -4626,34 +4624,6 @@ std::ostream& operator <<(std::ostream &s, const Paths &p)
s << "\n";
return s;
}
IntPoint::IntPoint(const Marble::GeoDataCoordinates *coordinates) :
X(qRound64(coordinates->longitude() * scale)),
Y(qRound64(coordinates->latitude() * scale)),
m_coordinates(coordinates)
{
// nothing to do
}
Marble::GeoDataCoordinates IntPoint::coordinates() const
{
using namespace Marble;
GeoDataCoordinates const coords = GeoDataCoordinates(double(X) / scale, double(Y) / scale);
if (m_coordinates) {
bool const clipperKeptTheNode = qRound64(m_coordinates->longitude() * scale) == X &&
qRound64(m_coordinates->latitude() * scale) == Y;
if (clipperKeptTheNode) {
return *m_coordinates;
}
}
return coords;
}
bool IntPoint::isInside(const cInt &minX, const cInt &maxX, const cInt &minY, const cInt &maxY) const
{
return X > minX && X < maxX && Y > minY && Y < maxY;
}
//------------------------------------------------------------------------------
} //ClipperLib namespace
......@@ -59,8 +59,6 @@
#include <functional>
#include <queue>
#include <GeoDataCoordinates.h>
namespace ClipperLib {
enum ClipType { ctIntersection, ctUnion, ctDifference, ctXor };
......@@ -87,14 +85,11 @@ enum PolyFillType { pftEvenOdd, pftNonZero, pftPositive, pftNegative };
struct IntPoint {
cInt X;
cInt Y;
// convert radian-based coordinates to 10^-7 degree (100 nanodegree) integer coordinates used by the clipper library
constexpr static qint64 const scale = 10000000 / M_PI * 180;
#ifdef use_xyz
cInt Z;
IntPoint(cInt x = 0, cInt y = 0, cInt z = 0): X(x), Y(y), Z(z) {};
#else
IntPoint(cInt x = 0, cInt y = 0): X(x), Y(y) {}
IntPoint(const Marble::GeoDataCoordinates* coordinates);
IntPoint(cInt x = 0, cInt y = 0): X(x), Y(y) {};
#endif
friend inline bool operator== (const IntPoint& a, const IntPoint& b)
......@@ -105,12 +100,6 @@ struct IntPoint {
{
return a.X != b.X || a.Y != b.Y;
}
Marble::GeoDataCoordinates coordinates() const;
bool isInside(const cInt &minX, const cInt &maxX, const cInt &minY, const cInt &maxY) const;
private:
const Marble::GeoDataCoordinates * m_coordinates = nullptr;
};
//------------------------------------------------------------------------------
......@@ -129,7 +118,7 @@ struct DoublePoint
double X;
double Y;
DoublePoint(double x = 0, double y = 0) : X(x), Y(y) {}
DoublePoint(const IntPoint &ip) : X((double)ip.X), Y((double)ip.Y) {}
DoublePoint(IntPoint ip) : X((double)ip.X), Y((double)ip.Y) {}
};
//------------------------------------------------------------------------------
......@@ -350,10 +339,10 @@ private:
bool IsHole(TEdge *e);
bool FindOwnerFromSplitRecs(OutRec &outRec, OutRec *&currOrfl);
void FixHoleLinkage(OutRec &outrec);
void AddJoin(OutPt *op1, OutPt *op2, const IntPoint &offPt);
void AddJoin(OutPt *op1, OutPt *op2, const IntPoint offPt);
void ClearJoins();
void ClearGhostJoins();
void AddGhostJoin(OutPt *op, const IntPoint &offPt);
void AddGhostJoin(OutPt *op, const IntPoint offPt);
bool JoinPoints(Join *j, OutRec* outRec1, OutRec* outRec2);
void JoinCommonEdges();
void DoSimplePolygons();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment