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
Vector tiles #166
Vector tiles #166
Conversation
I noticed it's not setting the right mime-type on requests. Still works but that's something I need to fix... --Steve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this @sdlime . I'm surprised there's so little change needed, I think we need a few more checks for robustness, notably:
- disallowing metatile support when using mvt
- disallowing tile merging when one of the layers is mvt
- going through the code and checking everywhere tile->raw_image is used to make sure the blob case is handled
… mime-type can be set.
Thanks for the review. I was surprised too. Once I got the tile written to the disk cache the response piece just sort of worked (just fixed mime-type). I knew robustness would be an issue. Aside from that the approach looks reasonable? Could also (maybe) enable UTFGrids through this. I thought that had been done via GSoC but I guess not. --Steve |
@tbonfort, I tried and this approach does work for UTFGrid too, for example:
Perhaps BLOB as a type is too narrow, maybe RAW instead? Steve |
…ta (like vector tiles). It can also be used for something like UTFGrid, so...
Testing comment (will update as I can test cache types):
|
@tbonfort I think it's easiest just to disable WMS support for tilesets referencing raw formats. I've updated service_wms.c to do that but wanted to get your thoughts. In getTile mode with one layer it can work but that's the only case where it seems to make sense. --Steve |
@tbonfort, just a bump - I know you're busy. |
sorry about my inactivity... ok for disabling WMS for such layers. do you want a more in depth review? cheers |
No worries… I wouldn’t feel comfortable merging without a thorough review. I think I hit all the integration spots but I don’t know that code like you do. Would be nice to get a new MapCache release out with this patch and whatever else is sitting in master. I suspect I owe you some tests as well – some thoughts on what those should entail would be helpful.
From: Thomas Bonfort [mailto:notifications@github.com]
Sent: Wednesday, November 14, 2018 4:17 PM
To: mapserver/mapcache <mapcache@noreply.github.com>
Cc: Lime, Steve D (MNIT) <steve.lime@state.mn.us>; Mention <mention@noreply.github.com>
Subject: Re: [mapserver/mapcache] Vector tiles (#166)
sorry about my inactivity... ok for disabling WMS for such layers. do you want a more in depth review? cheers
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#166 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABhm-zs2OnbZmeOyxTNc2ZsLwPr8oAYeks5uvJZ3gaJpZM4M1F-2>.
|
lib/source_wms.c
Outdated
@@ -92,7 +93,7 @@ void _mapcache_source_wms_render_map(mapcache_context *ctx, mapcache_map *map) | |||
mapcache_http_do_request(ctx,http,map->encoded_data,NULL,NULL); | |||
GC_CHECK_ERROR(ctx); | |||
|
|||
if(!mapcache_imageio_is_valid_format(ctx,map->encoded_data)) { | |||
if(map->tileset->format->type != GC_RAW && !mapcache_imageio_is_valid_format(ctx,map->encoded_data)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not checking tileset->format!=NULL might be the cause of the crash in MapServer/MapServer#5596 (comment)_
lib/service_wms.c
Outdated
@@ -728,7 +733,7 @@ void _mapcache_service_wms_parse_request(mapcache_context *ctx, mapcache_service | |||
goto proxies; | |||
} else { | |||
mapcache_tileset *tileset = mapcache_configuration_get_tileset(config,str); | |||
if(!tileset) { | |||
if(!tileset || tileset->format->type == GC_RAW) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not checking tileset->format!=NULL might be the cause of the crash in MapServer/MapServer#5596 (comment)_
lib/core.c
Outdated
@@ -502,6 +512,7 @@ mapcache_http_response *mapcache_core_get_map(mapcache_context *ctx, mapcache_re | |||
} | |||
|
|||
/* compute the content-type */ | |||
ctx->log(ctx,MAPCACHE_DEBUG,"SDL: setting content type (2)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stray debug message to remove
LGTM once the checks for tileset->format!=NULL are added (there are a couple few others I haven't ref'd directly in service_wms.c) |
…w format (in response to tbonfort comments).
I think I caught all the format != NULL checks... added a little helper function. |
Hi Thomas: I have something working with WMS source and a disk cache. Just wondering if you could take a peek. I'm thinking WMS is probably the most meaningful source, but other cache types would need support. Config would look something like:
--Steve