[geos-commits] [SCM] GEOS branch main updated. 159cc1f9dba3ee820e31123af4f90a2b6e090546
git at osgeo.org
git at osgeo.org
Sun Nov 1 18:52:09 PST 2026
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GEOS".
The branch, main has been updated
via 159cc1f9dba3ee820e31123af4f90a2b6e090546 (commit)
via 080bf38feb81ffae17dd6d7e48b7601e468ef1b0 (commit)
via 1c29a5fa73afabaa78d4310ea48187097723611b (commit)
from 1441ad5b48a86e8ff99a2f693fe777377d0f766f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 159cc1f9dba3ee820e31123af4f90a2b6e090546
Author: Alejandro Pacheco <ale at alepacheco.dev>
Date: Thu Oct 29 07:27:45 2026 +0100
CircularArc: fix copy assignment of arc not starting at position 0
The copy assignment operator copied the three points of the arc into a
new CoordinateSequence, but kept the position of the arc in the original
sequence, so an arc assigned from one that did not start at position 0
referred to points past the end of its copy.
Implement copy assignment with the copy constructor and move assignment.
diff --git a/src/geom/CircularArc.cpp b/src/geom/CircularArc.cpp
index 44414c0ad..1ae2e9e9f 100644
--- a/src/geom/CircularArc.cpp
+++ b/src/geom/CircularArc.cpp
@@ -145,28 +145,10 @@ CircularArc::CircularArc(CircularArc&& other) noexcept {
CircularArc&
CircularArc::operator=(const CircularArc& other)
{
- if (this == &other) {
- return *this;
+ if (this != &other) {
+ *this = CircularArc(other);
}
- if (m_own_coordinates) {
- delete m_seq;
- }
-
- m_seq = new CoordinateSequence(0, other.getCoordinateSequence()->hasZ(), other.getCoordinateSequence()->hasM());
- m_pos = other.m_pos;
- m_own_coordinates = true;
- m_orientation = other.m_orientation;
- m_orientation_known = other.m_orientation_known;
- m_center = other.m_center;
- m_center_known = other.m_center_known;
- m_radius = other.m_radius;
- m_radius_known = other.m_radius_known;
-
- CoordinateSequence* seq = const_cast<CoordinateSequence*>(m_seq);
- seq->reserve(3);
- seq->add(*other.getCoordinateSequence(), other.getCoordinatePosition(), other.getCoordinatePosition() + 2);
-
return *this;
}
diff --git a/tests/unit/geom/CircularArcTest.cpp b/tests/unit/geom/CircularArcTest.cpp
index 47078db1f..3d33a3110 100644
--- a/tests/unit/geom/CircularArcTest.cpp
+++ b/tests/unit/geom/CircularArcTest.cpp
@@ -3,6 +3,7 @@
#include <geos/algorithm/Orientation.h>
#include <geos/geom/Coordinate.h>
#include <geos/geom/CircularArc.h>
+#include <geos/geom/CoordinateSequence.h>
#include <geos/constants.h>
using geos::geom::CoordinateXY;
@@ -176,4 +177,31 @@ void object::test<7>() {
}
+template<>
+template<>
+void object::test<8>() {
+ set_test_name("copy assignment from arc that does not start at the beginning of its CoordinateSequence");
+
+ CoordinateSequence seq{
+ CoordinateXY(0, 0),
+ CoordinateXY(1, 1),
+ CoordinateXY(2, 0),
+ CoordinateXY(3, -1),
+ CoordinateXY(4, 0)
+ };
+
+ CircularArc second(seq, 2);
+
+ CircularArc arc = CircularArc::create(CoordinateXY(0, 0), CoordinateXY(1, 1), CoordinateXY(2, 0));
+ arc = second;
+
+ ensure_equals(arc.p0(), CoordinateXY(2, 0));
+ ensure_equals(arc.p1(), CoordinateXY(3, -1));
+ ensure_equals(arc.p2(), CoordinateXY(4, 0));
+
+ // arc has its own copy of the points
+ seq.setAt(CoordinateXY(5, 5), 2);
+ ensure_equals(arc.p0(), CoordinateXY(2, 0));
+}
+
}
commit 080bf38feb81ffae17dd6d7e48b7601e468ef1b0
Author: Alejandro Pacheco <ale at alepacheco.dev>
Date: Thu Oct 29 07:27:45 2026 +0100
CompoundCurve: update component curves in geometryChanged()
CompoundCurve::geometryChangedAction() recomputed its envelope from the
envelopes of its component curves, but those curves are not visited by
apply_rw(GeometryComponentFilter*), so their envelopes (and the cached
arcs of a CircularString) were not updated. After apply_rw() and
geometryChanged(), as done by GEOSGeom_transformXY, a CompoundCurve kept
the envelope of its original coordinates.
Call geometryChanged() on each component curve before recomputing the
envelope.
diff --git a/include/geos/geom/CompoundCurve.h b/include/geos/geom/CompoundCurve.h
index d090be2af..50146b8bb 100644
--- a/include/geos/geom/CompoundCurve.h
+++ b/include/geos/geom/CompoundCurve.h
@@ -119,6 +119,11 @@ protected:
void geometryChangedAction() override
{
+ // The component curves are not visited by apply_rw(GeometryComponentFilter*),
+ // so update them before computing the envelope from theirs.
+ for (auto& curve : curves) {
+ curve->geometryChanged();
+ }
envelope = computeEnvelopeInternal();
}
diff --git a/tests/unit/geom/CompoundCurveTest.cpp b/tests/unit/geom/CompoundCurveTest.cpp
index d6f5cbfde..040f3fd8d 100644
--- a/tests/unit/geom/CompoundCurveTest.cpp
+++ b/tests/unit/geom/CompoundCurveTest.cpp
@@ -454,4 +454,30 @@ void object::test<13>()
ensure_equals_exact_geometry_xyzm(curved.get(), cc_.get(), 0);
}
+template<>
+template<>
+void object::test<14>()
+{
+ set_test_name("getEnvelopeInternal() after apply_rw(const CoordinateFilter*) and geometryChanged()");
+
+ struct TranslateFilter : public geos::geom::CoordinateFilter {
+ void filter_rw(CoordinateXY* c) const override
+ {
+ c->x += 10;
+ }
+ };
+
+ TranslateFilter filter;
+ cc_->apply_rw(&filter);
+ cc_->geometryChanged();
+
+ auto expected = wktreader_.read<CompoundCurve>("COMPOUNDCURVE(CIRCULARSTRING(10 0, 11 1, 12 0), (12 0, 12 2))");
+
+ ensure_equals(*cc_->getEnvelopeInternal(), *expected->getEnvelopeInternal());
+ for (std::size_t i = 0; i < cc_->getNumCurves(); i++) {
+ ensure_equals(*cc_->getCurveN(i)->getEnvelopeInternal(), *expected->getCurveN(i)->getEnvelopeInternal());
+ }
+ ensure_equals("length", cc_->getLength(), expected->getLength(), 1e-12);
+}
+
}
commit 1c29a5fa73afabaa78d4310ea48187097723611b
Author: Alejandro Pacheco <ale at alepacheco.dev>
Date: Thu Oct 29 07:27:45 2026 +0100
CircularString: clear cached arcs when coordinates change
CircularString::getArcs() caches CircularArcs that refer to positions in
the CircularString's CoordinateSequence and cache their center, radius
and orientation. The cache was cleared when normalize() reversed an open
CircularString, but not when:
- normalize() of a closed CircularString replaced the CoordinateSequence
to change the start point, or reversed it
- apply_rw() modified the coordinates, or first replaced a shared
CoordinateSequence with a copy
- geometryChanged() was called after an external change
In these cases the arcs described the old coordinates, or referred to a
CoordinateSequence that had since been freed. For example, the length of
a CircularString returned by GEOSGeom_transformXY could be the length of
its input, and calling GEOSNormalize on a closed CircularString after
GEOSLength left its arcs referring to freed memory.
Clear the cache in each of these cases. The apply_rw() overloads are
overridden because SimpleCurve::apply_rw() can replace shared coordinates
before the caller has a chance to call geometryChanged().
diff --git a/NEWS.md b/NEWS.md
index a6510e8d4..dc9ffb1fb 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -10,6 +10,7 @@
- GeometrySplitter: Allow splitting a MultiSurface (GH-1520, Dan Baston)
- GEOSGridIntersectionFractions: Set cells outside geometry bounding box to zero (GH-1532, Dan Baston)
- GEOSisSimpleDetail returns locations with input SRID (Sandro Santilli)
+ - CircularString: clear cached arcs when coordinates change (Alejandro Pacheco)
- New things:
- Expose code revision: GEOS_REVISION, GEOSrevision, geos-config --revision
diff --git a/include/geos/geom/CircularString.h b/include/geos/geom/CircularString.h
index af5fb6555..fd282f180 100644
--- a/include/geos/geom/CircularString.h
+++ b/include/geos/geom/CircularString.h
@@ -25,11 +25,16 @@ class GEOS_DLL CircularString : public SimpleCurve {
public:
using SimpleCurve::SimpleCurve;
+ using SimpleCurve::apply_rw;
friend class GeometryFactory;
~CircularString() override;
+ void apply_rw(const CoordinateFilter* filter) override;
+
+ void apply_rw(CoordinateSequenceFilter& filter) override;
+
std::unique_ptr<CircularString> clone() const;
const std::vector<CircularArc>& getArcs() const;
@@ -82,6 +87,7 @@ protected:
void geometryChangedAction() override
{
envelope = computeEnvelopeInternal(false);
+ arcs.clear();
}
CircularString* getCurvedImpl(const algorithm::LineToCurveParams&) const override {
@@ -105,6 +111,9 @@ private:
void normalizeClosed();
+ /// Arcs describing `points`, created on demand by getArcs(). They refer to
+ /// positions in `points` (or, in a copy, to copies of the points of the
+ /// original) and must be cleared whenever `points` is replaced or modified.
mutable std::vector<CircularArc> arcs;
};
diff --git a/src/geom/CircularString.cpp b/src/geom/CircularString.cpp
index 400094df8..4ec463453 100644
--- a/src/geom/CircularString.cpp
+++ b/src/geom/CircularString.cpp
@@ -40,6 +40,22 @@ CircularString::CircularString(const std::shared_ptr<const CoordinateSequence>&
CircularString::~CircularString() = default;
+// SimpleCurve::apply_rw may replace shared coordinates with a copy, and
+// callers are not required to call geometryChanged(), so clear the arcs here.
+void
+CircularString::apply_rw(const CoordinateFilter* filter)
+{
+ arcs.clear();
+ SimpleCurve::apply_rw(filter);
+}
+
+void
+CircularString::apply_rw(CoordinateSequenceFilter& filter)
+{
+ arcs.clear();
+ SimpleCurve::apply_rw(filter);
+}
+
std::unique_ptr<CircularString>
CircularString::clone() const
{
@@ -145,11 +161,13 @@ CircularString::normalizeClosed() {
coords->reverse();
}
points = std::move(coords);
+ arcs.clear();
} else if (reverse) {
if (points.use_count() > 1) {
points = points->clone();
}
const_cast<CoordinateSequence*>(points.get())->reverse();
+ arcs.clear();
}
}
diff --git a/tests/unit/geom/CircularStringTest.cpp b/tests/unit/geom/CircularStringTest.cpp
index 3be6a7b7e..41cf5a77d 100644
--- a/tests/unit/geom/CircularStringTest.cpp
+++ b/tests/unit/geom/CircularStringTest.cpp
@@ -6,7 +6,9 @@
#include <geos/geom/CircularArc.h>
#include <geos/geom/CircularString.h>
#include <geos/geom/GeometryFactory.h>
+#include <geos/geom/CoordinateFilter.h>
#include <geos/geom/CoordinateSequence.h>
+#include <geos/geom/CoordinateSequenceFilter.h>
#include <geos/geom/IntersectionMatrix.h>
#include <geos/io/WKTReader.h>
#include <geos/util/UnsupportedOperationException.h>
@@ -50,6 +52,29 @@ struct test_circularstring_data {
ensure_equals_exact_geometry_xyzm(cs.get(), expected.get(), 0);
}
+ // Check that the arcs of a CircularString refer to its current coordinates and
+ // match the arcs of a new CircularString created from the same coordinates.
+ void checkArcs(const CircularString& cs) const {
+ auto expected = factory_->createCircularString(*cs.getCoordinatesRO());
+
+ const auto& arcs = cs.getArcs();
+ const auto& expectedArcs = expected->getArcs();
+
+ ensure_equals("number of arcs", arcs.size(), expectedArcs.size());
+ for (std::size_t i = 0; i < arcs.size(); i++) {
+ ensure("arc refers to coordinates of CircularString", arcs[i].getCoordinateSequence() == cs.getCoordinatesRO());
+ ensure_equals("arc position", arcs[i].getCoordinatePosition(), 2*i);
+ ensure_equals_xy(arcs[i].p0(), expectedArcs[i].p0());
+ ensure_equals_xy(arcs[i].p1(), expectedArcs[i].p1());
+ ensure_equals_xy(arcs[i].p2(), expectedArcs[i].p2());
+ ensure_equals("orientation", arcs[i].getOrientation(), expectedArcs[i].getOrientation());
+ ensure_equals_xy(arcs[i].getCenter(), expectedArcs[i].getCenter());
+ ensure_equals("radius", arcs[i].getRadius(), expectedArcs[i].getRadius());
+ }
+
+ ensure_equals("length", cs.getLength(), expected->getLength());
+ }
+
void checkLinearizeMaxDeviation(const std::string& wkt_in, const std::string& wkt_expected, double maxDeviation, double tol=1e-12) {
auto ctl = CurveToLineParams::maxDeviation(maxDeviation);
checkLinearize(wkt_in, wkt_expected, ctl, tol);
@@ -535,4 +560,149 @@ void object::test<27>()
ensure_equals_xyzm(seq->getAt<XYZM>(5), XYZM{2.5, -4.33, 6 + (30.0 - 6)*(2.0/3), 8 + (40.0 - 8)*(2.0/3)}, 1e-3);
}
+template<>
+template<>
+void object::test<28>()
+{
+ set_test_name("getArcs() after normalize() of closed CircularString");
+
+ for (const char* wkt : {
+ // circle, start point is OK but need to reverse points
+ "CIRCULARSTRING (-5 0, 0 -5, 5 0, 0 5, -5 0)",
+ // circle, need to change start point but orientation is OK
+ "CIRCULARSTRING (5 0, 0 -5, -5 0, 0 5, 5 0)",
+ // circle, need to change start point and orientation
+ "CIRCULARSTRING (5 0, 0 5, -5 0, 0 -5, 5 0)",
+ // half-moon, start point is OK but need to make CW
+ "CIRCULARSTRING (0 0, -3 5, 0 10, -5 5, 0 0)",
+ // half-moon, need to change start point but orientation is OK
+ "CIRCULARSTRING (0 10, -3 5, 0 0, -5 5, 0 10)"
+ }) {
+ auto cs = wktreader_.read<CircularString>(wkt);
+ cs->getLength(); // create arcs, computing their center, radius and orientation
+ cs->normalize();
+
+ checkArcs(*cs);
+ }
+}
+
+template<>
+template<>
+void object::test<29>()
+{
+ set_test_name("getArcs() after normalize() of closed CircularString with shared coordinates");
+
+ for (const char* wkt : {
+ // start point is OK but need to reverse points
+ "CIRCULARSTRING (-5 0, 0 -5, 5 0, 0 5, -5 0)",
+ // need to change start point but orientation is OK
+ "CIRCULARSTRING (5 0, 0 -5, -5 0, 0 5, 5 0)",
+ // need to change start point and orientation
+ "CIRCULARSTRING (5 0, 0 5, -5 0, 0 -5, 5 0)"
+ }) {
+ auto other = wktreader_.read<CircularString>(wkt);
+ auto cs = factory_->createCircularString(other->getSharedCoordinates());
+ cs->getLength(); // create arcs, computing their center, radius and orientation
+ cs->normalize();
+ other.reset(); // destroy the coordinates used by cs before normalize()
+
+ checkArcs(*cs);
+ }
+}
+
+template<>
+template<>
+void object::test<30>()
+{
+ set_test_name("getArcs() after apply_rw(const CoordinateFilter*)");
+
+ struct ScaleFilter : public geos::geom::CoordinateFilter {
+ void filter_rw(XY* c) const override
+ {
+ c->x *= 2;
+ c->y *= 2;
+ }
+ };
+
+ ScaleFilter filter;
+
+ // coordinates modified in place
+ cs_->getLength(); // create arcs, computing their center, radius and orientation
+ cs_->apply_rw(&filter);
+ checkArcs(*cs_);
+
+ // coordinates copied before modification because they are shared
+ auto cs = factory_->createCircularString(cs_->getSharedCoordinates());
+ cs->getLength(); // create arcs, computing their center, radius and orientation
+ cs->apply_rw(&filter);
+ checkArcs(*cs);
+ cs_.reset(); // destroy the coordinates used by cs before apply_rw()
+ checkArcs(*cs);
+
+ // arcs copied by clone()
+ auto copy = cs->clone();
+ copy->apply_rw(&filter);
+ checkArcs(*copy);
+}
+
+template<>
+template<>
+void object::test<31>()
+{
+ set_test_name("getArcs() after apply_rw(CoordinateSequenceFilter&)");
+
+ struct ScaleFilter : public geos::geom::CoordinateSequenceFilter {
+ explicit ScaleFilter(double p_factor) : factor(p_factor) {}
+
+ void filter_rw(CoordinateSequence& seq, std::size_t i) override
+ {
+ auto& c = seq.getAt<XY>(i);
+ c.x *= factor;
+ c.y *= factor;
+ }
+
+ bool isDone() const override
+ {
+ return false;
+ }
+
+ bool isGeometryChanged() const override
+ {
+ return factor != 1;
+ }
+
+ double factor;
+ };
+
+ // coordinates modified in place
+ ScaleFilter scale(2);
+ cs_->getLength(); // create arcs, computing their center, radius and orientation
+ cs_->apply_rw(scale);
+ checkArcs(*cs_);
+
+ // coordinates unchanged, but copied before the filter is applied because they are shared
+ ScaleFilter identity(1);
+ auto cs = factory_->createCircularString(cs_->getSharedCoordinates());
+ cs->getLength(); // create arcs, computing their center, radius and orientation
+ cs->apply_rw(identity);
+ checkArcs(*cs);
+ cs_.reset(); // destroy the coordinates used by cs before apply_rw()
+ checkArcs(*cs);
+}
+
+template<>
+template<>
+void object::test<32>()
+{
+ set_test_name("getArcs() after geometryChanged()");
+
+ cs_->getLength(); // create arcs, computing their center, radius and orientation
+
+ auto& pts = *const_cast<CoordinateSequence*>(cs_->getCoordinatesRO());
+ pts.setAt(XY(1, 2), 1);
+ cs_->geometryChanged();
+
+ checkArcs(*cs_);
+}
+
}
-----------------------------------------------------------------------
Summary of changes:
NEWS.md | 1 +
include/geos/geom/CircularString.h | 9 ++
include/geos/geom/CompoundCurve.h | 5 +
src/geom/CircularArc.cpp | 22 +----
src/geom/CircularString.cpp | 18 ++++
tests/unit/geom/CircularArcTest.cpp | 28 ++++++
tests/unit/geom/CircularStringTest.cpp | 170 +++++++++++++++++++++++++++++++++
tests/unit/geom/CompoundCurveTest.cpp | 26 +++++
8 files changed, 259 insertions(+), 20 deletions(-)
hooks/post-receive
--
GEOS
More information about the geos-commits
mailing list