NetTopologySuite / NetTopologySuite.IO.VectorTiles

A package that can be used to generate vector tiles using NTS.
Other
21 stars 13 forks source link

Fix misplacement of geometry parts #30

Closed FObermaier closed 3 months ago

FObermaier commented 8 months ago

If components of geometries are too small to be encoded at the provided zoom level the encoded data is deleted in Encode(CoordinateSequence seq, ..., ref int currentX, ref int currentY, ...) but the values of currentX and currentY are not reset to their initial values. This causes all subsequent geometry items to have a wrong offset.

This PR fixes this by

closes #29

FObermaier commented 8 months ago

@rbrundritt, @xivk, @kolyanch I'd like your opinion on whether or not we should check if the extent of an encoded LineString exceeds 1 pixel and only write it to the tile if it does:

https://github.com/NetTopologySuite/NetTopologySuite.IO.VectorTiles/blob/556e70c79d67de34e788d0cc3f4c6c66cbbfad2e/src/NetTopologySuite.IO.VectorTiles.Mapbox/MapboxTileWriter.cs#L236-L249

kolyanch commented 8 months ago

I believe that the processing of small objects should be the same for polygonal and linear objects. I opened the MVT specification, there is nothing about linear objects. But there is a strict rule for polygonal ones:

A linear ring SHOULD NOT have an area calculated by the surveyor's formula equal to zero, as this would signify a ring with anomalous geometric points

In short I vote for checks for linear objects)

rbrundritt commented 8 months ago

I did think about this, as well as the 1-pixel area polygon scenario (or user defined minimums). The main issue I've ran into in the past with this is you end up in scenarios where the end user is zoomed out and you have an empty map and can't see where the data is. I've found that it is often better to show 1-pixel for lines/polygons in this scenario, at least from a real-world user experience perspective. That said, it would depend on your data set. If you are showing a single data set, it's nice to have these show, but if you have multiple datasets where shapes are different sizes, then it may be desirable to not have the smaller shapes appear.

With all this in mind, having a configuration setting on a per layer level, where the minimum area/length of a polygon/line could be set, would be a "nice to have" feature. But this definitely shouldn't be forced.

xivk commented 8 months ago

I also believe by default we should drop anything that is too small. I even thought this was the default behavior, I have some checking to do on some of our services now...

FObermaier commented 3 months ago

I went with @rbrundritt suggestion to have it configurable. MapBoxTileWriter has Write overloads that take minLinealExtent (default: 1) and minPolygonalExtent (default: 2, resembles previous greater than 1 constraint). The implementation is a bit akward as I did not want to break binary compatibility.

Please reeavaluate @rbrundritt , @xivk and @kolyanch.