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

Adjust tile queries to allow for z16 metatiles #478

Closed
nvkelso opened this issue Jan 8, 2016 · 14 comments
Closed

Adjust tile queries to allow for z16 metatiles #478

nvkelso opened this issue Jan 8, 2016 · 14 comments
Assignees
Milestone

Comments

@nvkelso
Copy link
Member

nvkelso commented Jan 8, 2016

Specifically, we pack some stuff only into z17 and z18 tiles now. If we meta tile, then those need to be included in z16 (and perhaps marked that their min_zoom is only valid at z17, z18).

EG, in POIs:

             WHEN (aeroway_val IN ('gate')
                   OR amenity_val IN (
                 'atm', 'bicycle_rental', 'bicycle_parking', 'bus_stop',
                 'drinking_water', 'emergency_phone',
                 'parking', 'post_box', 'telephone', 'theatre',
                 'toilets')
                   OR highway_val IN ('bus_stop', 'traffic_signals')
                   OR historic_val IN ('memorial')
                   OR leisure_val IN ('playground', 'slipway')
                   OR man_made_val IN ('mast', 'water_tower')
                   OR office_val IN ('accountant', 'administrative', 'advertising_agency',
                'architect', 'association', 'company', 'consulting', 'educational_institution',
                'employment_agency', 'estate_agent', 'financial', 'foundation', 'government',
                'insurance', 'it', 'lawyer', 'newspaper', 'ngo', 'notary', 'physician',
                'political_party', 'religion', 'research', 'tax_advisor', 'telecommunication',
                'therapist', 'travel_agent', 'yes')
                   OR shop_val IN ('bakery', 'bicycle', 'books', 'butcher', 'car', 'car_repair',
                                 'clothes', 'computer', 'convenience',
                                 'doityourself', 'dry_cleaning', 'fashion', 'florist', 'gift',
                                 'greengrocer', 'hairdresser', 'jewelry', 'mobile_phone',
                                 'optician', 'pet')
                   OR tourism_val IN ('bed_and_breakfast', 'chalet', 'guest_house',
                                    'hostel', 'hotel', 'motel')
                   OR railway_val IN ('subway_entrance')) THEN 17
             WHEN (amenity_val IN ('bench', 'waste_basket')) THEN 18
@nvkelso
Copy link
Member Author

nvkelso commented Feb 4, 2016

Needs to be coordinated with scene file changes (to suppress these from display at zoom 16/17 as appropriate)

@rmarianski
Copy link
Member

@nvkelso I discussed the implementation with @zerebubuth a little bit, and a good way forward would be to start including all pois in z16 tiles. For pois that don't have a min_zoom, we could set that to z18, or maybe flag that some other way?

@zerebubuth
Copy link
Member

All POIs have a mz_poi_min_zoom, don't they? Otherwise,

  WHERE                                                                                              
    {{ bounds|bbox_filter('way') }}                                                                  
    AND mz_poi_min_zoom <= {{ zoom }}                                                                

The second part of the query would never be true?

@rmarianski
Copy link
Member

Ah, never mind, I misunderstood earlier. If we just query for all pois that have a min zoom starting at z16, we'll get the behavior that we want.

@nvkelso
Copy link
Member Author

nvkelso commented Feb 9, 2016

Should we export the min_zoom property on all the POIs now, or only on the z17 and z18 features?

@rmarianski
Copy link
Member

@rmarianski
Copy link
Member

I exported the min_zoom property on all POIs in the vector-datasource pr.

@rmarianski
Copy link
Member

We might want to consider adding min_zoom to the boundaries layer too, to handle moving the barrier=fence and boundary=fence to z16.

@nvkelso
Copy link
Member Author

nvkelso commented Feb 23, 2016

LGTM, let's merge this in and test on dev.

Regarding adding min_zoom to the boundaries layer – let's file a new issue for that for future consideration. It's related to this work, but involves a bunch of other decisions that can be separated.

@rmarianski
Copy link
Member

When deploying this change, we'll need to run a script that takes all toi at z>16, and ensures that the corresponding tile at z16 is added.

Should I make a python script for this and stick it in the migrations directory?

@zerebubuth
Copy link
Member

Yup, sounds good.

@nvkelso
Copy link
Member Author

nvkelso commented Feb 26, 2016

@rmarianski: Do we need to prune any z17 and z18 tiles from Tiles of Interest list to support this z16 meta-tiling change, or was that already accomplished in our earlier pruning? I think it was, just double checking.

@rmarianski
Copy link
Member

We shouldn't have do to worry about anything wrt pruning specifically for this change. The earlier pruning would have removed anything in this range with just 0 or 1 view.

@nvkelso
Copy link
Member Author

nvkelso commented Feb 26, 2016

I've tested on TopoJSON, GeoJSON, and MVT formats and all looks good.

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

No branches or pull requests

3 participants