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

Remove unaligned vertex accesses #175

Merged
merged 5 commits into from
Nov 9, 2023
Merged
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
2 changes: 1 addition & 1 deletion .github/workflows/Linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ jobs:
openssl: 0

- name: Setup vcpkg
uses: lukka/run-vcpkg@v11
uses: lukka/run-vcpkg@v11.1
with:
vcpkgGitCommitId: 501db0f17ef6df184fcdbfbe0f87cde2313b6ab1

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/MainDistributionPipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ concurrency:
jobs:
duckdb-stable-build:
name: Build extension binaries
uses: duckdb/duckdb/.github/workflows/_extension_distribution.yml@main
uses: duckdb/duckdb/.github/workflows/_extension_distribution.yml@60ddc316ca0c1585f14d55aa73f9db59d8fc05d1
with:
duckdb_version: v0.9.1
extension_name: spatial
Expand Down
3 changes: 1 addition & 2 deletions spatial/include/spatial/core/geometry/geometry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ class Point {
}
string ToString() const;
bool IsEmpty() const;
Vertex &GetVertex();
const Vertex &GetVertex() const;
Vertex GetVertex() const;

const VertexVector &Vertices() const {
return vertices;
Expand Down
44 changes: 15 additions & 29 deletions spatial/include/spatial/core/geometry/vertex_vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,39 +66,18 @@ class VertexVector {
public:
uint32_t count;
uint32_t capacity;
Vertex *data;
data_ptr_t data;

explicit VertexVector(Vertex *data, uint32_t count, uint32_t capacity)
explicit VertexVector(data_ptr_t data, uint32_t count, uint32_t capacity)
: count(count), capacity(capacity), data(data) {
}

// Create a VertexVector from an already existing buffer
static VertexVector FromBuffer(Vertex *buffer, uint32_t count) {
static VertexVector FromBuffer(data_ptr_t buffer, uint32_t count) {
VertexVector array(buffer, count, count);
return array;
}

inline Vertex &operator[](uint32_t index) const {
D_ASSERT(index < count);
return data[index];
}

inline Vertex *begin() {
return data;
}

inline Vertex *end() {
return data + count;
}

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

const Vertex *end() const {
return data + count;
}

inline uint32_t Count() const {
return count;
}
Expand All @@ -109,7 +88,18 @@ class VertexVector {

inline void Add(const Vertex &v) {
D_ASSERT(count < capacity);
data[count++] = v;
Store<Vertex>(v, data + count * sizeof(Vertex));
count++;
}

inline void Set(uint32_t index, const Vertex &v) const {
D_ASSERT(index < count);
Store<Vertex>(v, data + index * sizeof(Vertex));
}

inline Vertex Get(uint32_t index) const {
D_ASSERT(index < count);
return Load<Vertex>(data + index * sizeof(Vertex));
}

// Returns the number of bytes that this VertexVector requires to be serialized
Expand All @@ -120,10 +110,6 @@ class VertexVector {
void Serialize(Cursor &cursor) const;
void SerializeAndUpdateBounds(Cursor &cursor, BoundingBox &bbox) const;

inline Vertex *Data() {
return data;
}

double Length() const;
double SignedArea() const;
double Area() const;
Expand Down
27 changes: 12 additions & 15 deletions spatial/src/spatial/core/functions/cast/geometry_cast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static bool GeometryToPoint2DCast(Vector &source, Vector &result, idx_t count, C
if (point.IsEmpty()) {
throw CastException("Cannot cast empty point GEOMETRY to POINT_2D");
}
auto &vertex = point.GetVertex();
auto vertex = point.GetVertex();
return POINT_TYPE {vertex.x, vertex.y};
});
return true;
Expand Down Expand Up @@ -103,8 +103,8 @@ static bool GeometryToLineString2DCast(Vector &source, Vector &result, idx_t cou
ListVector::Reserve(result, total_coords);

for (idx_t i = 0; i < line_size; i++) {
x_data[entry.offset + i] = line.Vertices()[i].x;
y_data[entry.offset + i] = line.Vertices()[i].y;
x_data[entry.offset + i] = line.Vertices().Get(i).x;
y_data[entry.offset + i] = line.Vertices().Get(i).y;
}
return entry;
});
Expand Down Expand Up @@ -182,8 +182,8 @@ static bool GeometryToPolygon2DCast(Vector &source, Vector &result, idx_t count,
ring_entries[total_rings + ring_idx] = ring_entry;

for (idx_t j = 0; j < ring_size; j++) {
x_data[ring_entry.offset + j] = ring.data[j].x;
y_data[ring_entry.offset + j] = ring.data[j].y;
x_data[ring_entry.offset + j] = ring.Get(j).x;
y_data[ring_entry.offset + j] = ring.Get(j).y;
}
total_coords += ring_size;
}
Expand Down Expand Up @@ -217,16 +217,13 @@ static bool Box2DToGeometryCast(Vector &source, Vector &result, idx_t count, Cas

auto geom = lstate.factory.CreatePolygon(1, &capacity);
auto &shell = geom.Ring(0);
shell.data[0].x = minx;
shell.data[0].y = miny;
shell.data[1].x = maxx;
shell.data[1].y = miny;
shell.data[2].x = maxx;
shell.data[2].y = maxy;
shell.data[3].x = minx;
shell.data[3].y = maxy;
shell.data[4].x = minx;
shell.data[4].y = miny;

shell.Set(0, Vertex(minx, miny));
shell.Set(1, Vertex(maxx, miny));
shell.Set(2, Vertex(maxx, maxy));
shell.Set(3, Vertex(minx, maxy));
shell.Set(4, Vertex(minx, miny));

shell.count = 5;
return lstate.factory.Serialize(result, Geometry(geom));
});
Expand Down
4 changes: 2 additions & 2 deletions spatial/src/spatial/core/functions/scalar/st_asgeojson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class JSONAllocator {
static void VerticesToGeoJSON(const VertexVector &vertices, yyjson_mut_doc *doc, yyjson_mut_val *arr) {
// TODO: If the vertexvector is empty, do we null, add an empty array or a pair of NaN?
for (uint32_t i = 0; i < vertices.count; i++) {
auto &vertex = vertices[i];
auto vertex = vertices.Get(i);
auto coord = yyjson_mut_arr(doc);
yyjson_mut_arr_add_real(doc, coord, vertex.x);
yyjson_mut_arr_add_real(doc, coord, vertex.y);
Expand All @@ -75,7 +75,7 @@ static void ToGeoJSON(const Point &point, yyjson_mut_doc *doc, yyjson_mut_val *o
yyjson_mut_obj_add_val(doc, obj, "coordinates", coords);

if (!point.IsEmpty()) {
auto &vertex = point.GetVertex();
auto vertex = point.GetVertex();
yyjson_mut_arr_add_real(doc, coords, vertex.x);
yyjson_mut_arr_add_real(doc, coords, vertex.y);
}
Expand Down
30 changes: 14 additions & 16 deletions spatial/src/spatial/core/functions/scalar/st_dump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ static void DumpFunction(DataChunk &args, ExpressionState &state, Vector &result
idx_t total_geom_count = 0;
idx_t total_path_count = 0;

for(idx_t out_row_idx = 0; out_row_idx < count; out_row_idx++) {
for (idx_t out_row_idx = 0; out_row_idx < count; out_row_idx++) {
auto in_row_idx = geom_format.sel->get_index(out_row_idx);

if (!geom_format.validity.RowIsValid(in_row_idx)) {
Expand All @@ -39,38 +39,35 @@ static void DumpFunction(DataChunk &args, ExpressionState &state, Vector &result

stack.emplace_back(geometry, vector<int32_t>());

while(!stack.empty()) {
while (!stack.empty()) {
auto current = stack.back();
auto current_geom = std::get<0>(current);
auto current_path = std::get<1>(current);

stack.pop_back();

if(current_geom.Type() == GeometryType::MULTIPOINT) {
if (current_geom.Type() == GeometryType::MULTIPOINT) {
auto mpoint = current_geom.GetMultiPoint();
for (int32_t i = 0; i < mpoint.Count(); i++) {
auto path = current_path;
path.push_back(i + 1); // path is 1-indexed
stack.emplace_back(mpoint[i], path);
}
}
else if(current_geom.Type() == GeometryType::MULTILINESTRING) {
} else if (current_geom.Type() == GeometryType::MULTILINESTRING) {
auto mline = current_geom.GetMultiLineString();
for (int32_t i = 0; i < mline.Count(); i++) {
auto path = current_path;
path.push_back(i + 1);
stack.emplace_back(mline[i], path);
}
}
else if(current_geom.Type() == GeometryType::MULTIPOLYGON) {
} else if (current_geom.Type() == GeometryType::MULTIPOLYGON) {
auto mpoly = current_geom.GetMultiPolygon();
for (int32_t i = 0; i < mpoly.Count(); i++) {
auto path = current_path;
path.push_back(i + 1);
stack.emplace_back(mpoly[i], path);
}
}
else if (current_geom.Type() == GeometryType::GEOMETRYCOLLECTION) {
} else if (current_geom.Type() == GeometryType::GEOMETRYCOLLECTION) {
auto collection = current_geom.GetGeometryCollection();
for (int32_t i = 0; i < collection.Count(); i++) {
auto path = current_path;
Expand Down Expand Up @@ -105,7 +102,7 @@ static void DumpFunction(DataChunk &args, ExpressionState &state, Vector &result
auto &result_path_vec = result_list_children[1];

auto geom_data = FlatVector::GetData<string_t>(*result_geom_vec);
for(idx_t i = 0; i < geom_length; i++) {
for (idx_t i = 0; i < geom_length; i++) {
// Write the geometry
auto &item_blob = std::get<0>(items[i]);
geom_data[geom_offset + i] = lstate.factory.Serialize(*result_geom_vec, item_blob);
Expand All @@ -128,24 +125,25 @@ static void DumpFunction(DataChunk &args, ExpressionState &state, Vector &result
auto &path_data_vec = ListVector::GetEntry(*result_path_vec);
auto path_data = FlatVector::GetData<int32_t>(path_data_vec);

for(idx_t j = 0; j < path_length; j++) {
for (idx_t j = 0; j < path_length; j++) {
path_data[path_offset + j] = path[j];
}
}
}

if(count == 1) {
if (count == 1) {
result.SetVectorType(VectorType::CONSTANT_VECTOR);
}
}

void CoreScalarFunctions::RegisterStDump(DatabaseInstance &db) {
ScalarFunctionSet set("ST_Dump");

set.AddFunction(ScalarFunction({GeoTypes::GEOMETRY()},
LogicalType::LIST(LogicalType::STRUCT({{"geom", GeoTypes::GEOMETRY()}, {"path", LogicalType::LIST(LogicalType::INTEGER)}})),
DumpFunction,
nullptr, nullptr, nullptr, GeometryFunctionLocalState::Init));
set.AddFunction(
ScalarFunction({GeoTypes::GEOMETRY()},
LogicalType::LIST(LogicalType::STRUCT(
{{"geom", GeoTypes::GEOMETRY()}, {"path", LogicalType::LIST(LogicalType::INTEGER)}})),
DumpFunction, nullptr, nullptr, nullptr, GeometryFunctionLocalState::Init));

ExtensionUtil::RegisterFunction(db, set);
}
Expand Down
2 changes: 1 addition & 1 deletion spatial/src/spatial/core/functions/scalar/st_endpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static void GeometryEndPointFunction(DataChunk &args, ExpressionState &state, Ve
return string_t();
}

auto &point = line.Vertices()[point_count - 1];
auto point = line.Vertices().Get(point_count - 1);
return lstate.factory.Serialize(result, Geometry(lstate.factory.CreatePoint(point.x, point.y)));
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ static void GeometryExteriorRingFunction(DataChunk &args, ExpressionState &state

auto line = lstate.factory.CreateLineString(num_points);
for (uint32_t i = 0; i < num_points; i++) {
line.Vertices().Add(shell[i]);
line.Vertices().Add(shell.Get(i));
}
return lstate.factory.Serialize(result, Geometry(line));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ static void BoxFlipCoordinatesFunction(DataChunk &args, ExpressionState &state,
//------------------------------------------------------------------------------
static void FlipVertexVector(VertexVector &vertices) {
for (idx_t i = 0; i < vertices.count; i++) {
std::swap(vertices[i].x, vertices[i].y);
auto vertex = vertices.Get(i);
std::swap(vertex.x, vertex.y);
vertices.Set(i, vertex);
}
}
static void FlipGeometry(Point &point) {
Expand Down
15 changes: 8 additions & 7 deletions spatial/src/spatial/core/functions/scalar/st_makepolygon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static void MakePolygonFromRingsFunction(DataChunk &args, ExpressionState &state
}

auto &shell_verts = shell.Vertices();
if (shell_verts[0] != shell_verts[shell_vert_count - 1]) {
if (!shell_verts.IsClosed()) {
throw InvalidInputException(
"ST_MakePolygon shell must be closed (first and last vertex must be equal)");
}
Expand Down Expand Up @@ -70,7 +70,7 @@ static void MakePolygonFromRingsFunction(DataChunk &args, ExpressionState &state
}

auto &ring_verts = hole.Vertices();
if (ring_verts[0] != ring_verts[hole_count - 1]) {
if (!ring_verts.IsClosed()) {
throw InvalidInputException(StringUtil::Format(
"ST_MakePolygon hole #%lu must be closed (first and last vertex must be equal)", hole_idx + 1));
}
Expand All @@ -84,8 +84,9 @@ static void MakePolygonFromRingsFunction(DataChunk &args, ExpressionState &state
for (idx_t ring_idx = 0; ring_idx < rings.size(); ring_idx++) {
auto &new_ring = rings[ring_idx];
auto &poly_ring = polygon.Ring(ring_idx);
for (auto &v : new_ring.Vertices()) {
poly_ring.Add(v);

for (auto i = 0; i < new_ring.Vertices().Count(); i++) {
poly_ring.Add(new_ring.Vertices().Get(i));
}
}

Expand All @@ -111,13 +112,13 @@ static void MakePolygonFromShellFunction(DataChunk &args, ExpressionState &state
}

auto &line_verts = line.Vertices();
if (line_verts[0] != line_verts[line_count - 1]) {
if (!line_verts.IsClosed()) {
throw InvalidInputException("ST_MakePolygon shell must be closed (first and last vertex must be equal)");
}

auto polygon = lstate.factory.CreatePolygon(1, &line_count);
for (auto &v : line_verts) {
polygon.Shell().Add(v);
for (uint32_t i = 0; i < line_count; i++) {
polygon.Shell().Add(line_verts.Get(i));
}

return lstate.factory.Serialize(result, Geometry(polygon));
Expand Down
4 changes: 2 additions & 2 deletions spatial/src/spatial/core/functions/scalar/st_perimeter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ static double PolygonPerimeter(const Polygon &poly) {
double perimeter = 0;
for (auto &ring : poly.Rings()) {
for (uint32_t i = 0; i < ring.Count() - 1; i++) {
auto &v1 = ring[i];
auto &v2 = ring[i + 1];
auto v1 = ring.Get(i);
auto v2 = ring.Get(i + 1);
perimeter += std::sqrt(std::pow(v1.x - v2.x, 2) + std::pow(v1.y - v2.y, 2));
}
}
Expand Down
2 changes: 1 addition & 1 deletion spatial/src/spatial/core/functions/scalar/st_pointn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static void GeometryPointNFunction(DataChunk &args, ExpressionState &state, Vect
}

auto actual_index = index < 0 ? point_count + index : index - 1;
auto &point = line.Vertices()[actual_index];
auto point = line.Vertices().Get(actual_index);
return lstate.factory.Serialize(result, Geometry(lstate.factory.CreatePoint(point.x, point.y)));
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static void GeometryStartPointFunction(DataChunk &args, ExpressionState &state,
return string_t();
}

auto &point = line.Vertices()[0];
auto point = line.Vertices().Get(0);
return lstate.factory.Serialize(result, Geometry(lstate.factory.CreatePoint(point.x, point.y)));
});
}
Expand Down
Loading
Loading