Closed springmeyer closed 9 years ago
We need to intelligently skip and coordinates that fail to reproject (!prjtrans.backward())
Writing unit tests for this in mapnik-vt I hit this unhandled condition: https://github.com/mapnik/mapnik/pull/2901.
It turns out that inf
at https://github.com/mapbox/mapnik-vector-tile/blob/0c1dd1699e954f5024525cbd085ba7778d656962/src/vector_tile_strategy.hpp#L67-L68 will flip to -9223372036854775807
and this will trigger ClipperLib::AddPath
to throw in RangeTest
. So, finally we have a valid and likely way to reproduce #111
9dd2729 adds the new transformer. Due to the ability to call std::reserve
perf is noticeably better:
$ ./build/Release/vtile-transform
2192.54ms (cpu 2179.91ms) | boost::geometry::transform
1511.22ms (cpu 1500.99ms) | transform_visitor with reserve
After f6fb9d4 we are now again handling pj_transform failures without throwing.
The goal is not to perfectly represent shapes with coordinates outside the valid bounds - to do that robustly would require clipping before reprojecting. While we did that before it would now be inconvenient. So the key here is to perfectly represent shapes with 100% valid coords and therefore to:
1) not risk aborting rendering due to invalid ones. 2) do a decent job at trying to render shapes with some invalid coords
As an example of what rendering a shape with invalid coords looks like see the following image (which is the testcase geometry visualized in studio a few ways):
pj_transform
and skips failures resulting in a partially collapsed shape. Not pretty but expected.pj_transform
failures because proj4 is not invoked and invalid bounds are clamped by https://github.com/mapnik/mapnik/blob/352586e9d7f9276fb27c10d5449863fe67450612/include/mapnik/well_known_srs.hpp#L67-L70. So, the shape is not collapsed which is nice. Since this result is nicer we could consider clamping for more projections in the future, but that is pretty low priority in my opinion.
Our vector tile processor here uses
boost::geometry::transform
and this strategy. The underlying strategy is sound howeverboost::geometry::transform
is not the right tool for the job because:!prj_trans_.backward()
)vector.reserve(size)
Therefore we need: