Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CoordinateSequence: create using external buffer #747

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions capi/geos_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,12 @@ extern "C" {
return GEOSCoordSeq_create_r(handle, size, dims);
}

CoordinateSequence*
GEOSCoordSeq_createFromBuffer(double* buf, unsigned int size, int hasZ, int hasM)
{
return GEOSCoordSeq_createFromBuffer_r(handle, buf, size, hasZ, hasM);
}

CoordinateSequence*
GEOSCoordSeq_copyFromBuffer(const double* buf, unsigned int size, int hasZ, int hasM)
{
Expand Down
29 changes: 29 additions & 0 deletions capi/geos_c.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,14 @@ extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_create_r(
unsigned int size,
unsigned int dims);

/** \see GEOSCoordSeq_createFromBuffer */
extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_createFromBuffer_r(
GEOSContextHandle_t handle,
double* buf,
unsigned int size,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't that be size_t ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if the other coordinate sequence functions did not use unsigned int

int hasZ,
int hasM);

/** \see GEOSCoordSeq_copyFromBuffer */
extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_copyFromBuffer_r(
GEOSContextHandle_t handle,
Expand Down Expand Up @@ -2034,7 +2042,24 @@ extern void GEOS_DLL GEOSFree(void *buffer);
extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_create(unsigned int size, unsigned int dims);

/**
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge conflict markers

* Create a coordinate sequence by copying from an interleaved buffer of doubles (e.g., XYXY or XYZXYZ)
=======
* Create a coordinate sequence from a buffer of interleaved doubles (XYXY, XYZXYZ, etc.)
* If possible, the CoordinateSequence will use the values directly from the buffer without
* copying. (Currently, this is supported for XYZ and XYZM buffers only.)
*
* \param buf pointer to buffer
* \param size number of coordinates in the sequence
* \param hasZ does buffer have Z values?
* \param hasM does buffer have M values?
* \return the sequence or NULL on exception
*/
extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_createFromBuffer(double* buf, unsigned int size, int hasZ, int hasM);

/**
* Create a coordinate sequence by copying from a buffer of doubles (XYXY, XYZXYZ, etc.)
>>>>>>> 720bccf9e (CoordinateSequence: create using external buffer)
* \param buf pointer to buffer
* \param size number of coordinates in the sequence
* \param hasZ does buffer have Z values?
Expand All @@ -2055,7 +2080,11 @@ extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_copyFromBuffer(const double* buf
extern GEOSCoordSequence GEOS_DLL *GEOSCoordSeq_copyFromArrays(const double* x, const double* y, const double* z, const double* m, unsigned int size);

/**
<<<<<<< HEAD
* Copy the contents of a coordinate sequence to an interleaved buffer of doubles (e.g., XYXY or XYZXYZ)
=======
* Copy the contents of a coordinate sequence to a buffer of doubles (XYXY, XYZXYZ, etc.)
>>>>>>> 720bccf9e (CoordinateSequence: create using external buffer)
* \param s sequence to copy
* \param buf buffer to which coordinates should be copied
* \param hasZ copy Z values to buffer?
Expand Down
8 changes: 8 additions & 0 deletions capi/geos_ts_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,14 @@ extern "C" {
});
}

CoordinateSequence*
GEOSCoordSeq_createFromBuffer_r(GEOSContextHandle_t extHandle, double* buf, unsigned int size, int hasZ, int hasM)
{
return execute(extHandle, [&]() {
return new CoordinateSequence(buf, size, hasZ, hasM);
});
}

CoordinateSequence*
GEOSCoordSeq_copyFromBuffer_r(GEOSContextHandle_t extHandle, const double* buf, unsigned int size, int hasZ, int hasM)
{
Expand Down
29 changes: 22 additions & 7 deletions include/geos/geom/CoordinateSequence.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <geos/geom/Coordinate.h> // for applyCoordinateFilter
#include <geos/geom/CoordinateSequenceIterator.h>
#include <geos/util/Vector.h>

#include <cassert>
#include <vector>
Expand Down Expand Up @@ -89,12 +90,28 @@ class GEOS_DLL CoordinateSequence {
* are not actually stored in the sequence.
*
* @param size size of the sequence to create
* @param hasz true if the stored
* @param hasm
* @param initialize
* @param hasz true if the sequence should store z values
* @param hasm true if the sequence should store m values
* @param initialize true if the sequence should be initialized to default coordinate values
*/
CoordinateSequence(std::size_t size, bool hasz, bool hasm, bool initialize = true);

/**
* Create a CoordinateSequence from an externally-owned buffer
* of packed Coordinates. If Coordinates are added to the CoordinateSequence
* or the CoordinateSequence requires repacking, the values will
* be copied into a new buffer owned by this CoordinateSeuqnce.
* Code using a CoordinateSequence constructed in this way must not
* attempt to access references to coordinates with dimensions that
* are not actually stored in the sequence.
*
* @param buf buffer of interleaved coordinates
* @param size number of coordinates in the buffer
* @param hasz true if the buffer has z values
* @param hasm true if the buffer has m values
*/
CoordinateSequence(double* buf, std::size_t size, bool hasz, bool hasm);

/**
* Create a CoordinateSequence from a list of XYZ coordinates.
* Code using the sequence may only access references to CoordinateXY
Expand Down Expand Up @@ -723,7 +740,7 @@ class GEOS_DLL CoordinateSequence {
}

private:
std::vector<double> m_vect; // Vector to store values
geos::util::Vector<double> m_vect; // Vector to store values

uint8_t m_stride; // Stride of stored values, corresponding to underlying type

Expand All @@ -742,9 +759,7 @@ class GEOS_DLL CoordinateSequence {
}

void make_space(std::size_t pos, std::size_t n) {
m_vect.insert(std::next(m_vect.begin(), static_cast<std::ptrdiff_t>(pos * stride())),
m_stride * n,
DoubleNotANumber);
m_vect.make_space(m_vect.begin() + pos * stride(), n * stride());
}

std::uint8_t stride() const {
Expand Down
256 changes: 256 additions & 0 deletions include/geos/util/Vector.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
/**********************************************************************
*
* GEOS - Geometry Engine Open Source
* http://geos.osgeo.org
*
* Copyright (C) 2022 ISciences LLC
*
* This is free software; you can redistribute and/or modify it under
* the terms of the GNU Lesser General Public Licence as published
* by the Free Software Foundation.
* See the COPYING file for more information.
*
**********************************************************************/

#pragma once

#include <cstdint>
#include <cstring>
#include <iterator>

namespace geos {
namespace util {

/**
* \brief
* A partial re-implementation of std::vector intended for use by
* CoordinateSequence. Unlike std::vector, it can be backed by an
* external buffer or an internally-owned buffed.
*/
template<typename T>
class Vector {

public:
Vector() :
m_buf(nullptr),
m_capacity(0),
m_size(0)
{}

Vector(std::size_t sz) :
m_buf(nullptr),
m_capacity(0),
m_size(static_cast<std::uint32_t>(0))
{
resize(sz);
}

Vector(std::size_t sz, T* buf) :
m_buf(buf),
m_capacity(0),
m_size(static_cast<std::uint32_t>(sz))
{}

Vector(const Vector& other) : Vector() {
if (!other.empty()) {
m_buf = make_space(nullptr, other.size());
std::memcpy(m_buf, other.m_buf, other.size()*sizeof(T));
}
}

Vector& operator=(const Vector& other) {
clear();

if (!other.empty()) {
m_buf = make_space(nullptr, other.size());
std::memcpy(m_buf, other.m_buf, other.size()*sizeof(T));
}

return *this;
}

Vector(Vector&& other) :
m_buf(other.m_buf),
m_capacity(other.m_capacity),
m_size(other.m_size)
{
other.m_buf = nullptr;
other.m_capacity = 0;
other.m_size = 0;
}

Vector& operator=(Vector&& other) {
if (owned()) {
delete[] m_buf;
}

m_buf = other.m_buf;
m_capacity = other.m_capacity;
m_size = other.m_size;
other.m_buf = nullptr;
other.m_capacity = 0;
other.m_size = 0;

return *this;
}

~Vector() {
if (owned()) {
delete[] m_buf;
}
}

void push_back(T item) {
growIfNeeded(1);
assert(size() < capacity());
m_buf[m_size++] = item;
}

void pop_back() {
assert(size() > 0);
m_size--;
}

T* make_space(T* pos, std::size_t n) {
auto loc = pos == nullptr ? 0 : pos - begin();
growIfNeeded(n);
pos = begin() + loc;

if (pos != end()) {
auto num_to_move = static_cast<std::size_t>(end() - pos);
std::memmove(pos + n, pos, num_to_move*sizeof(T));
}
m_size += static_cast<std::uint32_t>(n);

return pos;
}

void insert(T* pos, std::size_t n, const T& value) {
pos = make_space(pos, n);
std::fill(pos, pos + n, value);
}

template<typename Iter>
void insert(T* pos, Iter from, Iter to) {
auto n = static_cast<std::size_t>(to - from);

if (from >= begin() && from < end()) {
// from and to may be invalidated
auto from_n = from - begin();
auto to_n = to - begin();

pos = make_space(pos, n);

from = begin() + from_n;
to = begin() + to_n;
} else {
pos = make_space(pos, n);
}

std::copy(from, to, pos);
}

template<typename Iter>
void assign(Iter from, Iter to) {
assert(static_cast<std::size_t>(to - from) <= size());
std::copy(from, to, begin());
}

void reserve(std::size_t sz) {
if (sz <= capacity()) {
return;
}

T* tmp = sz > 0 ? new T[sz] : nullptr;
if (tmp && !empty()) {
std::memcpy(tmp, m_buf, m_size * sizeof(T));
}
if (owned()) {
delete[] m_buf;
}
m_buf = tmp;
m_capacity = static_cast<std::uint32_t>(sz);
}

void resize(std::size_t sz) {
reserve(sz);
m_size = static_cast<std::uint32_t>(sz);
}

void clear() {
m_size = 0;
}

std::size_t capacity() const {
return m_capacity;
}

std::size_t size() const {
return m_size;
}

bool empty() const {
return m_size == 0;
}

bool owned() const {
return data() == nullptr || !(capacity() == 0 && size() > 0);
}

const T& operator[](std::size_t i) const {
return *(data() + i);
}

T& operator[](std::size_t i) {
return *(data() + i);
}

T* release() {
m_capacity = 0;
return m_buf;
}

T* data() {
return m_buf;
}

const T* data() const {
return m_buf;
}

T* begin() {
return data();
};

T* end() {
return data() + size();
}

const T* begin() const {
return data();
};

const T* end() const {
return data() + size();
}

private:

void growIfNeeded(std::size_t num_to_add) {
if (size() + num_to_add > capacity()) {
auto new_capacity = capacity() == 0 ?
std::max(size() + num_to_add, static_cast<std::size_t>(4)) :
static_cast<std::size_t>(static_cast<double>(capacity()) * 1.5);
new_capacity = std::max(new_capacity, capacity() + num_to_add);
reserve(new_capacity);
}
}

T* m_buf;
std::uint32_t m_capacity;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use size_t for m_capacity and m_size ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C API uses 32 bits for sizes and incides, I'm doubtful a coordinate sequence with > 1 billion points would function in practice, and I'm trying to be sensitive to the object size of Point, whose API requires that it be backed by a CoordinateSequence.

std::uint32_t m_size;
};


}
}
Loading