Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fix tile_to_bbox(): use pixel_width instead of extent. #333

Merged
merged 2 commits into from
Oct 30, 2020

Conversation

lazaa32
Copy link
Collaborator

@lazaa32 lazaa32 commented Oct 29, 2020

When PgQuery makes a query into postgres it uses smaller tile buffer than Mapnik.
In example below layer place has a buffer_size set to 256.

Example:
Queried tile: 7/69/43
Queried layer: layer_place
Green: bbox of tile
Purple: bbox of PgQuery query of layer_place before bug fix
Blue dash: bbox of PgQuery query of layer_place after bug fix
Red dot dash: bbox of Mapnik query of layer_place
Blue:
image

@nyurik
Copy link
Member

nyurik commented Oct 29, 2020

I could be wrong, but I suspect this was a bug in Mapnik in that case. My understanding is that extent is how many units are in a single tile, i.e. a tile is 4096 x 4096 units. The buffer is the number of those units, so if a tile width is a whole world (along the equator), the formula would be equatorial_length / 4096 * buffer_size. For higher zooms, we would divide it by 2 ^ zoom.

I do not understand what dividing by pixel size would actually mean in this context.

@lazaa32
Copy link
Collaborator Author

lazaa32 commented Oct 30, 2020

Hi @nyurik
as I understood it, pixel_width is actually taken from tileset.pixel_scale in https://github.com/openmaptiles/openmaptiles-tools/blob/master/openmaptiles/sqltomvt.py#L31 Then you get a ratio of buffer_size/pixel_scale which in the case of layer_place where the buffer_size=256 ends up in 256/256 and thats why the buffer of tile is equal to tile size in Mapnik.
I can't say whether it is bug in Mapnik or not however I think that for layer_place you want to have a big buffer as Mapnik has due to labels. And if the calculation of buffer size is correct now in tile_to_bbox() function the only way how to fix it is to modify buffer size in all layer/tileset definitions.

@nyurik
Copy link
Member

nyurik commented Oct 30, 2020

@lazaa32 what are the units of measurements for the pixel_scale and buffer_size? I think the legacy tile is 256x256 pixels (raster), whereas the vector tile is (i guess) 4096x4096 (extent). If the measurement for buffer_size was in pixels, than yes - we need to convert 256 to be the width of a tile. I can't recall if we have 256 hardcoded anywhere.

@lazaa32
Copy link
Collaborator Author

lazaa32 commented Oct 30, 2020

@nyurik I think that pixel_scale and buffer_size are really in pixels by legacy.
On the other hand, right now we don't have any parameter that defines extent of vector tile. It is only hardcoded here: https://github.com/openmaptiles/openmaptiles-tools/blob/master/openmaptiles/sqltomvt.py#L25

We probably don't want to modify the buffer_size in yaml definitions as it makes huge buffer in case you render tiles old way with Mapnik and increases the size of tile.

Copy link
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.

thx, makes sense now

@nyurik
Copy link
Member

nyurik commented Oct 30, 2020

please re-run make rebuild-expected and submit changes

@nyurik nyurik merged commit c0cb6df into openmaptiles:master Oct 30, 2020
@lazaa32 lazaa32 mentioned this pull request Nov 2, 2020
nyurik pushed a commit that referenced this pull request 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 #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.
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