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

New Processing Algorithm to generate raster XYZ tiles #9857

Merged
merged 10 commits into from Apr 26, 2019
Merged

New Processing Algorithm to generate raster XYZ tiles #9857

merged 10 commits into from Apr 26, 2019

Conversation

marcel-dancak
Copy link
Contributor

@marcel-dancak marcel-dancak commented Apr 24, 2019

Description

This is a new processing tool for exporting map as XYZ raster tiles. It supports directory structure and MBTiles output format. Tiles can be stored in PNG or JPG image format.

Sponsored by:

@wonder-sk wonder-sk added this to the 3.8 milestone Apr 24, 2019
@m-kuhn
Copy link
Member

m-kuhn commented Apr 24, 2019

How about merging this with the rasterize map algorithm?

@wonder-sk
Copy link
Member

How about merging this with the rasterize map algorithm?

I don't think there is enough overlap... Both algorithms render a map, but the main focus here is to 1) output XYZ tiles (web mercator), 2) output multiple zoom levels, 3) support specific formats (directory / MBTiles), 4) use metatiling. And the output of this algorithm is not a single raster layer - it can be a directory tree etc.

@m-kuhn
Copy link
Member

m-kuhn commented Apr 24, 2019

Ok then.
A combination of any CRS with multiple zoom levels and metatiles - optionally in any raster file supporting pyramids - would have sounded like a nice use case to me.

@saberraz
Copy link
Contributor

Some good points by @luipir:

IMHO the game changes is to allow:

  1. metatile rendering (as in your code and commonly done)
  2. async tile writing
  3. parallel renderer instead of sync renderer gaining 50% in rendering time for complex stuffs.

Not sure if we are able to cover them within this work. But good to have it documented here for future reference, when we revisit the additional features.

@luipir
Copy link
Contributor

luipir commented Apr 24, 2019

Some good points by @luipir:

I already did this kind of work (but not integrated in processing) for a public institution... but unfortunately they did not published the code :( so I can't share. but they publish the city cadastre data directly from a qgis project. Rendering takes hours so it's important any further optimization.
QTile3 already have parallel writing (and seems that writer of this PR comes from that plugin). What I wasn't able to fix is a rendering problem when using ParallelRenderingJob that does not generate some styles, but it could be a problem introduced by my standalone application (do not have resources to investigate more).

@Gustry
Copy link
Contributor

Gustry commented Apr 24, 2019

Nice addition!
The help, in the YML file, is missing from the PR for now.

python/plugins/processing/algs/qgis/TilesXYZ.py Outdated Show resolved Hide resolved
python/plugins/processing/algs/qgis/TilesXYZ.py Outdated Show resolved Hide resolved
python/plugins/processing/algs/qgis/TilesXYZ.py Outdated Show resolved Hide resolved
python/plugins/processing/algs/qgis/TilesXYZ.py Outdated Show resolved Hide resolved
image.setDotsPerMeterX(dpm)
image.setDotsPerMeterY(dpm)
painter = QPainter(image)
job = QgsMapRendererCustomPainterJob(settings, painter)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is safe to do from a background thread -- I think you need to setup ALL the jobs in advance, in the prepareAlgorithm step

Copy link
Contributor

Choose a reason for hiding this comment

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

@nyalldawson this would mean allocating all tiles at different zoom level in advance! for a mobile project ti may work, but not rendering a big area. I've experience with Barcelona municipality and it generate many GBs from 10 to 21 zoom level. Any clue to prepare jobs without this memory allocatio? something like a job queue prepared in the main thread and the processor/renderer as consumer?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I was thinking about the same - right now we don't have a way to modify map settings of an existing map renderer job and creating great amount of these in advance is probably going to be very wasteful of resources. Looking at the Rasterize algorithm, it also creates QgsMapRendererCustomPainterJob for each tile it renders within processAlgorithm.

While I fully agree this is suboptimal and potentially unsafe, it works fine and it's probably better to keep it this way until there's API to handle this...

@nyalldawson
Copy link
Collaborator

Looking good, nice feature!

My strong preference would be that any new algorithm, especially one as complex as this, is written in c++ and not Python. The Python algorithms have just proved too fragile in the past, and we very much rely on compile time checks to ensure that they don't break. Just something for (everyone) to keep in mind for future work -- in the meantime, you'll need to soak this one with tests 😜

@wonder-sk
Copy link
Member

My strong preference would be that any new algorithm, especially one as complex as this, is written in c++ and not Python. The Python algorithms have just proved too fragile in the past, and we very much rely on compile time checks to ensure that they don't break.

Agreed. Here the main reason to do it in Python was the intention to provide this algorithm as a processing provider in a plugin for QGIS 3.4 and 3.6. I would be happy to move it to c++ post 3.8 release together with safety improvements you were concerned about.

Marcel Dancak and others added 5 commits April 25, 2019 12:15
…test

- for algorithms that produce directory output, it is possible to test
  that directory contents are exactly the same (recursively)
- added possibility to have a project file loaded before an algorithm is run
- documented the new additions (+ few existing ones)
For output to directory, OUTPUT_DIRECTORY destination parameter is used.
For output to MBTiles file, OUTPUT_FILE destimation parameter is used.
@wonder-sk wonder-sk changed the title [WIP] New Processing Algorithm to generate raster XYZ tiles New Processing Algorithm to generate raster XYZ tiles Apr 25, 2019
@wonder-sk wonder-sk merged commit 5e4ea73 into qgis:master Apr 26, 2019

Tile images can be saved as individual images in directory structure, or as single file in MBTiles format.

Tile size is fixed to 256x256.
Copy link
Contributor

Choose a reason for hiding this comment

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

Some information about the output projection would be nice IMHO. Does it always output 3857? I guess yes, but it's good to mention.

BTW, the algorithm is missing tags. I was looking 'mbtiles' in the processing toolbox without finding it for example.

Nice addition, thanks.

@dhelie
Copy link

dhelie commented Apr 30, 2019

that's great. Upon testing i noticed that it is producing pngs with white backgrounds.
adding settings.setBackgroundColor(QColor(Qt.transparent)) to QgsMapSettings() fixes the issue

@isghj5
Copy link

isghj5 commented Jun 27, 2019

I missed the part where this is getting converted to C++ soon, but I got a basic multithreading working with the python version:

In TilesXYZ.py, I took most of the looping code and moved it to a new function, then mapped the function to all the tiles through ThreadPoolExecutor, which handled all the threading for me, except the settings, to avoid threads fighting over settings I made settings a dictionary, one settings per thread.

In my loose test, with the same extent, zoom, image quality, I dropped from 53 seconds to 15 seconds with my quad core, I'm sure people rendering tiles on new beefier ryzen/threadripper/xeon machines will see a bigger boost from this.
speed boost

I'm bad with UI design, not sure how to add a core count option to the UI so the user can change it if they want, this code detects the max from the computer and uses that, but you can just replace cpu_count() with a number if you want in the code.

Known bug: if you hit [Close] before hitting [Cancel] while it's rendering, the threads will stay alive in the background spinning. Not sure where the close button code is.

TilesXYZ.diff.txt

TilesXYZ.py.txt

@PeterPetrik
Copy link
Contributor

@isghj5 Would it be possible to create separate pull request with your addition and discuss there? looks like valuable addition

@DelazJ
Copy link
Contributor

DelazJ commented Dec 24, 2019

@Gustry

The help, in the YML file, is missing from the PR for now.

And it has been merged without (and without labels to generate the issue report in docs repo btw). I think a short help in the dialog for each new algorithm and expression function should be a requirement. This helps users and docs writers.

@wonder-sk
Copy link
Member

@Gustry @DelazJ apologies for ignoring the note about update of YML file and thanks for the reminder - will try to fix that

@Gustry
Copy link
Contributor

Gustry commented Jan 13, 2020

A description in the YML has been added, no? I can see it's the first file in the PR.
But I think it was tags in the alg which were missing.

@DelazJ
Copy link
Contributor

DelazJ commented Jan 13, 2020

@wonder-sk We were documenting it in the docs so had to do some more research but no worries. The issue is that for some reason, they do not display in the GUI. Thanks

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

Successfully merging this pull request may close these issues.

None yet