Tile download manager [grant 2020] #40998
Conversation
| * - Upon a request, a QgsTileDownloadManagerReply object (based on QObject) is returned, | ||
| * encapsulating the pending reply. Client can wait for its finished() signal - when | ||
| * it gets emitted, the request has finished with success or failure. Client can delete | ||
| * the reply object before the request is processed - download manager will finish its | ||
| * download (and it will get cached for a future use). |
There was a problem hiding this comment.
Question -- how would a renderer operating on the main thread (eg layout map exports) safely use this? From my interpretation this process implies use of an event loop to wait until the finished signal can be emitted, which isn't safe to do in this circumstance.
There was a problem hiding this comment.
I am not sure there is a safe way to have renderers run in the main thread and doing some async network requests at the same time. As you say, that implies having an event loop to wait for signals, and other GUI events could cause havoc.
The tile download manager is nearly a drop-in replacement for direct use of QNetworkRequest - the behavior is meant to be the same with the finished() signal, so there should be no change in this regard (not making things better nor worse).
I guess to make things safe one would need to use some other means of thread synchronization instead of signals and event loops, or some horrible hacks like adding an event filter on the embedded event loop, "stealing" the events we don't want to deal with, and then posting them back after the event loop has finished :-)
There was a problem hiding this comment.
Well, there's the QgsBlockingNetworkRequest class which was designed for use by providers to handle this situation gracefully (it uses no event loops on the main thread).
This change doesn't make the situation any worse regarding this issue, but it also doesn't help and at some point in the future I think we're going to have to address this properly and ban all event loops/processEvent calls from the data providers. That's why I'd have loved to see this considered in the design on the tile cache...
nyalldawson
added
the
Bug
| }; | ||
|
|
||
|
|
||
| /// @cond PRIVATE |
There was a problem hiding this comment.
suggestion: wouldn't it be better to move all these private classes to new qgstiledownloadmanager_p.h/cpp files?
qgis/QGIS-Enhancement-Proposals#181
Currently used for vector tiles only.