search

NetTopologySuite / NetTopologySuite.IO.VectorTiles

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

Feature: Let the writer support EPSG:3857 Web Mercator coordinates #13

Closed rbrundritt closed 2 years ago

rbrundritt commented 2 years ago

Currently the writer expects the input data to be in EPSG:4326 WGS84 coordinates, and then the writer converts these to EPSG:3857 Web Mercator coordinates. If we add support for EPSG:3857 coordinates, the conversion step could be skipped in that case which would be useful in high performance scenarios (i.e. on the fly tile generation within a server). You could have the data in the right projection ahead of time and not have to convert it at runtime.

If we want to add support for this, we could add a simple boolean flag to this line of code: https://github.com/NetTopologySuite/NetTopologySuite.IO.VectorTiles/blob/a8cc35105d424d8957957c9b89f6336f929f9ce1/src/NetTopologySuite.IO.VectorTiles.Mapbox/MapboxTileWriter.cs#L63

Alternatively, we could get fancy and add support for any projection and pull in Proj4Net or some other library, however, that would increase the overall size and complexity of this library which may not be desired. If we simply support 3857, and a dev wanted to support other coordinate reference systems, they could add code to convert to 3857 using a library or customer code themselves, while avoiding the data being converted a second time by this library.

It's also good to point out that even with 3857 there is a conversion step from meters to pixels that would need to be done. The math for that however is much less intensive that converting from 4326 to 3857 as its a simple scale and offset calculation.

This would however bring up the question of should the reader also support 3857 as an option?

rbrundritt commented 2 years ago

I went through and got this working locally and found it required adding a lot of code/complexity. In the end it managed to see a 14% increase in performance when using 3857 data, however there was a small decrease in performance for 4326 data (likely possible to make this negligible with some code clean up).

That said, I'm not overly impressed with this improvement. In a high-performance scenario, if I really wanted to make a difference, converting the source data ahead of time to pixels at a high zoom level (like 22 for a tile size of 4096 to match Mapbox's tile extent) and then simply scaling the values would be the way to go. I experimented with this and had a rough solution working that was 300% faster, however, this again added a lot of complexity and impacted 4326 perf. At the end of the day, if someone is going to go to that extent for performance, they likely would be willing to use C or C++ for added performance. I know several of the major mapping platforms maintain pixel data rather than latlon data for high performance tile generation. With this in mind, I don't think it makes sense to add this functionality to this library at this time, so closing this issue.

I did however make a lot of other improvements that I'll put into a pull request.

donnyv commented 3 weeks ago

I still think this is valid. Removing a conversation step would be a great perf jump. Plus it should standardize on EPSG:3857 if thats what its using internally. I am doing what you mentioned here. All data imported is being standardized into EPSG:3857. I'm doing on the fly tile generation, so that I can change what additional field data gets added. I'm currently building a vector tile service using C#. .Net's performance has been getting better with every release. I don't think there's a need to go to C or C++.