From 964a1bb8532249f82d4d647bd496d77ccb5e240f Mon Sep 17 00:00:00 2001 From: Paolo Angelelli Date: Wed, 18 Dec 2019 21:33:57 +0100 Subject: Fix QGeoPolygon translate not updating clipper cache Problem resulting in failure when calling the contains() method. This patch caches the leftbound that is used in many other methods. This patch also fixes the copy constructor, now correctly converting rectangles and circles into polygons. Tests are added in a subsequent patch. [ChangeLog][QtPositioning][QGeoPolygon] Fixed contains method. Change-Id: I324e5a6e9bf981f830a8bb88e5f2abedb5ece5dd Reviewed-by: Alex Blasche --- src/positioning/qgeopath.cpp | 24 ++++--- src/positioning/qgeopath_p.h | 1 + src/positioning/qgeopolygon.cpp | 78 ++++++++++++++++++---- .../declarative_geoshape/tst_locationsingleton.qml | 4 +- 4 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/positioning/qgeopath.cpp b/src/positioning/qgeopath.cpp index 1225d7c8..b6513bc7 100644 --- a/src/positioning/qgeopath.cpp +++ b/src/positioning/qgeopath.cpp @@ -475,6 +475,9 @@ bool QGeoPathPrivate::lineContains(const QGeoCoordinate &coordinate) const // If the mercator x value of a coordinate of the line, or the coordinate parameter, is less // than mercator(m_bbox).x, add that to the conversion. + if (m_bboxDirty) + const_cast(*this).computeBoundingBox(); + double lineRadius = qMax(width() * 0.5, 0.2); // minimum radius: 20cm if (!m_path.size()) @@ -482,23 +485,21 @@ bool QGeoPathPrivate::lineContains(const QGeoCoordinate &coordinate) const else if (m_path.size() == 1) return (m_path[0].distanceTo(coordinate) <= lineRadius); - double leftBoundMercator = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); - QDoubleVector2D p = QWebMercator::coordToMercator(coordinate); - if (p.x() < leftBoundMercator) - p.setX(p.x() + leftBoundMercator); // unwrap X + if (p.x() < m_leftBoundWrapped) + p.setX(p.x() + m_leftBoundWrapped); // unwrap X QDoubleVector2D a; QDoubleVector2D b; if (m_path.size()) { a = QWebMercator::coordToMercator(m_path[0]); - if (a.x() < leftBoundMercator) - a.setX(a.x() + leftBoundMercator); // unwrap X + if (a.x() < m_leftBoundWrapped) + a.setX(a.x() + m_leftBoundWrapped); // unwrap X } for (int i = 1; i < m_path.size(); i++) { b = QWebMercator::coordToMercator(m_path[i]); - if (b.x() < leftBoundMercator) - b.setX(b.x() + leftBoundMercator); // unwrap X + if (b.x() < m_leftBoundWrapped) + b.setX(b.x() + m_leftBoundWrapped); // unwrap X if (b == a) continue; @@ -513,7 +514,7 @@ bool QGeoPathPrivate::lineContains(const QGeoCoordinate &coordinate) const if (candidate.x() > 1.0) - candidate.setX(candidate.x() - leftBoundMercator); // wrap X + candidate.setX(candidate.x() - m_leftBoundWrapped); // wrap X QGeoCoordinate closest = QWebMercator::mercatorToCoord(candidate); @@ -600,6 +601,7 @@ void QGeoPathPrivate::translate(double degreesLatitude, double degreesLongitude) p.setLongitude(QLocationUtils::wrapLong(p.longitude() + degreesLongitude)); } m_bbox.translate(degreesLatitude, degreesLongitude); + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); } QGeoRectangle QGeoPathPrivate::boundingGeoRectangle() const @@ -673,6 +675,7 @@ void QGeoPathPrivate::computeBoundingBox() double m_minX, m_maxX, m_minLati, m_maxLati; m_bboxDirty = false; computeBBox(m_path, m_deltaXs, m_minX, m_maxX, m_minLati, m_maxLati, m_bbox); + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); } QGeoPathPrivateEager::QGeoPathPrivateEager() @@ -715,6 +718,7 @@ void QGeoPathPrivateEager::translate(double degreesLatitude, double degreesLongi m_bbox.translate(degreesLatitude, degreesLongitude); m_minLati += degreesLatitude; m_maxLati += degreesLatitude; + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); } void QGeoPathPrivateEager::addCoordinate(const QGeoCoordinate &coordinate) @@ -729,11 +733,13 @@ void QGeoPathPrivateEager::addCoordinate(const QGeoCoordinate &coordinate) void QGeoPathPrivateEager::QGeoPathPrivateEager::computeBoundingBox() { computeBBox(m_path, m_deltaXs, m_minX, m_maxX, m_minLati, m_maxLati, m_bbox); + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); } void QGeoPathPrivateEager::QGeoPathPrivateEager::updateBoundingBox() { updateBBox(m_path, m_deltaXs, m_minX, m_maxX, m_minLati, m_maxLati, m_bbox); + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); } QGeoPathEager::QGeoPathEager() : QGeoPath() diff --git a/src/positioning/qgeopath_p.h b/src/positioning/qgeopath_p.h index be73994f..6dd17b09 100644 --- a/src/positioning/qgeopath_p.h +++ b/src/positioning/qgeopath_p.h @@ -220,6 +220,7 @@ public: QList m_path; qreal m_width = 0; QGeoRectangle m_bbox; // cached + double m_leftBoundWrapped; // cached bool m_bboxDirty = false; }; diff --git a/src/positioning/qgeopolygon.cpp b/src/positioning/qgeopolygon.cpp index e8fdda76..301759df 100644 --- a/src/positioning/qgeopolygon.cpp +++ b/src/positioning/qgeopolygon.cpp @@ -39,6 +39,8 @@ #include "qgeopolygon.h" #include "qgeopolygon_p.h" +#include "qgeopath_p.h" +#include "qgeocircle.h" #include "qgeocoordinate.h" #include "qnumeric.h" @@ -135,6 +137,41 @@ QGeoPolygon::QGeoPolygon(const QGeoPolygon &other) initPolygonConversions(); } +static void calculatePeripheralPoints(QList &path, + const QGeoCircle &circle, + int steps) +{ + const QGeoCoordinate ¢er = circle.center(); + const qreal distance = circle.radius(); + // Calculate points based on great-circle distance + // Calculation is the same as GeoCoordinate's atDistanceAndAzimuth function + // but tweaked here for computing multiple points + + // pre-calculations + steps = qMax(steps, 3); + qreal centerLon = center.longitude(); + qreal latRad = QLocationUtils::radians(center.latitude()); + qreal lonRad = QLocationUtils::radians(centerLon); + qreal cosLatRad = std::cos(latRad); + qreal sinLatRad = std::sin(latRad); + qreal ratio = (distance / QLocationUtils::earthMeanRadius()); + qreal cosRatio = std::cos(ratio); + qreal sinRatio = std::sin(ratio); + qreal sinLatRad_x_cosRatio = sinLatRad * cosRatio; + qreal cosLatRad_x_sinRatio = cosLatRad * sinRatio; + for (int i = 0; i < steps; ++i) { + qreal azimuthRad = 2 * M_PI * i / steps; + qreal resultLatRad = std::asin(sinLatRad_x_cosRatio + + cosLatRad_x_sinRatio * std::cos(azimuthRad)); + qreal resultLonRad = lonRad + std::atan2(std::sin(azimuthRad) * cosLatRad_x_sinRatio, + cosRatio - sinLatRad * std::sin(resultLatRad)); + qreal lat2 = QLocationUtils::degrees(resultLatRad); + qreal lon2 = QLocationUtils::wrapLong(QLocationUtils::degrees(resultLonRad)); + + path << QGeoCoordinate(lat2, lon2, center.altitude()); + } +} + /*! Constructs a new geo polygon from the contents of \a other. */ @@ -142,8 +179,22 @@ QGeoPolygon::QGeoPolygon(const QGeoShape &other) : QGeoShape(other) { initPolygonConversions(); - if (type() != QGeoShape::PolygonType) - d_ptr = new QGeoPolygonPrivate(); + if (type() != QGeoShape::PolygonType) { + QGeoPolygonPrivate *poly = new QGeoPolygonPrivate(); + if (type() == QGeoShape::CircleType) { + const QGeoCircle &circle = static_cast(other); + QList perimeter; + calculatePeripheralPoints(perimeter, circle, 128); + poly->setPath(perimeter); + } else if (type() == QGeoShape::RectangleType) { + const QGeoRectangle &rect = static_cast(other); + QList perimeter; + perimeter << rect.topLeft() << rect.topRight() + << rect.bottomRight() << rect.bottomLeft(); + poly->setPath(perimeter); + } + d_ptr = poly; + } } /*! @@ -504,10 +555,11 @@ void QGeoPolygonPrivate::translate(double degreesLatitude, double degreesLongitu // Need min/maxLati, so update bbox QVector m_deltaXs; double m_minX, m_maxX, m_minLati, m_maxLati; - m_bboxDirty = false; + m_bboxDirty = false; // Updated in translatePoly computeBBox(m_path, m_deltaXs, m_minX, m_maxX, m_minLati, m_maxLati, m_bbox); - translatePoly(m_path, m_holesList, m_bbox, degreesLatitude, degreesLongitude, m_maxLati, m_minLati); + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); + m_clipperDirty = true; } bool QGeoPolygonPrivate::operator==(const QGeoShapePrivate &other) const @@ -557,10 +609,11 @@ bool QGeoPolygonPrivate::polygonContains(const QGeoCoordinate &coordinate) const const_cast(this)->updateClipperPath(); // this one updates bbox too if needed QDoubleVector2D coord = QWebMercator::coordToMercator(coordinate); - double tlx = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); - if (coord.x() < tlx) + + if (coord.x() < m_leftBoundWrapped) coord.setX(coord.x() + 1.0); + IntPoint intCoord = QClipperUtils::toIntPoint(coord); if (!c2t::clip2tri::pointInPolygon(intCoord, m_clipperPath)) return false; @@ -570,12 +623,7 @@ bool QGeoPolygonPrivate::polygonContains(const QGeoCoordinate &coordinate) const // ToDo: cache these QGeoPolygon holePolygon; holePolygon.setPath(holePath); - // QGeoPath holeBoundary; - // holeBoundary.setPath(holePath); - - if (holePolygon.contains(coordinate) - // && !(holeBoundary.contains(coordinate)) - ) + if (holePolygon.contains(coordinate)) return false; } return true; @@ -591,11 +639,11 @@ void QGeoPolygonPrivate::updateClipperPath() if (m_bboxDirty) computeBoundingBox(); m_clipperDirty = false; - double tlx = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); + QList preservedPath; for (const QGeoCoordinate &c : m_path) { QDoubleVector2D crd = QWebMercator::coordToMercator(c); - if (crd.x() < tlx) + if (crd.x() < m_leftBoundWrapped) crd.setX(crd.x() + 1.0); preservedPath << crd; } @@ -625,6 +673,7 @@ QGeoShapePrivate *QGeoPolygonPrivateEager::clone() const void QGeoPolygonPrivateEager::translate(double degreesLatitude, double degreesLongitude) { translatePoly(m_path, m_holesList, m_bbox, degreesLatitude, degreesLongitude, m_maxLati, m_minLati); + m_clipperDirty = true; } void QGeoPolygonPrivateEager::markDirty() @@ -645,6 +694,7 @@ void QGeoPolygonPrivateEager::addCoordinate(const QGeoCoordinate &coordinate) void QGeoPolygonPrivateEager::computeBoundingBox() { computeBBox(m_path, m_deltaXs, m_minX, m_maxX, m_minLati, m_maxLati, m_bbox); + m_leftBoundWrapped = QWebMercator::coordToMercator(m_bbox.topLeft()).x(); } void QGeoPolygonPrivateEager::updateBoundingBox() diff --git a/tests/auto/declarative_geoshape/tst_locationsingleton.qml b/tests/auto/declarative_geoshape/tst_locationsingleton.qml index 096a72e9..b58eedfd 100644 --- a/tests/auto/declarative_geoshape/tst_locationsingleton.qml +++ b/tests/auto/declarative_geoshape/tst_locationsingleton.qml @@ -222,11 +222,11 @@ Item { polygon = QtPositioning.shapeToPolygon(QtPositioning.circle()) verify(!polygon.isValid) polygon = QtPositioning.shapeToPolygon(QtPositioning.circle(tl, 10000)) - verify(!polygon.isValid) + verify(polygon.isValid) // fixed, polygon copy constructor can now initialize from a circle. polygon = QtPositioning.shapeToPolygon(QtPositioning.rectangle()) verify(!polygon.isValid) polygon = QtPositioning.shapeToPolygon(QtPositioning.rectangle(tl, br)) - verify(!polygon.isValid) + verify(polygon.isValid) // fixed, polygon copy constructor can now initialize from a rectangle. polygon = QtPositioning.shapeToPolygon(QtPositioning.polygon()) verify(!polygon.isValid) -- cgit v1.2.1