Skip to content

Conversation

@Kontinuation
Copy link
Contributor

@Kontinuation Kontinuation commented Nov 4, 2025

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.
  • I ran cargo fmt

Fix #86

This is a follow-up of #87. We refactored the code to remove the buf field from Wkb and add buf methods for geometry types to return sliced WKB buffers. There are also some refactorings to simplify various offset computations included in this PR.

@Kontinuation Kontinuation force-pushed the refactor-buf branch 2 times, most recently from 32a6552 to 598f9d8 Compare November 4, 2025 18:35
@Kontinuation Kontinuation marked this pull request as ready for review November 4, 2025 18:40
/// See page 65 of <https://portal.ogc.org/files/?artifact_id=25355>.
#[derive(Debug, Clone, Copy)]
pub struct Coord<'a> {
/// The underlying WKB buffer
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to add comments here saying that this buffer is solely the length of one coordinate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we do not have this requirement for Coord here. The Coords are constructed using buffers with trailing data, and slicing is only handled in Coord::coord_slice.

I can also slice buf when constructing the Coord, which will make Coord.buf consistent with other types: buf contains only the data needed by that object.

impl<'a> Coord<'a> {
pub(crate) fn new(buf: &'a [u8], byte_order: Endianness, offset: u64, dim: Dimension) -> Self {
pub(crate) fn new(buf: &'a [u8], byte_order: Endianness, dim: Dimension) -> Self {
Self {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be descriptive to add a

debug_assert_eq!(buf.length(), dim.size() * F64_WIDTH)

in this new()?

🤷‍♂️

@kylebarron kylebarron merged commit 3158e62 into georust:main Nov 7, 2025
1 check passed
@kylebarron
Copy link
Member

Thanks!

@yutannihilation
Copy link

Awesome!

@jiayuasu
Copy link

@kylebarron Hi Kyle, can you create a new release? We need this to make a new release of SedonaDB. Thank you!

@kylebarron
Copy link
Member

I've been busy with conferences and vacation; I just published 0.9.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wkb::new() should slice the buffer by the actual length

4 participants