[Qgis-developer] Patch for screen updates during long rendering operations

[Marco Hugentobler]

Hi Martin,

Thanks for looking into this. I think you are right, everything runs just in 
one thread and we don't need mutexes. So I attached a second version of the 
patch that includes your changes to qgsmapcanvas.cpp (and contains one 
additional if(mDrawing) query).
I'd like to apply the patch to trunk before the feature freeze for 0.9.2, 
which is on 10th January. Everybody is invited to test the patch, especially 
on other platforms.

> Footnotes :)
> - we can enable emitting drawingProgress in QgsMapRender to show the
> progress bar while rendering
> - we can call processEvents() also after rendering every layer (to
> update the progress bar and optionally also map in canvas)

+1. It will be great to have the progress bar back!

> What interests me here most is cancellation of the rendering. In some
> thread on qgis-dev I've been raving already about something called
> rendering context, which would contain all those parameters we'd like
> to pass to layers when drawing and we would pass only that object
> instead of all the arguments:
> - painter
> - extent to draw
> - transformation between map and screen coordinates
> - transformation between layer and map coordinates
> - whether we're in overview or not
> - (in future) scaling parameters
> For me the best solution would be to add a flag "rendering cancelled"
> to that context which would be set by application in case user hits
> Esc.
> As you can see this would mean also some small changes in qgis
> libraries (to add the rendering context) but the changes can be done
> without breaking current API - we could keep original layers' draw()
> functions and mark them as deprecated.

Yes, it is very important to have the possibility to cancel long renderings. 
In my opinion it is worth to do some API changes and to have it prior to 
version 1.0 (coded as usual in a development branch).

Regards and a happy new year,
Marco



Am Dienstag 25 Dezember 2007 02:52:49 schrieb Martin Dobias:
> Hey Marco,
>
> finally I've took a look at the patch and I have some comments:
>
> - from the few tests I've done the patch behaves well :-)
>
> - you use mutexes in the patch. Mutexes are good for synchronization
> between threads, but QGIS runs in one thread - so is it really
> necessary? What we're trying to acheive is to protect the functions
> from indirect recursion - resizeEvent calls rendering, rendering calls
> event loop, event loop calls resizeEvent... But it's all done just in
> one thread so we should be perfectly OK just with some (e.g. static)
> variables that would indicate if we're already in resizeEvent or not.
> I've tried it and seems to work fine.
>
> - the same applies to refresh() - it suffices to skip the refresh if
> mDrawing flag is already true.
>
> - we don't need to store queue of all resizes, we need to store just
> the last one. Moreover if we were using threads, that variable
> mResizeQueue should have been also guarded by mutex (in the current
> implementation of resizeEvent it's not).
>
> I've attached my changes in qgsmapcanvas.cpp (I've changed only
> resizeEvent() and refresh()) if you're interested in my solution
> (works for me on linux).
>
> Footnotes :)
> - we can enable emitting drawingProgress in QgsMapRender to show the
> progress bar while rendering
> - we can call processEvents() also after rendering every layer (to
> update the progress bar and optionally also map in canvas)
>
> What interests me here most is cancellation of the rendering. In some
> thread on qgis-dev I've been raving already about something called
> rendering context, which would contain all those parameters we'd like
> to pass to layers when drawing and we would pass only that object
> instead of all the arguments:
> - painter
> - extent to draw
> - transformation between map and screen coordinates
> - transformation between layer and map coordinates
> - whether we're in overview or not
> - (in future) scaling parameters
> For me the best solution would be to add a flag "rendering cancelled"
> to that context which would be set by application in case user hits
> Esc.
> As you can see this would mean also some small changes in qgis
> libraries (to add the rendering context) but the changes can be done
> without breaking current API - we could keep original layers' draw()
> functions and mark them as deprecated.
>
> This change is important also for fixing the map composer since we
> should add new flag that would indicate whether the output must be
> vector (print or export to PDF/SVG) or can be raster (drawing to
> screen). In case of raster output we can cache symbols as bitmaps.
> Unfortunately this caching is currently happening also with vector
> output and thus resulting with ugly appearance of symbols.
>
> Happy holidays
> Martin
>
>
> On Dec 22, 2007 12:06 PM, Marco Hugentobler
>
> <marco.hugentobler at karto.baug.ethz.ch> wrote:
> > The problem on Mac seems to be the flag Qt::excludeUserInputEvents.
> > Obviously, the resize event is a user input event on Mac, but not on Win
> > and Linux :-)
> >
> > Please test this new patch, it works for me also on Mac. As
> > QApplication->processEvents() is used now instead
> > QApplication->processEvents(Qt::excludeUserInputEvents), some queries
> > have been added to prevent users from adding layers, removing layers,
> > zooming, etc. while the drawing is in progress.
> >
> > Regards,
> > Marco



-- 
Dr. Marco Hugentobler
Institute of Cartography
ETH Zurich
Technical Advisor QGIS Project Steering Committee
-------------- next part --------------
A non-text attachment was scrubbed...
Name: qgis_screen_update2.diff
Type: text/x-diff
Size: 19444 bytes
Desc: not available
Url : http://lists.osgeo.org/pipermail/qgis-developer/attachments/20071230/21e9d834/qgis_screen_update2.bin