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

Implement 3D and Measure support for geo types #742

Closed
wants to merge 13 commits into from
Closed

Conversation

nyurik
Copy link
Member

@nyurik nyurik commented Feb 22, 2022

UPDATE: most of this pr has been moved to #797 (all of geo-types changes). 797 should be merged and published first. This PR will need to be updated once 797 is published. I will mark this as a draft in the mean time.


This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

Many other pending PRs are included here - most of them can be merged without affecting existing users, and will make reviewing this PR easier. See the list below.

geo-types restructuring

All geo type structs have been renamed from Foo<T>(...) to FooTZM<T,Z,M>(...), and several type aliases were added:

// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineStringTZM<T: CoordNum, Z: ZCoord, M: Measure>(pub Vec<CoordTZM<T, Z, M>>);
pub type LineString<T> = LineStringTZM<T, NoValue, NoValue>;
pub type LineStringM<T, M> = LineStringTZM<T, NoValue, M>;
pub type LineStringZ<T> = LineStringTZM<T, T, NoValue>;
pub type LineStringZM<T, M> = LineStringTZM<T, T, M>;

NoValue magic

NoValue is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a Z or M value can be manipulated without checking if it is actually there. This code works for Z and M being either a number or a NoValue:

pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for CoordTZM<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}

Variant algorithm implementations

Function implementations can keep just the original 2D <T> variant, or add support for 3D <Z> and/or the Measure <M>. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure (Z is set to NoValue by LineM type alias.

impl<T: CoordNum, M: Measure> LineM<T, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}

Breaking changes

Tuple constructors

It will not be possible to use implicit type constructor for tuples because type aliases do not support them. Most geo types will have a new(...) fn if they didn't have one before.

// old
MultiPoint(vec![...]);
...iter().map(MultiPoint);
// new
MultiPoint::new(vec![...]);
...iter().map(MultiPoint::new);

Type destructors

Destructuring assignments seem to require real type rather than an alias.

// old
let Point(c) = p;
// new
let PointTZM(c) = p;

Related PRs

This PR includes #778, #782, #783, #784, #786, #787, #788, #775, #772, #785 which can be easily merged without breaking existing code.

TODO

  • implement some simple algorithm that works with 2d/3d/keeps measurement values
  • implement some simple SRID example

@pka
Copy link
Member

pka commented Feb 22, 2022

Wow! It's astonishing that such a major extension is not more intrusive. For geo-types users, the major changes are probably that they have to use new for construction and x() and y() for access.

One point which should be prototyped probably, is how to implement 2D functions keeping additional dimensions and meta data.

Naming:

  • I would prefer an other word for "Metadata"/"meta". Additional dimensions like "measurement" or "time" are not "meta" for me. There are things like an srid which I would consider as "meta", though.
  • For Point3D or similar, I would prefer the same naming as PostGIS: PointZor PointZM

@lnicola
Copy link
Member

lnicola commented Feb 22, 2022

Yeah, this looks great.

But I wonder how it affects the IDE support? rust-analyzer knows almost nothing about generics.

@nyurik
Copy link
Member Author

nyurik commented Feb 22, 2022

But I wonder how it affects the IDE support? rust-analyzer knows almost nothing about generics.

I haven't had any issues with it in IntelliJ - seems to work well -- e.g. if there is a set_x() implemented just for the D = 2 generic const, it suggests set_x as an available option, whereas it hides it for D = 3. I'm not an expert in VS Code, and it seems the Rust plugin is a bit dated (last commit was in 2021), so can't really say.

@nyurik
Copy link
Member Author

nyurik commented Feb 22, 2022

Naming is hard. @pka I renamed Coordinate3D to CoordinateZ. What should we do about metadata? There are two types of info:

  • data that affects how coordinates are handled, i.e. SRID
  • user-supplied tag data that algorithms do not handle, but try to keep around, i.e. measurements/times/extra tags/...

SRID could (later?) be introduced as a separate generic field, with some other trait magic that implements a default. The tag data could be called something else: tag, extra, info, custom, ...?

@michaelkirk
Copy link
Member

Disclaimer that I've only given this a cursory look. There's a lot of noise in the diff from the upstream commits.

re: the naming of Coord that you've proposed - there is some previous discussion about using that word elsewhere. See #540.
As such, it would be good to reserve that nice short name for the place it's likely to be used the most.

A couple of things I don't understand yet:

  1. Is the proposed metadata field actually required to implement the 3d/4d coordinates, or is that a separable change?

  2. Is your intention to add 3d/4d for all geometry types - e.g. LineString3D, LineString4D or only for the Coordinates?

@nyurik
Copy link
Member Author

nyurik commented Feb 22, 2022

re: the naming of Coord that you've proposed - there is some previous discussion about using that word elsewhere. See #540. As such, it would be good to reserve that nice short name for the place it's likely to be used the most.

Sure, any name is fine here - MultiCoord, CoordStore, GenericCoord, ... cast your votes :)

  1. Is the proposed metadata field actually required to implement the 3d/4d coordinates, or is that a separable change?

No, metadata (to be renamed per above) is not required for 3D, but seems to be requested enough (and common enough in the industry) to introduce at the same time. This reduces code churn because adding M generic is identical to adding D generic, so the same code spots will be changed twice.

  1. Is your intention to add 3d/4d for all geometry types - e.g. LineString3D, LineString4D or only for the Coordinates?

The intention is to add LineStringZ and the rest of them. (per PostGIS convention as also suggested above). I am not an expert in geo algorithms, so this is more of a basis for experimentations, to see how we can adapt existing algorithms to work on multidimensional features, and to possibly introduce 3D-specific ones. This also experiments with adding the extra data, to see which algos can keep that data around if possible.

@michaelkirk
Copy link
Member

No, metadata (to be renamed per above) is not required for 3D, but seems to be requested enough (and common enough in the industry) to introduce at the same time. This reduces code churn because adding M generic is identical to adding D generic, so the same code spots will be changed twice.

Thanks for your consideration of breaking changes. It is indeed nice to consolidate breakage.

Is it clear to you what the behavior of a simple algorithm like bounding_rect would do with respect to the metadata field? Is it just always default?

type MyCoord = Coord<f64, char, 2>;
let a = MyCoord::new(1.0, 2.0, 'a');
let b = MyCoord::new(2.0, 8.0, 'b');
let b = MyCoord::new(10.0, 9.0, 'c');
let ls = line_string![a, b, c];
let rect = ls.bounding_rect();

// Is this what you're planning?
assert_eq!(rect.meta(), Default::default());

It seems kind of at odds with it's utility. e.g. if I'm holding my SRID in meta, maybe I expect that to be preserved in the meta of the output geometry?

You don't have to respond right now, I'm sure you're still thinking about the design. I just hope to avoid a situation where, in the effort to accommodate two new things, it becomes too challenging and then we end up with neither. Spend your time as you like, of course.

@nyurik
Copy link
Member Author

nyurik commented Feb 22, 2022

@michaelkirk actually see #742 (comment) -- i think SRID won't be part of the (what is now known as metadata, but shall be renamed). SRID affects algorithms. "meta" does not, so it is more like an extra/tag thing. And no, no idea of how it will be used -- need to look at how postgis deals with the measurement. One example is ST_AddMeasure. In that doc, note the important comment:

This function supports 3d and will not drop the z-index.

So in PostGIS every function capable of understanding z-index is marked as such.

@nyurik
Copy link
Member Author

nyurik commented Feb 22, 2022

Looking further at various M-related PostGIS functions like ST_FilterByM, they all work with M value that can be compared, extrapolated, etc. That implies that M should be used as where M: CoordNum, but for flexibility it should be separate from the x/y/z type T: CoordNum. SRID on the other hand seems to always be an integer, so we could probably introduce that using the same S: CoordNum construct. The reason for generic is because it allows () as a valid parameter type - effectively removing it from the storage

@nyurik
Copy link
Member Author

nyurik commented Feb 24, 2022

After a lot of experimentation, it seems the constant-generic-based array would be too difficult and pointless to implement. Instead, per a wonderfully quick stackoverflow suggestion, I'm introducing a new NoValue struct with no fields. Using it as a type for z and m values allows far fewer changes - must still use coord! { x, y } instead of Coordinate { x, y }, but no longer need x(), y() functional accessors. Serde support is back in. Now if only i could somehow enforce the type of z to always be either NoValue or the same as the type of x/y... Not certain how to do that yet.

P.S. SRID should be done as one value per feature (point/line/...), not one per coordinate, so removing it for now.

@nyurik
Copy link
Member Author

nyurik commented Mar 16, 2022

Update: stabilizing coordinates and points. Most algorithms will not work if the point/coord has z or m values because algorithms are accepting/returning Point<T> instead of PointTZM<...>. There is one test that shows multi-dimensional usage - contains(). Also note that PointTZM must now be used everywhere to create new instances, e.g. Point(c) is now invalid, and must be replaced with point!(c) or PointTZM(c). This is a trade-off to keep existing function and var declaration signatures like fn foo() -> Point<T>.

Alternatively we can keep the existing Point and extend its generic params: Point<T, Z, M>, plus create type aliases like Point2D<T>. This would allow constructor usage like Point(c), but all var declarations and function returns would have to be changed to either fn foo() -> Point<T,Z,M> or fn foo() -> Point2D<T>.

To simplify migration, some of these changes were moved to #775

/// Test that actually works but only if all types match
assert!(point!(x: 1, y: 2, z: 3, m: 4).contains(&point!(x: 1, y: 2, z: 3, m: 4)));

/// Coordinate storage
pub struct CoordTZM<T: CoordNum, Z: ZCoord, M: Measure> {
    pub x: T,
    pub y: T,
    pub z: Z,
    pub m: M,
}

pub type Coordinate<T> = CoordTZM<T, NoValue, NoValue>;
pub type CoordinateM<T, M> = CoordTZM<T, NoValue, M>;
pub type CoordinateZ<T> = CoordTZM<T, T, NoValue>;
pub type CoordinateZM<T, M> = CoordTZM<T, T, M>;

// points
pub struct PointTZM<T: CoordNum, Z: ZCoord, M: Measure>(pub CoordTZM<T, Z, M>);
pub type Point<T> = PointTZM<T, NoValue, NoValue>;
pub type PointM<T, M> = PointTZM<T, NoValue, M>;
pub type PointZ<T> = PointTZM<T, T, NoValue>;
pub type PointZM<T, M> = PointTZM<T, T, M>;

nyurik added a commit to nyurik/geo that referenced this pull request Mar 17, 2022
Rust does not allow tuple type aliases to be instantiated, so `Point(c)` must always be a real struct, and cannot be a type alias like `type Point<T> = GenericPoint<T,Z,M>`. To make the migration transparent, this PR adds support for a single-expression `point!` macro. Now it can be used in 3 ways:

```
point! { x: <number>, y: <number> }
point!(<x_number>, <y_number>)
point!(<coordinate>)
```

See also georust#742 (comment)

* Replace `Point(c)` with `point!(c)`
* Replace a few `format!("{}", v)` with `v.to_string()`
* Use `Self` or `Self::Output` instead of a concrete type when possible.
nyurik added a commit to nyurik/geo that referenced this pull request Mar 17, 2022
Rust does not allow tuple type aliases to be instantiated, so `Point(c)` must always be a real struct, and cannot be a type alias like `type Point<T> = GenericPoint<T,Z,M>`. To make the migration transparent, this PR adds support for a single-expression `point!` macro. Now it can be used in 3 ways:

```
point! { x: <number>, y: <number> }
point!(<x_number>, <y_number>)
point!(<coordinate>)
```

See also georust#742 (comment)

* Replace `Point(c)` with `point!(c)`
* Replace a few `format!("{}", v)` with `v.to_string()`
* Use `Self` or `Self::Output` instead of a concrete type when possible.
@nyurik nyurik force-pushed the dim branch 2 times, most recently from aef15ab to d92621d Compare March 19, 2022 05:44
@nyurik
Copy link
Member Author

nyurik commented Mar 19, 2022

Update: TZM support has been implemented on all geo types, and many simple implementations for those types. Some open questions:

  • How should coordinate math be handled? e.g. c1 + c2 does c1.x+c2.x, c1.y+c2.y. What should it be when z and/or m are given? Same for divide/multiply/negate/subtract

@nyurik nyurik force-pushed the dim branch 3 times, most recently from 30618a2 to c1e99d2 Compare March 19, 2022 21:42
@nyurik nyurik marked this pull request as ready for review March 20, 2022 06:36
@nyurik
Copy link
Member Author

nyurik commented Mar 20, 2022

This PR is ready for review. I updated the top comment with lots of details and examples. Reviewing should become much easier after related non-breaking PRs are merged (see list at the top)

@nyurik nyurik changed the title [WIP] Migrating to multi-dimensional coordinates Add support for 3D and Measure to geo types Mar 20, 2022
@nyurik nyurik changed the title Add support for 3D and Measure to geo types Implement 3D and Measure support for geo types Mar 20, 2022
@nyurik nyurik requested a review from urschrei March 20, 2022 18:12
@nyurik
Copy link
Member Author

nyurik commented Apr 6, 2022

See #795

I left the patch to WKT in because it seems preferable to adding the conversion code. We'll have to update WKT to work with the latest geo-types eventually anyway, right?

@michaelkirk sure, this works, thanks! With WKT -- yes, those changes will have to be made soon anyway, so yes, this will be simpler. Would everyone be ok to merge code that uses reference to the unpublished WKT? This goes a little against the principle that the main branch should always be release-ready (not mandatory, but does keep things cleaner).

I'm ok to just merge your changes into my branch, and will eventually squash it right before the release to keep the history cleaner (unless we can configure bors to auto-squash on merge)

@michaelkirk
Copy link
Member

michaelkirk commented Apr 6, 2022

@michaelkirk sure, this works, thanks! With WKT -- yes, those changes will have to be made soon anyway, so yes, this will be simpler. Would everyone be ok to merge code that uses reference to the unpublished WKT? This goes a little against the principle that the main branch should always be release-ready (not mandatory, but does keep things cleaner).

I think we shouldn't merge either until they are both approved.

In the meanwhile, do as I've done in #795, and update the patch section of your geo branch to reference your wkt branch, and update the patch section of your wkt branch to reference your geo branch.

@nyurik
Copy link
Member Author

nyurik commented Apr 6, 2022

@michaelkirk thanks, applied your changes, seem to pass. The WKT workaround is still bothering me.

I believe any code in the main branch should be "release-ready" - you can just publish whatever is in the main right now, and it should work for everyone because it passed CI. This code passes CI only because it patches WKT to a non-released version. And the WKT it uses is also not using released version. This is a hack because we fool CI to pass even though the code is not "production-stable" and cannot be merged/released (assuming we agree that there are no other issues).

## geo-types

* BREAKING: Remove deprecated functions on the `Geometry<T>`:
  * `into_point` - Switch to `std::convert::TryInto<Point>`
  * `into_line_string` - Switch to `std::convert::TryInto<LineString>`
  * `into_line` - Switch to `std::convert::TryInto<Line>`
  * `into_polygon` - Switch to `std::convert::TryInto<Polygon>`
  * `into_multi_point` - Switch to `std::convert::TryInto<MultiPoint>`
  * `into_multi_line_string` - Switch to `std::convert::TryInto<MultiLineString>`
  * `into_multi_polygon` - Switch to `std::convert::TryInto<MultiPolygon>`
* BREAKING: Remove deprecated `CoordinateType` trait. Use `CoordFloat` or `CoordNum` instead.
* BREAKING: Remove deprecated functions from `LineString<T>`
  * Remove `points_iter()` -- use `points()` instead.
  * Remove `num_coords()` -- use `geo::algorithm::coords_iter::CoordsIter::coords_count` instead.
* BREAKING: Remove deprecated functions from `Point<T>`
  * Remove `lng()` -- use `x()` instead.
  * Remove `set_lng()` -- use `set_x()` instead.
  * Remove `lat()` -- use `y()` instead.
  * Remove `set_lat()` -- use `set_y()` instead.
* BREAKING: Remove deprecated `Polygon<T>::is_convex()` -- use `geo::is_convex` on `poly.exterior()` instead.
* BREAKING: Remove deprecated `Rect<T>::try_new()` -- use `Rect::new` instead, since `Rect::try_new` will never Error. Also removes corresponding `InvalidRectCoordinatesError`.

## geo

* BREAKING: Remove deprecated `rotate()` from the `Rotate` trait:
> Equivalent to `rotate_around_centroid` except for `Polygon<T>`, where it is equivalent to rotating around the polygon's outer ring. Call that instead, or `rotate_around_center` if you'd like to rotate around the geometry's bounding box center.
* BREAKING: Remove `ToGeo` trait. Switch to `std::convert::Into<Geo>` or `std::convert::TryInto<Geo>`.
nyurik added a commit to nyurik/geo that referenced this pull request Apr 6, 2022
This PR focuses on `geo-types` only, making it possible to merge without conflicts because it no longer uses local relative paths. This is a part of georust#742. It also bumps geo-types requirements to 0.7.4.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

Many other pending PRs are included here - most of them can be merged without affecting existing users, and will make reviewing this PR easier. See the list below.

## geo-types restructuring
All geo type structs have been renamed from `Foo<T>(...)` to `FooTZM<T,Z,M>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineStringTZM<T: CoordNum, Z: ZCoord, M: Measure>(pub Vec<CoordTZM<T, Z, M>>);
pub type LineString<T> = LineStringTZM<T, NoValue, NoValue>;
pub type LineStringM<T, M> = LineStringTZM<T, NoValue, M>;
pub type LineStringZ<T> = LineStringTZM<T, T, NoValue>;
pub type LineStringZM<T, M> = LineStringTZM<T, T, M>;
```

## NoValue magic
`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for CoordTZM<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

## Variant algorithm implementations
Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure (Z is set to `NoValue` by `LineM` type alias.

```rust
impl<T: CoordNum, M: Measure> LineM<T, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

## Breaking changes
### Tuple constructors
It will not be possible to use implicit type constructor for tuples because type aliases do not support them. Most geo types will have a `new(...)` fn if they didn't have one before.
```rust
// old
MultiPoint(vec![...]);
...iter().map(MultiPoint);
// new
MultiPoint::new(vec![...]);
...iter().map(MultiPoint::new);
```
### Type destructors
Destructuring assignments seem to require real type rather than an alias.
```rust
// old
let Point(c) = p;
// new
let PointTZM(c) = p;
```
nyurik added a commit to nyurik/geo that referenced this pull request Apr 6, 2022
This PR focuses on `geo-types` only, making it possible to merge without conflicts because it no longer uses local relative paths. This is a part of georust#742. It also bumps geo-types requirements to 0.7.4.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

Many other pending PRs are included here - most of them can be merged without affecting existing users, and will make reviewing this PR easier. See the list below.

All geo type structs have been renamed from `Foo<T>(...)` to `FooTZM<T,Z,M>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineStringTZM<T: CoordNum, Z: ZCoord, M: Measure>(pub Vec<CoordTZM<T, Z, M>>);
pub type LineString<T> = LineStringTZM<T, NoValue, NoValue>;
pub type LineStringM<T, M> = LineStringTZM<T, NoValue, M>;
pub type LineStringZ<T> = LineStringTZM<T, T, NoValue>;
pub type LineStringZM<T, M> = LineStringTZM<T, T, M>;
```

`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for CoordTZM<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure (Z is set to `NoValue` by `LineM` type alias.

```rust
impl<T: CoordNum, M: Measure> LineM<T, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

It will not be possible to use implicit type constructor for tuples because type aliases do not support them. Most geo types will have a `new(...)` fn if they didn't have one before.
```rust
// old
MultiPoint(vec![...]);
...iter().map(MultiPoint);
// new
MultiPoint::new(vec![...]);
...iter().map(MultiPoint::new);
```
Destructuring assignments seem to require real type rather than an alias.
```rust
// old
let Point(c) = p;
// new
let PointTZM(c) = p;
```
@nyurik
Copy link
Member Author

nyurik commented Apr 6, 2022

After many attempts to make a clean switch, it seems the only real way to do this is as was suggested by @frewsxcv -- update just the geo-types crate and cut all local relative paths to it from other crates. This way we can review and merge it separately without impacting any other crates, and once merged and released, we can upgrade all other project one at a time. Please review #797 and once it is approved, this PR will only change non - geo-types crates.

@nyurik nyurik marked this pull request as draft April 6, 2022 17:59
nyurik added a commit to nyurik/geo that referenced this pull request Apr 6, 2022
This PR focuses on `geo-types` only, making it possible to merge without conflicts because it no longer uses local relative paths. This is a part of georust#742. It also bumps geo-types requirements to 0.7.4.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

Many other pending PRs are included here - most of them can be merged without affecting existing users, and will make reviewing this PR easier. See the list below.

All geo type structs have been renamed from `Foo<T>(...)` to `FooTZM<T,Z,M>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineStringTZM<T: CoordNum, Z: ZCoord, M: Measure>(pub Vec<CoordTZM<T, Z, M>>);
pub type LineString<T> = LineStringTZM<T, NoValue, NoValue>;
pub type LineStringM<T, M> = LineStringTZM<T, NoValue, M>;
pub type LineStringZ<T> = LineStringTZM<T, T, NoValue>;
pub type LineStringZM<T, M> = LineStringTZM<T, T, M>;
```

`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for CoordTZM<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure (Z is set to `NoValue` by `LineM` type alias.

```rust
impl<T: CoordNum, M: Measure> LineM<T, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

It will not be possible to use implicit type constructor for tuples because type aliases do not support them. Most geo types will have a `new(...)` fn if they didn't have one before.
```rust
// old
MultiPoint(vec![...]);
...iter().map(MultiPoint);
// new
MultiPoint::new(vec![...]);
...iter().map(MultiPoint::new);
```
Destructuring assignments seem to require real type rather than an alias.
```rust
// old
let Point(c) = p;
// new
let PointTZM(c) = p;
```
nyurik added a commit to nyurik/geo that referenced this pull request Apr 21, 2022
This PR includes georust#772 and georust#812

---

This PR focuses on `geo-types` only. This is a part of georust#742.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

## geo-types restructuring
All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>);

pub type LineStringM<T, M=T> = LineString<T, NoValue, M>;
pub type LineString3D<T> = LineString<T, T, NoValue>;
pub type LineString3DM<T, M=T> = LineString<T, T, M>;
```

## NoValue magic
`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

## Variant algorithm implementations
Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure.

```rust
impl<T: CoordNum, M: Measure> Line<T, NoValue, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

## Breaking changes
* It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
nyurik added a commit to nyurik/geo that referenced this pull request Apr 21, 2022
This PR includes georust#772 and georust#812

---

This PR focuses on `geo-types` only. This is a part of georust#742.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

## geo-types restructuring
All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>);

pub type LineStringM<T, M=T> = LineString<T, NoValue, M>;
pub type LineString3D<T> = LineString<T, T, NoValue>;
pub type LineString3DM<T, M=T> = LineString<T, T, M>;
```

## NoValue magic
`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

## Variant algorithm implementations
Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure.

```rust
impl<T: CoordNum, M: Measure> Line<T, NoValue, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

## Breaking changes
* It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
urschrei pushed a commit that referenced this pull request Aug 29, 2022
This PR includes #772

---

This PR focuses on `geo-types` only. This is a part of #742.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>);

pub type LineStringM<T, M=T> = LineString<T, NoValue, M>;
pub type LineString3D<T> = LineString<T, T, NoValue>;
pub type LineString3DM<T, M=T> = LineString<T, T, M>;
```

`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure.

```rust
impl<T: CoordNum, M: Measure> Line<T, NoValue, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

* It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
urschrei pushed a commit that referenced this pull request Aug 29, 2022
This PR includes #772

---

This PR focuses on `geo-types` only. This is a part of #742.

This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change.

All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added:

```rust
// old
pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>);

// new
pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>);

pub type LineStringM<T, M=T> = LineString<T, NoValue, M>;
pub type LineString3D<T> = LineString<T, T, NoValue>;
pub type LineString3DM<T, M=T> = LineString<T, T, M>;
```

`NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there.  This code works for Z and M being either a number or a NoValue:

```rust
pub struct NoValue;

impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> {
    type Output = Self;

    fn sub(self, rhs: Self) -> Self {
        coord! {
            x: self.x - rhs.x,
            y: self.y - rhs.y,
            z: self.z - rhs.z,
            m: self.m - rhs.m,
        }
    }
}
```

Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure.

```rust
impl<T: CoordNum, M: Measure> Line<T, NoValue, M> {
    /// Calculate the slope (Δy/Δx).
    pub fn slope(&self) -> T {
        self.dy() / self.dx()
    }
}
```

* It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
@frewsxcv
Copy link
Member

Closing for inactivity. Feel free to reopen if you resume work on this.

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

Successfully merging this pull request may close these issues.

5 participants