From 444d23b49296cbd5aba926fb75b2fb4cb1ea8764 Mon Sep 17 00:00:00 2001 From: Daniel Baston Date: Tue, 17 Jan 2023 19:41:03 -0500 Subject: [PATCH] IndexedPointInAreaLocator: Support all CoordinateSequence dimensions The previous implementation relied on coordinates in all indexed segments being offset by 24 bytes in order to optimize index construction and query time. This commit retains the same performance performance while allowing offsets of 16, 24, or 32 bytes to accommodate the different coordinate types. --- .../locate/IndexedPointInAreaLocator.h | 30 ++++--- .../locate/IndexedPointInAreaLocator.cpp | 4 +- .../locate/IndexedPointInAreaLocatorTest.cpp | 89 +++++++++++++++++++ 3 files changed, 112 insertions(+), 11 deletions(-) create mode 100644 tests/unit/algorithm/locate/IndexedPointInAreaLocatorTest.cpp diff --git a/include/geos/algorithm/locate/IndexedPointInAreaLocator.h b/include/geos/algorithm/locate/IndexedPointInAreaLocator.h index c9ddeeb7f2..44ddf88a16 100644 --- a/include/geos/algorithm/locate/IndexedPointInAreaLocator.h +++ b/include/geos/algorithm/locate/IndexedPointInAreaLocator.h @@ -54,22 +54,32 @@ namespace locate { // geos::algorithm::locate class GEOS_DLL IndexedPointInAreaLocator : public PointOnGeometryLocator { private: struct SegmentView { - SegmentView(const geom::Coordinate* p_p0, const geom::Coordinate* p_p1) : m_p0(p_p0) { - // All GEOS CoordinateSequences store their coordinates sequentially. - // Should that ever change, this assert will fail. - (void) p_p1; - assert(p_p0 + 1 == p_p1); + SegmentView(const geom::CoordinateXY* p0, const geom::CoordinateXY* p1) { + // There is a significant performance benefit in fitting our + // line segment into 8 bytes (about 15-20%). Because we know that + // p1 follows p0 in a CoordinateSequence, we know that the address + // of p1 is 16, 24, or 32 bytes greater than the address of p0. + // By packing this offset into the least significant bits of p0, + // we can retrieve both p0 and p1 while only using 8 byytes. + std::size_t os = static_cast(reinterpret_cast(p1) - reinterpret_cast(p0)) - 2u; + m_p0 = reinterpret_cast(p0) | os; + + assert(&this->p0() == p0); + assert(&this->p1() == p1); } - const geom::Coordinate& p0() const { - return *m_p0; + const geom::CoordinateXY& p0() const { + auto ret = reinterpret_cast(m_p0 >> 2 << 2); + return *ret; } - const geom::Coordinate& p1() const { - return *(m_p0 + 1); + const geom::CoordinateXY& p1() const { + auto offset = (m_p0 & 0x03) + 2; + auto ret = reinterpret_cast(reinterpret_cast(m_p0 >> 2 << 2) + offset); + return *ret; } - const geom::Coordinate* m_p0; + std::size_t m_p0; }; class IntervalIndexedGeometry { diff --git a/src/algorithm/locate/IndexedPointInAreaLocator.cpp b/src/algorithm/locate/IndexedPointInAreaLocator.cpp index 0182903efe..0382c5deb8 100644 --- a/src/algorithm/locate/IndexedPointInAreaLocator.cpp +++ b/src/algorithm/locate/IndexedPointInAreaLocator.cpp @@ -32,6 +32,8 @@ #include #include +using geos::geom::CoordinateXY; + namespace geos { namespace algorithm { namespace locate { @@ -73,7 +75,7 @@ void IndexedPointInAreaLocator::IntervalIndexedGeometry::addLine(const geom::CoordinateSequence* pts) { for(std::size_t i = 1, ni = pts->size(); i < ni; i++) { - SegmentView seg(&pts->getAt(i-1), &pts->getAt(i)); + SegmentView seg(&pts->getAt(i-1), &pts->getAt(i)); auto r = std::minmax(seg.p0().y, seg.p1().y); index.insert(index::strtree::Interval(r.first, r.second), seg); diff --git a/tests/unit/algorithm/locate/IndexedPointInAreaLocatorTest.cpp b/tests/unit/algorithm/locate/IndexedPointInAreaLocatorTest.cpp new file mode 100644 index 0000000000..db9cadc56c --- /dev/null +++ b/tests/unit/algorithm/locate/IndexedPointInAreaLocatorTest.cpp @@ -0,0 +1,89 @@ +#include +// geos +#include +#include +#include + +// std +#include +#include +#include + +using geos::geom::CoordinateXY; +using geos::geom::CoordinateSequence; +using geos::geom::GeometryFactory; +using geos::geom::Location; +using geos::algorithm::locate::IndexedPointInAreaLocator; + +namespace tut { +// +// Test Group +// + +// dummy data, not used +struct test_indexedpointinarealocator_data { + const GeometryFactory& factory_ = *GeometryFactory::getDefaultInstance(); +}; + +typedef test_group group; +typedef group::object object; + +group test_indexedpointinarealocator_group("geos::algorithm::locate::IndexedPointInAreaLocator"); + +// Test all CoordinateSequence dimensions +template<> +template<> +void object::test<1> +() +{ + // XY + CoordinateSequence seq_xy = CoordinateSequence::XY(0); + seq_xy.add(0.0, 0.0); + seq_xy.add(1.0, 0.0); + seq_xy.add(1.0, 1.0); + seq_xy.add(0.0, 1.0); + seq_xy.add(0.0, 0.0); + auto ls_xy = factory_.createLineString(seq_xy.clone()); + IndexedPointInAreaLocator ipa_xy(*ls_xy); + + CoordinateXY pt_boundary(0.5, 0); + CoordinateXY pt_interior(0.5, 0.5); + CoordinateXY pt_exterior(1.5, 0.5); + + ensure_equals(ipa_xy.locate(&pt_boundary), Location::BOUNDARY); + ensure_equals(ipa_xy.locate(&pt_interior), Location::INTERIOR); + ensure_equals(ipa_xy.locate(&pt_exterior), Location::EXTERIOR); + + // XYZ + CoordinateSequence seq_xyz = CoordinateSequence::XYZ(0); + seq_xyz.add(seq_xy); + auto ls_xyz = factory_.createLineString(seq_xyz.clone()); + + IndexedPointInAreaLocator ipa_xyz(*ls_xy); + ensure_equals(ipa_xyz.locate(&pt_boundary), Location::BOUNDARY); + ensure_equals(ipa_xyz.locate(&pt_interior), Location::INTERIOR); + ensure_equals(ipa_xyz.locate(&pt_exterior), Location::EXTERIOR); + + // XYM + CoordinateSequence seq_xym = CoordinateSequence::XYM(0); + seq_xym.add(seq_xy); + auto ls_xym = factory_.createLineString(seq_xym.clone()); + + IndexedPointInAreaLocator ipa_xym(*ls_xy); + ensure_equals(ipa_xym.locate(&pt_boundary), Location::BOUNDARY); + ensure_equals(ipa_xym.locate(&pt_interior), Location::INTERIOR); + ensure_equals(ipa_xym.locate(&pt_exterior), Location::EXTERIOR); + + // XYZM + CoordinateSequence seq_xyzm = CoordinateSequence::XYZM(0); + seq_xyzm.add(seq_xy); + auto ls_xyzm = factory_.createLineString(seq_xyzm.clone()); + + IndexedPointInAreaLocator ipa_xyzm(*ls_xy); + ensure_equals(ipa_xyzm.locate(&pt_boundary), Location::BOUNDARY); + ensure_equals(ipa_xyzm.locate(&pt_interior), Location::INTERIOR); + ensure_equals(ipa_xyzm.locate(&pt_exterior), Location::EXTERIOR); +} + +} +