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

Added support for the use of tile_map_edge_buffer when using WMS with… #5199

Closed
wants to merge 1 commit into from

Conversation

deduikertjes
Copy link
Contributor

… tiled=true vendor specific parameter (WMS-C)

The reasoning behind this:
I really prefer to use tiled request in combination with WMS above using the cgi tiled mode with the mode= parameter. I then can use all the nice WMS-goodies as user styling, getlegend, wms-dimensions, etc.
In an OpenLayers client I can set a gutter to prevent tiling artifacts, but that doesn't help if a user prefers a different client (like QGIS). Even OpenLayers recommends to remove tiling artifacts server side.

Besides that, mapserver is great as a back end to any cached tiling service. But all of these require an extra config to maintaing and extra software in the stack.

Nginx has a very nice LRU-cache which can be combined with operating mapserver in WMS-C mode by aligning the GetMap requests along a grid. By combining mapserver with nginx caching we have an auto configuring cached tiling service.
When operating mapserver in such a way it becomes important to add a buffer (just as in cgi tiling mode) to prevent tiling artifacts.

Comments on my code:

  • As far as I can judge this path is rather unobtrusive.
  • I think I added all parts at the right places. I might be mistaken as mapwms.c is rather involved.
  • I didn't add support for meta-tiling as I don't quiet understand the use for that in tiling mode.
  • I duplicated some code form tilemap.c. It might or might not be neater to separate this code.
  • This is actually my first coding effort in C. Although everything works fine for me it might be wise to check for stupid mistakes and/ or adapt coding style.

Future development:

  • it would be nice to update the docs stating that adding 'tiled=true' to the WMS-request triggers this functionality.
  • it would be nice to actually create a WMS-C getcapabilities document when the request is invoked with the tiled=true parameter.

… tiled=true vendor specific parameter (WMS-C)
map->height += 2 * map_edge_buffer;

/*
** Ensure the labelcache buffer is greater than the buffer.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about this one, and I haven't tested it myself as I'm not using any labels in this setup.
But it's copied from maptile.c so It shouldn't be too wrong.

@tbonfort
Copy link
Member

This is an interesting option that needs a bit more investigation:

  • Given that this is new functionality, it should target a 7.2 release (i.e. be a pull request against master, not branch-7-0)
  • Expanding the geographical extents as you are doing may prove to be problematic in case reprojections are at play (e.g. -180,-90,180,90 is OK, but expanded to -185,-95,185,95 is going to cause issues with the dateline wrapping when reprojecting).
  • the msImageCreate and mergeRasterBuffer part are not optimal. We can obtain the same result by playing with the rasterBufferObj's data->rgb.pixels and data->rgb.row_stride values.

@tbonfort tbonfort added this to the 7.2 Release milestone Feb 24, 2016
@sdlime
Copy link
Member

sdlime commented Mar 20, 2018

Not sure where this one sits. @tbonfort references some refactoring should be done but I don't see that has been done.

@MarcoDuiker
Copy link

Refactoring has not been done. At the time it was beyond my Git-fu an C-fu to do the requested refactoring. Probably still is :-(.

My assignment where this work took place has ended. If I were convinced that I could do the requested refactoring I would do so anyhow.

As the reasoning behind this patch, in my opinion, is still very valid it would be nice if someone more knowledgeable than me could adopt this. For such a person these two issues:

  • Expanding the geographical extents as you are doing may prove to be problematic in case reprojections are at play (e.g. -180,-90,180,90 is OK, but expanded to -185,-95,185,95 is going to cause issues with the dateline wrapping when reprojecting).
  • the msImageCreate and mergeRasterBuffer part are not optimal. We can obtain the same result by playing with the rasterBufferObj's data->rgb.pixels and data->rgb.row_stride values.

probably are not that difficult to address.

@jmckenna jmckenna modified the milestones: 7.2 Release, 7.6.1 Release Mar 20, 2020
@jmckenna jmckenna modified the milestones: 7.6.1 Release, 8.0 Release Aug 19, 2020
@jmckenna
Copy link
Member

@deduikertjes would you mind rebasing this against master (instead of branch-7-0) ?

rouault added a commit to rouault/mapserver that referenced this pull request Feb 12, 2021
… tiled=true vendor specific parameter (WMS-C)

This is a rebase of MapServer#5199
on top of latest main, with the addition of a test.

Comments of MapServer#5199 (comment)
have been only adressed by adding appropriate TODOs in the code.

Co-authored-by: deduikertjes <deduikertjes@xs4all.nl>
@rouault
Copy link
Contributor

rouault commented Feb 12, 2021

Superseded per #6233

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.

None yet

6 participants