feat(rust): [PoC] Support 3D geometry (GeometryZ)#1507
Conversation
Replace the 2D-only geo-types concrete types with the geo-traits abstraction, laying the groundwork for experimental 3D (Z) support without changing the wire format or any behavior. - Encoder input now accepts any `&impl geo_traits::GeometryTrait<T = i32>` (so geo_types, wkt, or any geo-traits geometry works as input). - Decoder output is now `wkt::Wkt<i32>` (dimension-aware, Z-capable), replacing `geo_types::Geometry<i32>` in TileFeature/FeatureRef/ to_geojson/geojson::Feature. Currently always 2D (z = None). - Add `convert::geom::to_wkt` to normalize any geo-traits geometry into `wkt::Wkt`, preserving coordinate dimensionality; use `geo_traits::to_geo::ToGeoGeometry` for the reverse at the 2D MVT/WKB boundary. - Migrate all decoder-output consumers (geojson serde, MVT, mlt-py WKB, mlt TUI rendering) to read via wkt / geo-traits accessors. Depends on `wkt` 0.14 (whose types implement geo-traits 0.3), keeping the workspace `unsafe_code = "forbid"` intact (the trait impls' unsafe lives inside the wkt crate; mlt-core writes none). No wire-format change; encoding is byte-identical and the full test suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Encode and decode 3D geometries end-to-end: an interleaved `x, y, z`
vertex buffer marked by a new `GeometryZ` column type.
- Add `ColumnType::GeometryZ` (the in-tile analog of the spec's
`ComplexType::GEOMETRY_Z`) and a `CoordDim { Xy, Xyz }` enum carried by
`RawGeometry` and `GeometryValues`. The column type, not the stream data,
is the single source of truth for dimensionality; it is threaded
ColumnType -> RawGeometry -> GeometryValues -> vertex stride.
- Generalize the componentwise-delta vertex codec to an arbitrary stride
(`encode/decode_componentwise_delta(.., n_dims, ..)`); the plain Vec3
layout is used for 3D. Morton/Hilbert dictionaries remain 2D-only and
are skipped for 3D columns.
- Decoder builds `wkt::Wkt` with `Dimension::XYZ` and a populated Z;
encoder reads Z from the input geometry's coordinates (column
dimensionality is taken from the first geometry pushed).
- GeoJSON serde emits/parses `[x, y, z]` when present; MVT stays 2D.
The `CoordDim` enum keeps dimensionality type-safe at the API boundaries
while the low-level codec/vertex helpers take a plain `usize` stride.
2D tiles are unaffected and byte-identical; added 3D round-trip tests
(point/linestring/multipoint/polygon through the full `GeometryZ` wire
path, plus geojson `[x,y,z]` and a stride-3 codec test). Full suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mlt-py's WKB encoder previously routed decoded geometry through geo_types (2D), silently dropping Z. Rewrite `geom32_to_wkb` to read the dimension-aware `wkt::Wkt` directly and emit ISO WKB with Z preserved (type code `base + 1000`, an extra f64 per coordinate) for 3D geometries; 2D output is unchanged. The tile transform applies to x/y only — z (elevation) is written as-is. MVT stays 2D (it has no Z). Adds a 3D WKB round-trip test. Note: the shared tileset-metadata schema (spec/schema/*.proto) and the written spec are intentionally left untouched — 3D remains a Rust-only experiment for now, so the Java metadata path is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…le-spec into feat/rust-3d
There was a problem hiding this comment.
Pull request overview
This PR is a Rust proof-of-concept implementation of 3D geometry support by introducing a GeometryZ column type and switching decoded geometry output to a dimension-aware representation (wkt::Wkt<i32>), enabling optional Z coordinates to survive encode/decode.
Changes:
- Adds
ColumnType::GeometryZplus dimensionality tracking (CoordDim) and updates geometry encode/decode paths to support interleaved(x, y, z)vertices. - Migrates decoded geometry representation across the Rust UI, core, and Python bindings from
geo_types::Geometry<i32>towkt::Wkt<i32>, leveraginggeo-traits. - Updates conversions (GeoJSON/MVT) and tests to cover 3D roundtrips and Z-preserving outputs.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/mlt/src/ui/state.rs | UI selection/vertex extraction updated to wkt::Wkt APIs. |
| rust/mlt/src/ui/rendering/map.rs | Renderer updated to draw wkt::Wkt geometries. |
| rust/mlt/src/ui/rendering/layers.rs | Geometry stats panel updated for wkt::Wkt types/iterators. |
| rust/mlt/src/ui/mod.rs | Geometry helper utilities updated for wkt polygon ring APIs. |
| rust/mlt/src/ui/mbt.rs | World-vertex collection updated for wkt::Wkt. |
| rust/mlt-py/src/lib.rs | Python WKB output updated to emit Z-aware WKB for 3D. |
| rust/mlt-py/src/encode/geojson.rs | GeoJSON encoder updated for wkt::Wkt and builder API changes. |
| rust/mlt-core/src/lib.rs | Re-exports geo_traits/wkt and exports CoordDim. |
| rust/mlt-core/src/encoder/sort.rs | Sorting generalized to geo_traits::GeometryTrait traversal. |
| rust/mlt-core/src/encoder/property/tests.rs | Tests updated to feed wkt geometry into tile features. |
| rust/mlt-core/src/encoder/geometry/tests.rs | Adds end-to-end 3D encode→decode tests and updates parsing calls. |
| rust/mlt-core/src/encoder/geometry/geotype.rs | Geometry ingestion generalized to geo_traits and extended to 3D vertices. |
| rust/mlt-core/src/encoder/geometry/encode.rs | Vertex stream encoding generalized to N-dim stride; writes GeometryZ column type. |
| rust/mlt-core/src/decoder/stream/logical.rs | Adds stride-aware decode helper for componentwise-delta vertex streams. |
| rust/mlt-core/src/decoder/stream/decode.rs | Adds stride-aware stream decode wrapper for vertex buffers. |
| rust/mlt-core/src/decoder/root.rs | Parses GeometryZ and carries dimension into raw geometry. |
| rust/mlt-core/src/decoder/model.rs | Adds GeometryZ column type; switches tile geometry storage to wkt::Wkt. |
| rust/mlt-core/src/decoder/mod.rs | Re-exports CoordDim and updates internal exports. |
| rust/mlt-core/src/decoder/iterators.rs | Feature iterator now yields wkt::Wkt geometry. |
| rust/mlt-core/src/decoder/geometry/model.rs | Introduces CoordDim; stores dim on raw/decoded geometry values. |
| rust/mlt-core/src/decoder/geometry/geotype.rs | Decodes vertex buffers into dimensioned wkt::Wkt output. |
| rust/mlt-core/src/decoder/geometry/decode.rs | Uses CoordDim to decode vertex buffer with correct stride. |
| rust/mlt-core/src/decoder/column.rs | Updates column-name rules to include GeometryZ. |
| rust/mlt-core/src/convert/mvt/encode.rs | Converts wkt::Wkt to 2D geo_types geometry for MVT boundary. |
| rust/mlt-core/src/convert/mvt/decode.rs | Normalizes MVT decoded geometries into wkt::Wkt. |
| rust/mlt-core/src/convert/mod.rs | Adds internal convert::geom module. |
| rust/mlt-core/src/convert/geom.rs | New normalization utility from geo_traits geometries to wkt::Wkt. |
| rust/mlt-core/src/convert/geojson.rs | GeoJSON serde updated to support emitting/reading [x,y,z] coords. |
| rust/mlt-core/src/codecs/zigzag.rs | Generalizes componentwise-delta codec from vec2 to N-dim stride. |
| rust/mlt-core/Cargo.toml | Adds geo-traits and wkt dependencies. |
| rust/Cargo.toml | Adds workspace dependencies for geo-traits and wkt. |
| rust/Cargo.lock | Locks new geo-traits/wkt dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| target.clear(); | ||
| target.reserve(data.len()); | ||
| let mut prev_x = T::zero(); | ||
| let mut prev_y = T::zero(); | ||
| for chunk in data.chunks_exact(2) { | ||
| let (x, y) = (chunk[0], chunk[1]); | ||
| target.push(T::encode(x.wrapping_sub(&prev_x))); | ||
| target.push(T::encode(y.wrapping_sub(&prev_y))); | ||
| (prev_x, prev_y) = (x, y); | ||
| debug_assert!(n_dims <= MAX_COMPONENTS); | ||
| let mut prev = [T::zero(); MAX_COMPONENTS]; | ||
| for chunk in data.chunks_exact(n_dims) { | ||
| for (j, &v) in chunk.iter().enumerate() { | ||
| target.push(T::encode(v.wrapping_sub(&prev[j]))); | ||
| prev[j] = v; | ||
| } | ||
| } | ||
| target |
| // Strip the closing vertex if the ring repeats its first coordinate at the end. | ||
| let closed = n > 1 | ||
| && ring | ||
| .coord(0) | ||
| .zip(ring.coord(n - 1)) | ||
| .is_some_and(|(f, l)| f.x() == l.x() && f.y() == l.y()); | ||
| let len = if closed { n - 1 } else { n }; |
| if self.vector_types.is_empty() { | ||
| self.dim = if matches!(geom.dim(), Dimensions::Xyz) { | ||
| CoordDim::Xyz | ||
| } else { | ||
| CoordDim::Xy | ||
| }; | ||
| } |
| Wkt::Point(p) => ( | ||
| "Point", | ||
| serde_json::to_value(point_arr(p).unwrap_or_default()).unwrap(), | ||
| ), |
| fn arr_coord(a: &Arr) -> Coord<i32> { | ||
| Coord { | ||
| x: a[0], | ||
| y: a[1], | ||
| z: a.get(2).copied(), |
| pub(crate) id: Option<u64>, | ||
| /// Geometry as a [`geo_types`] form | ||
| pub(crate) geometry: geo_types::Geometry<i32>, | ||
| /// Geometry as a dimension-aware [`wkt::Wkt`] (currently always 2D). | ||
| pub(crate) geometry: wkt::Wkt<i32>, | ||
| /// One value per property column, in the same order as |
| /// Decoded geometry as a dimension-aware [`wkt::Wkt`] (owned, decoded on demand by the | ||
| /// iterator). Currently always 2D. | ||
| geometry: wkt::Wkt<i32>, |
| fn geom32_to_wkb(geom: &Wkt<i32>, xf: Option<TileTransform>) -> MltResult<Vec<u8>> { | ||
| let has_z = matches!(geom.dimension(), Dimension::XYZ); | ||
| let mut buf = Vec::with_capacity(128); | ||
| match geom { |
| //! MLT accepts any geometry that implements [`geo_traits::GeometryTrait`] as encoder input | ||
| //! (so both [`geo_types`] and [`wkt`] values work), and produces [`wkt::Wkt`] as decoder output | ||
| //! (the dimension-aware type that can carry an optional Z coordinate). | ||
| //! | ||
| //! [`to_wkt`] normalizes any geo-traits geometry into a [`wkt::Wkt`], preserving coordinate | ||
| //! dimensionality. [`geo_traits::to_geo::ToGeoGeometry::to_geometry`] handles the reverse | ||
| //! (`wkt::Wkt` -> `geo_types::Geometry`, dropping any Z) for the 2D MVT boundary. |
Performance ComparisonPerformance Comparison
|
| zoom | tiles | prev_total | curr_total | delta_total | avg_delta | pct | best | worst |
|---|---|---|---|---|---|---|---|---|
| 0 | 1 | 71471 | 71471 | 0 | 0.0 | 0.0 | ||
| 1 | 4 | 220986 | 220986 | 0 | 0.0 | 0.0 | ||
| 2 | 16 | 615003 | 615003 | 0 | 0.0 | 0.0 | ||
| 3 | 64 | 1653851 | 1653851 | 0 | 0.0 | 0.0 | ||
| 4 | 256 | 3828904 | 3828904 | 0 | 0.0 | 0.0 | ||
| 5 | 1024 | 9716821 | 9716821 | 0 | 0.0 | 0.0 | ||
| 6 | 4096 | 32401914 | 32401914 | 0 | 0.0 | 0.0 | ||
| ALL | 5461 | 48508950 | 48508950 | 0 | 0.0 | 0.0 |
Top 3 improvements
Top 3 degradations
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1507 +/- ##
==========================================
+ Coverage 62.82% 65.01% +2.18%
==========================================
Files 94 96 +2
Lines 15317 16015 +698
==========================================
+ Hits 9623 10412 +789
+ Misses 5694 5603 -91 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Since adding the new type would likely be a bit breaking for existing consumers of mlt@v1, we need to remove this part and re-add it in v2 😉 Would this be a good alternative for you? |
|
Thanks. I don't have strong opinion here, but let me clarify a bit for discussion.
In my understanding, adding the ability to decode/encode If you mean that the TypeScript decoder fails when the data contains That said,
this sounds good to me. One of the things I feel uneasy with is the gap between the specification and the implementation, so it sounds a reasonable idea to tweak the specification to match with the implementation. If v2 will come in the foreseeable future, I'm happy with your idea! |
|
I have a hunch that I might get the time to work towards V2 after my exams this way or that. ^^ |
|
Good luck on your exam! |
|
@yutannihilation I didn't go too far into the implementation yet, but I do want to mention a few direction thougths.
|
|
Thanks for sharing the directions! But, let me make it clear about this point.
I don't change anything. The current MLT specification supports 3D coordinates, and I just naively implemented it. I understand "a feature parity with MVT" is your current consensus on v1, but it's not what the specification says. |
|
@yutannihilation I am not saying you did anything wrong :) The specification was written long before we started v1 implementation, and it was initially trying many things at once. Later, we tried to clean it up to limit just the MVT-logical-compat for v1 - but we might have missed a thing or two - that should be clearly marked as the future development rather than part of the v1 specification. I will update it to clearly state that the current v1 implementation is only 2D, whereas we can easily add the 3D support later, using either the method mentioned in the spec, or we could alter it significantly - none of the z support is set in stone yet. |
|
See #1509 |
|
Ah, makes sense. Thanks for the context! |
|
I forgot to explain one caveat in this implementation; in order to use only what the specification document says, this implementation stores Z coordinate as-is. But, it's not obvious what this value should be translated. X and Y are in-tile coordinates. This means these have implicit scale and offset, which are uniquely determined by the combination of X, Y, and zoom level. But, it's not the case with Z coordinate. So, I wish v2 define some metadata like |
The current specification supports 3D geometry:
However, as far as I can tell, none of the current implementations support 3D geometry in actual. I know there's an ongoing discussion about 3D support, but I believe the MLT v1 spec is ready for supporting simple
(x, y, z)coordinate.This pull request is the proof of concept; while this is naively done by AI, I believe the implementation would be like this anyway if I implement all by hand. The changes are a bit lengthy, but the points are simple:
structfor 3D geometry, use the wkt crate for this.GeometryZto the column type, and treatGeometryandGeometryZdifferently.I also have the corresponding TypeScript implementation, and it works fine. So, I don't see any blocking issue on supporting 3D coordinates in this way.
Are there any particular reasons why the current implementations don't support the optional z coordinate? I don't expect this pull request to be merged as-is, but I'm hoping it can serve as a starting point for discussion — I'd love to know whether this is heading in the right direction!