Skip to content

Bugfix st as mvt#334

Merged
nyurik merged 4 commits intoopenmaptiles:masterfrom
maptiler:bugfix_ST_AsMVT
Nov 2, 2020
Merged

Bugfix st as mvt#334
nyurik merged 4 commits intoopenmaptiles:masterfrom
maptiler:bugfix_ST_AsMVT

Conversation

@lazaa32
Copy link
Copy Markdown
Collaborator

@lazaa32 lazaa32 commented Nov 2, 2020

This PR fixes two issues:

  1. Before ST_AsMVTGeom(geometry, ST_TileEnvelope(zoom, x, y), extent, buffer, true) function read buffer from layer yaml definition buffer_size (e.g. 256 for layer place). Which is in pixels but should be in vector tile unit because it is compared to extent (4096). Related to PR Bug fix tile_to_bbox(): use pixel_width instead of extent. #333.

  2. When replacing !pixel_width! and !pixel_height! in Mapnik query, self.pixel_width and self.pixel_height are used. That means that a constant value pixel_scale is used throughout all zooms. However I checked logs from Mapnik rendering and following values for pixel_width are used:

    Zoom pixel_width
    1 78271.5
    2 39135.8
    3 19567.9
    4 9783.94
    5 4891.97
    6 2445.98
    7 1222.99
    8 611.496
    9 305.748
    10 152.874
    11 76.437
    12 38.2185
    13 19.1093
    14 9.55463

    I modified the tool so it calculates pixel_width (which is actually zoom resolution meters/pixel) for each zoom_level. Using constant value for pixel_width leads to having different number of features in a same tile rendered via PgQuery and Mapnik.

Copy link
Copy Markdown
Member

@nyurik nyurik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome work, thanks!!! One question -- shouldn't the !pixel_width! change be reflected in the tests somewhere? I'm a bit surprised there is no change to it. Maybe we should add some explicit tests for it?

@lazaa32
Copy link
Copy Markdown
Collaborator Author

lazaa32 commented Nov 2, 2020

Hi @nyurik
you are probably right. It looks it is not covered now because the !pixel_width! change effects only POINT layers.
https://github.com/openmaptiles/openmaptiles-tools/blob/master/tests/sql/test-sql.sh#L149
I've found only this code where we test a tile without any POINT feature. I think that adding another test for a tile with some features should solve the problem.
But I've never dived into depth of tests so far so if it is more complicated let me know.

@nyurik nyurik merged commit 84e76e7 into openmaptiles:master Nov 2, 2020
@nyurik
Copy link
Copy Markdown
Member

nyurik commented Nov 2, 2020

@lazaa32 if you have a few extra minutes, please create a simple test similar to the ones that already exist in the https://github.com/openmaptiles/openmaptiles-tools/tree/master/tests/testlayers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants