Incorrect resolution set when using PostGIS input with simplify_geometries

[YenTheFirst]

When rendering a vector tile from a PostGIS input that has 'simplify_geometries=yes' set, it is querying Postgres with an inappropriately large simplification parameter.

Steps to reproduce:

0) create a basic geometry table, with geometries in 4326. 1) create a basic mapnik XML with a single postgis datasource, and simplify_geometries='yes', and output projection web mercator. 2) log executed postgres queries 3) run

mapnik = require('mapnik')
mapnik.register_datasource(mapnik.settings.paths.input_plugins+"/postgis.input")

map = new mapnik.Map(256,256)
vtile = new mapnik.VectorTile(11,619,757, {buffer_size: 0})
im = new mapnik.Image(256,256)

map.fromString(fs.readFileSync('basic.xml','utf8'), {}, function(err){console.log(err)});
map.extent = vtile.extent()

map.render(im, function(err,vt) {console.log(err)});
map.render(vtile, function(err,vt) {console.log(err)});

Observe that the call to raster renderer will request geometry as ST_AsBinary(ST_Simplify(ST_SnapToGrid("geom",1.26848e-05), 2.53696e-05))

The call to the vector render will request geometry as ST_AsBinary(ST_Simplify(ST_SnapToGrid("geom",1.91093), 3.82185))

---

I believe this is because https://github.com/mapbox/mapnik-vector-tile/blob/master/src/vector_tile_layer.hpp#L266 is calculating the resolution (used by postgis.input to determine simplification resolution) based on the tile extent in the output projection, not the table's projection. Due to difficulty getting a build working, I haven't yet confirmed this.

[YenTheFirst]

I've confirmed that changing those lines resolves the issue (https://github.com/opencounter/mapnik-vector-tile/commit/51fa1a890057b203982b56ecfdd765630840a4cc)

Once I get some tests together, I'll open a PR.

[flippmoke]

I am thinking this is correct and I screwed this up in the past at some point, good catch. It appears we used to use the query extent as well previously.

[flippmoke]

The resolution here should be the same as the feature style processor in mapnik.

Therefore, I think believe your change is correct. If you want to put up a PR I will accept it.

[springmeyer]

This change needs a test.

[YenTheFirst]

I've been able to confirm it works correctly when compiled into node-mapnik. I've tried and failed to get this repo building on its own, so I'm currently unable to run tests.

I attempted to use the mason-defined dependencies by running ./bootstrap.sh, and setting those directories as include directories, but it always fails with some compilation error or other. Would you or someone be available to help me get the build process working, and separately get that encoded in a script?

[flippmoke]

@springmeyer We do not have any postgis tests by default in our test setup currently and would rather not if we could avoid it. I believe you said the resolution parameter was also used in the GDAL driver correct?

[springmeyer]

@flippmoke Want to add some tests around tile_layer and calc_query?

[flippmoke]

@springmeyer that might definitely be the best way to do it, but the problem for me is going to be what is a correct resolution value anyhow. I am not really sure on the parameter's intent - @artemp might be able to help with that though.

[springmeyer]

This ended merging without tests in https://github.com/mapbox/mapnik-vector-tile/pull/233. It regressed labeling in testing, causing many labels to disappear. The regression was fixed in https://github.com/mapbox/mapnik-vector-tile/pull/245.