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

Preview of tiled layers (WMTS) + support XYZ tile layers #3473

Merged
merged 26 commits into from
Sep 13, 2016

Conversation

wonder-sk
Copy link
Member

This introduces live preview when rendering WMTS layers - as soon as individual tiles are loaded, they are shown in map canvas... no need to wait with a blank map until all tiles are fully downloaded. Additionally, if there are already locally cached tiles of other zoom levels, they may be used in the preview while the tiles with best matching zoom level are being downloaded. This greatly improves the user experience when working with WMTS layers.

Additionally, I have added native support for XYZ tile layers into WMS provider (based on existing implementation of WMTS tiling). This allows loading of various new raster tile sources (e.g. OpenStreetMap tiles) that were before available only with QuickMapServices or OpenLayers plugins. To use XYZ tile layers, open the browser dock in QGIS and look for "Tile Servers (XYZ)" root entry. Right-clicking will open a menu to add connections. For example for OpenStreetMap the URL would be http://c.tile.openstreetmap.org/{z}/{x}/{y}.png

The work on WMTS live preview has been funded by Land Information New Zealand.

The work on XYZ tile layers has been funded by Lutra Consulting.

Otherwise we would end up with rendering artifacts due to overpainting
Yes, that means OpenStreetMap tiles and various other sources!

No GUI so far...
... so we first receive tiles in the middle rather those at the border
Marked raster interface as uncopiable (it should have always been uncopiable)
Moved all temporary projector members to a private class,
so even recursive block() calls will not affect each other
(there is no new performance penalty as block() call always
recomputes the temporary control point matrix anyway)
It would make sense to have it enabled by default with WMTS too
This makes it easier to get at least basic preview without having to wait
until new tiles are downloaded. The strategy is to use tiles up to two levels
lower resolution tiles and one level higher resolution tiles.
@wonder-sk
Copy link
Member Author

For those looking for Python API for XYZ tile layers:

uri = QgsDataSourceUri()
uri.setParam("type", "xyz")
uri.setParam("url", "http://c.tile.openstreetmap.org/{z}/{x}/{y}.png")
layer = QgsRasterLayer(unicode(uri.encodedUri()), "OpenStreetMap", "wms")

@pcav
Copy link
Member

pcav commented Sep 9, 2016

Thanks a lot, very useful. Probably good to add a bunch of default server, especially OSM?

@m-kuhn
Copy link
Member

m-kuhn commented Sep 9, 2016

That's really good news! 👏

@nirvn
Copy link
Contributor

nirvn commented Sep 9, 2016

@wonder-sk , I was wondering what new feature would make me switch my daily QGIS build to master. This _ is _ it! 😄

@NathanW2
Copy link
Member

NathanW2 commented Sep 9, 2016

YES!

On Fri, 9 Sep 2016 7:31 pm Mathieu Pellerin notifications@github.com
wrote:

@wonder-sk https://github.com/wonder-sk , I was wondering what new
feature would make me switch my daily QGIS build to master. This _ is _ it!
😄


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#3473 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAXS3JcfvOYkuGRkSAmNcYX-qriwtXSGks5qoSeBgaJpZM4J42v5
.

@nirvn
Copy link
Contributor

nirvn commented Sep 9, 2016

@wonder-sk , @nyalldawson , yay, rotation support:
untitled

@nirvn
Copy link
Contributor

nirvn commented Sep 9, 2016

@wonder-sk , suggestion: fix layer menu item's "zoom to native resolution (100%)" to have the zoom level change to the closest value to display pixel-perfect tiles at the current tile level.

@nirvn
Copy link
Contributor

nirvn commented Sep 12, 2016

Ok, more findings:

  • Qt4 build does not crash
  • Trying a Qt4 build made me realize that progressive rendering is not working on Qt5 build, which is a pretty serious issue (possibly linked to why a crash occurs under Qt5)
  • On Qt5, I see the following error message repeatedly printed in the console: 2016-09-12T13:34:35 1 QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread
  • The Qt5-related crasher does not occur if you only have one XYZ layer

@wonder-sk
Copy link
Member Author

I have fixed previews in Qt5 and an unrelated crash in Qt5.

The crash mentioned above seems to be caused by a race in QgsNetworkAccessManager::createRequest(), specifically the part related to auth manager. @nirvn could you please confirm that the crash happens only if the layers use HTTPS protocol?


if ( it == tileMatrices.constEnd() )
return nullptr;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious - what's the change in qt5 which required this? It's hard to tell from this commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry I should have explained it in the commit message... in Qt4 if the iterator is at the first item, going backwards will set the iterator to be equal to constEnd(). However in Qt5 this is not the case and it would just leave iterator in undefined state. So the main change in the commit is to first check if we are not already at constBegin() before going backwards. Probably this was changed to be in line with behavior of iterators of ordinary c++ containers... not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good insight. I m assuming these bits of knowledge will come in handy for other folks too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @wonder-sk - just helps to know what other potential issues we'll run into..

@nirvn
Copy link
Contributor

nirvn commented Sep 13, 2016

@wonder-sk , great I'll try the updated PR and report.

@nirvn
Copy link
Contributor

nirvn commented Sep 13, 2016

@wonder-sk , preview/progressive rendering now works on Qt5, kudos.

Regarding the QSocketNotifier, the error is actually caused by a delimited text layer I have in the project, not related to this PR. Removing that layer seems to get rid of the crasher (so good so far). I'll file a separate issue on the hub regarding this.

@wonder-sk
Copy link
Member Author

Thanks for testing. If there are no objections, I will merge it later today.

It would be best to create feature requests afterwards for any extra functionality.

@nirvn
Copy link
Contributor

nirvn commented Sep 13, 2016

@wonder-sk , when the commit in PR #3483 is applied, I can't get this PR to crash anymore (with delimited text layer), yay.

IMO, the absence of a user-friendly way to set a zmin / zmax will have to be addressed during this dev cycle for the XYZ support to be as good as it gets. Editing the .qgs file to add a &zmax=12 value to the string isn't a viable option ;) that said, happy to report adding such a zmax value to the datasource uri does work.

@nirvn
Copy link
Contributor

nirvn commented Sep 13, 2016

@wonder-sk , just ran into a composer export to file issue, whereas not all tiles are rendered.

Here's the project file:
osm.qgs.zip

Simply open composer 6, and export (at 300dpi) the composer sheet to a .png. Over here, this is what I get:
z2

Max. 100 tiles is just too low. The max request area is currently 2000x2000 pixels
which boils down to approx 8x8 tiles of size 256x256, therefore max 64 tiles.
However this is the ideal case if the view scale matches the scale of tiles.
If the map view scale is approx in between two levels, we may need 12x12 tiles
to serve the request, so bumping the maximum to 256 should be enough.
@wonder-sk
Copy link
Member Author

@nirvn

  • great work with fixing the delimited text provider crash
  • composer export is fixed now
  • I will address zmin/zmax for XYZ tiles later separately. I know it is an issue for some tile sources, but unfortunately I need to get other things done with higher priority...

@wonder-sk
Copy link
Member Author

The recent test failure is unrelated, looks like a false positive in testziplayer.

@m-kuhn
Copy link
Member

m-kuhn commented Sep 13, 2016

The recent test failure is unrelated, looks like a false positive in testziplayer.

Seen these too many times but never a real alarm from this test, would be good to turn it off.

@nyalldawson
Copy link
Collaborator

@m-kuhn is it always the tar test which fails? Maybe we should just disable this one on Travis rather than the whole lot? (But otherwise, yes let's blacklist this)

@nirvn
Copy link
Contributor

nirvn commented Sep 13, 2016

@wonder-sk , composer export fixed. I've now emptied my bag of comments ;) great work.

@wonder-sk wonder-sk merged commit 717e716 into qgis:master Sep 13, 2016
@wonder-sk wonder-sk deleted the wmts-preview branch September 14, 2016 04:55
@nyalldawson
Copy link
Collaborator

@wonder-sk was playing with this today and it works really well. very smooth!

One thing I'm not sure about is the "grid" image used as a temporary placeholder. To me this is very distracting and I don't think it's necessary - we don't use placeholders for other providers while rendering is underway. Are you attached to this behaviour?

@wonder-sk
Copy link
Member Author

@nyalldawson thanks!

Nope I'm not attached to the temporary grid 😄 It was helpful when debugging some issues when loading tiles, we can get rid of it now...

@NathanW2
Copy link
Member

image

(joking....) :)

@Gustry
Copy link
Contributor

Gustry commented Sep 14, 2016

Hi @wonder-sk
This feature seems very nice !
But I can't compile master since this merge.

[ 56%] Building CXX object src/providers/wms/CMakeFiles/wmsprovider_a.dir/qgswmsprovider.cpp.o
cd /Users/etienne/compil/QGIS/build-debug-master/src/providers/wms && /usr/bin/g++   -DHAS_MOVE_SEMANTICS -DQGISDEBUG=1 -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_SQL_LIB -DQT_SVG_LIB -DQT_WEBKIT_LIB -DQT_XML_LIB -DWITH_QTWEBKIT -isystem /usr/local/Cellar/qt/4.8.7_2/include -iframework /usr/local/Cellar/qt/4.8.7_2/lib -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtSvg -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtWebKit -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtGui -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtXml -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtSql -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtNetwork -isystem /usr/local/Cellar/qt/4.8.7_2/lib/QtCore.framework/Headers -I/Users/etienne/compil/QGIS/build-debug-master -I/Users/etienne/compil/QGIS/src/providers/wms/. -I/Users/etienne/compil/QGIS/src/providers/wms/../../core -I/Users/etienne/compil/QGIS/src/providers/wms/../../core/auth -I/Users/etienne/compil/QGIS/src/providers/wms/../../core/geometry -I/Users/etienne/compil/QGIS/src/providers/wms/../../core/raster -I/Users/etienne/compil/QGIS/src/providers/wms/../../gui -I/Users/etienne/compil/QGIS/src/providers/wms/../../gui/auth -I/Users/etienne/compil/QGIS/build-debug-master/src/providers/wms/../../ui -isystem /usr/local/opt/gdal-20/include -isystem /usr/local/opt/geos/include -isystem /usr/local/Cellar/qt/4.8.7_2/include/QtScript -isystem /usr/local/opt/qca/lib/qca.framework/Headers  -DSPATIALITE_VERSION_GE_4_0_0 -DSPATIALITE_VERSION_G_4_1_1 -DSPATIALITE_HAS_INIT_EX -std=c++11 -Wno-error=c++11-narrowing -Wall -Wextra -Wno-long-long -Wformat-security -Wno-strict-aliasing -Wno-return-type-c-linkage -Wno-overloaded-virtual -Qunused-arguments -O2 -g -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk -fvisibility=hidden   -DCORE_EXPORT= -DGUI_EXPORT= -DPYTHON_EXPORT= -DANALYSIS_EXPORT= -DAPP_EXPORT= -DCUSTOMWIDGETS_EXPORT= -DSERVER_EXPORT= -F/usr/local/lib  -o CMakeFiles/wmsprovider_a.dir/qgswmsprovider.cpp.o -c /Users/etienne/compil/QGIS/src/providers/wms/qgswmsprovider.cpp
/Users/etienne/compil/QGIS/src/providers/wms/qgswmsprovider.cpp:509:20: error: use of undeclared identifier 'exp10'
  double epsilon = exp10( significantDigits - 5 ); // floats have 6-9 significant digits
                   ^
1 error generated.
make[2]: *** [src/providers/wms/CMakeFiles/wmsprovider_a.dir/qgswmsprovider.cpp.o] Error 1
make[1]: *** [src/providers/wms/CMakeFiles/wmsprovider_a.dir/all] Error 2
make: *** [all] Error 2
etienne@:~/compil/QGIS/build-debug-master (master) $ 

Mac 10.11.6
Thanks

@wonder-sk
Copy link
Member Author

@Gustry does it help if you add #include <math.h> at the top of qgswmsprovider.cpp ?

Just wondering - what platform is that? (all other platforms seem to build fine)

@Gustry
Copy link
Contributor

Gustry commented Sep 14, 2016

@rduivenvoorde
Copy link
Contributor

rduivenvoorde commented Sep 19, 2016

@wonder-sk Hi Martin, nice!

I'm wondering though how you determine the 'native resolution'?
Because we have some national WMTS services, for example:
http://geodata.nationaalgeoregister.nl/wmts?VERSION=1.0.0&request=GetCapabilities
(and take the 'brtachtergrondkaart' as layer)
And we have our own tile-schema for it (WITH their own resolutions).
So when I use the new 'native resolution' in the context menu, it is not the one I need (I think).

FYI: these are the native resolutions for our national schema (epsg:28992):

<qgsScales version="1.0">
    <scale value="1:13003994"/>
    <scale value="1:6501997"/>
    <scale value="1:3250998"/>
    <scale value="1:1625499"/>
    <scale value="1:812750"/>
    <scale value="1:406375"/>
    <scale value="1:203187"/>
    <scale value="1:101594"/>
    <scale value="1:50797"/>
    <scale value="1:25398"/>
    <scale value="1:12699"/>
    <scale value="1:6350"/>
    <scale value="1:3175"/>
    <scale value="1:1587"/>
    <scale value="1:794"/>
    <scale value="1:397"/>
    <scale value="1:198"/>
    <scale value="1:99"/>
</qgsScales>

Taken from: http://www.geonovum.nl/sites/default/files/nederlandse_richtlijn_tiling_-_versie_1.1.pdf

FYI2: the tile-slider picked up the right scale values (IF you enalbe the slide and make the wmts layer active)

@wonder-sk
Copy link
Member Author

Hi @rduivenvoorde

Native resolution of rasters does not always make sense. For GDAL rasters we have the true resolution of the whole layer, but that's not really the case for WMS/WMTS/XYZ where there may be multiple native resolutions. Calling layer.rasterUnitsPerPixelX() should give you horizontal resolution - if unknown it falls back to returning 1, a dummy resolution. As Mathieu suggested above, more reasonable approach would be to have "zoom to closest native resolution" for WMTS/XYZ layers instead. Possibly with a sub-menu to choose a zoom level.

Tile scale dock widget is currently the way how to switch between scales that are native to WMTS/XYZ layers. Internally, the widget calls layer.dataProvider().property("resolutions") to get list of native resolutions which are then used as values for the slider.

Please note that my recent work has not touched any of this existing functionality - this is just an explanation how things work. The context menu could get some more love to better support WMS/WMTS/XYZ layers

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

8 participants