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

Martin Dobias wonder.sk at gmail.com
Mon Dec 24 20:52:49 EST 2007


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
-------------- next part --------------
Index: src/gui/qgsmapcanvas.cpp
===================================================================
--- src/gui/qgsmapcanvas.cpp	(revision 7808)
+++ src/gui/qgsmapcanvas.cpp	(working copy)
@@ -109,7 +109,7 @@
   
   moveCanvasContents(TRUE);
   
-  connect(mMapRender, SIGNAL(updateMap()), this, SLOT(updateMap()));
+  //connect(mMapRender, SIGNAL(updateMap()), this, SLOT(updateMap()));
   connect(mMapRender, SIGNAL(drawError(QgsMapLayer*)), this, SLOT(showError(QgsMapLayer*)));
   
   // project handling
@@ -220,6 +220,10 @@
 
 void QgsMapCanvas::setLayerSet(QList<QgsMapCanvasLayer>& layers)
 {
+  if(mDrawing)
+    {
+      return;
+    }
   int i;
   
   // create layer set
@@ -251,6 +255,7 @@
       // Ticket #811 - racicot
       QgsMapLayer *currentLayer = getZpos(i);
       disconnect(currentLayer, SIGNAL(repaintRequested()), this, SLOT(refresh()));
+      disconnect(currentLayer, SIGNAL(screenUpdateRequested()), this, SLOT(updateMap()));
       QgsVectorLayer *isVectLyr = dynamic_cast < QgsVectorLayer * >(currentLayer);
       if (isVectLyr)
       {
@@ -266,6 +271,7 @@
       // Ticket #811 - racicot
       QgsMapLayer *currentLayer = getZpos(i);
       connect(currentLayer, SIGNAL(repaintRequested()), this, SLOT(refresh()));
+      connect(currentLayer, SIGNAL(screenUpdateRequested()), this, SLOT(updateMap()));
       QgsVectorLayer *isVectLyr = dynamic_cast < QgsVectorLayer * >(currentLayer);
       if (isVectLyr)
       {
@@ -344,42 +350,47 @@
 
 void QgsMapCanvas::refresh()
 {
+  // we can't draw again if already drawing...
+  if (mDrawing)
+  	return;
+
+  mDrawing = true;
+
   if (mRenderFlag && !mFrozen)
   {
     clear();
+
+    // Tell the user we're going to be a while
+    QApplication::setOverrideCursor(Qt::WaitCursor);
+    mMap->render();
+
+    mDirty = false;
+
+    // notify any listeners that rendering is complete
+    QPainter p;
+    p.begin(&mMap->pixmap());
+    emit renderComplete(&p);
+    p.end();
     
-    // schedule update of map
-    mMap->update();
+    // notifies current map tool
+    if (mMapTool)
+      mMapTool->renderComplete();
+    
+    // Tell the user we've finished going to be a while
+    QApplication::restoreOverrideCursor();
   }
+
+  mDrawing = false;
 } // refresh
 
-
-void QgsMapCanvas::render()
+void QgsMapCanvas::updateMap()
 {
-  QgsDebugMsg("Starting rendering");
+  if(mMap)
+    {
+      mMap->update();
+    }
+}
 
-  // Tell the user we're going to be a while
-  QApplication::setOverrideCursor(Qt::WaitCursor);
-
-  mMap->render();
-  mDirty = false;
-
-  // notify any listeners that rendering is complete
-  QPainter p;
-  p.begin(&mMap->pixmap());
-  emit renderComplete(&p);
-  p.end();
-  
-  // notifies current map tool
-  if (mMapTool)
-    mMapTool->renderComplete();
-
-  // Tell the user we've finished going to be a while
-  QApplication::restoreOverrideCursor();
-
-} // render
-
-
 //the format defaults to "PNG" if not specified
 void QgsMapCanvas::saveAsImage(QString theFileName, QPixmap * theQPixmap, QString theFormat)
 {
@@ -429,9 +440,13 @@
   refresh();
 }
 
-
 void QgsMapCanvas::setExtent(QgsRect const & r)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   if (r.isEmpty())
   {
     QgsDebugMsg("Setting empty extent!");
@@ -470,6 +485,11 @@
 
 void QgsMapCanvas::zoomFullExtent()
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   QgsRect extent = fullExtent();
   // If the full extent is an empty set, don't do the zoom
   if (!extent.isEmpty())
@@ -482,6 +502,11 @@
 
 void QgsMapCanvas::zoomPreviousExtent()
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   QgsRect current = extent();
   setExtent(mLastExtent);
   mLastExtent = current;
@@ -514,6 +539,11 @@
 
 void QgsMapCanvas::zoomToSelected()
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   QgsVectorLayer *lyr = dynamic_cast < QgsVectorLayer * >(mCurrentLayer);
 
   if (lyr)
@@ -543,6 +573,11 @@
 {
   QgsDebugMsg("keyPress event");
 
+  if(mDrawing)
+    {
+      return;
+    }
+
   emit keyPressed(e);
 
   if (mCanvasProperties->mouseButtonDown || mCanvasProperties->panSelectorDown)
@@ -622,6 +657,11 @@
 void QgsMapCanvas::keyReleaseEvent(QKeyEvent * e)
 {
   QgsDebugMsg("keyRelease event");
+  
+  if(mDrawing)
+    {
+      return;
+    }
 
   switch( e->key() )
   {
@@ -649,6 +689,11 @@
 
 void QgsMapCanvas::mousePressEvent(QMouseEvent * e)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   // call handler of current map tool
   if (mMapTool)
     mMapTool->canvasPressEvent(e);  
@@ -664,6 +709,11 @@
 
 void QgsMapCanvas::mouseReleaseEvent(QMouseEvent * e)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   // call handler of current map tool
   if (mMapTool)
   {
@@ -695,17 +745,31 @@
 
 void QgsMapCanvas::resizeEvent(QResizeEvent * e)
 {
-  int width = e->size().width(), height = e->size().height();
-//  int width = visibleWidth(), height = visibleHeight();
-  mScene->setSceneRect(QRectF(0,0,width, height));
+  static bool isAlreadyIn = false;
+  static QSize lastSize = QSize(-1,-1);
+  
+  lastSize = e->size();
 
-  mMap->resize(QSize(width,height));
+  if (isAlreadyIn) return;
+  isAlreadyIn = true;
 
-  // notify canvas items of change
-  updateCanvasItemsPositions();
-  
-  updateScale();
-  refresh();
+  while (lastSize != QSize(-1,-1))
+    {
+      int width = lastSize.width();
+      int height = lastSize.height();
+      lastSize = QSize(-1,-1);
+      
+      mScene->setSceneRect(QRectF(0,0,width, height));
+      
+      mMap->resize(QSize(width,height));
+      
+      // notify canvas items of change
+      updateCanvasItemsPositions();
+      
+      updateScale();
+      refresh();
+    }
+  isAlreadyIn = false;
 } // resizeEvent
 
 
@@ -734,6 +798,11 @@
   
   QgsDebugMsg("Wheel event delta " + QString::number(e->delta()));
 
+  if(mDrawing)
+    {
+      return;
+    }
+
   switch (mWheelAction)
   {
     case WheelZoom:
@@ -770,6 +839,11 @@
 
 void QgsMapCanvas::zoomWithCenter(int x, int y, bool zoomIn)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   double scaleFactor = (zoomIn ? 1/mWheelZoomFactor : mWheelZoomFactor);
 
   // transform the mouse pos to map coordinates
@@ -783,6 +857,11 @@
 
 void QgsMapCanvas::mouseMoveEvent(QMouseEvent * e)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   mCanvasProperties->mouseLastXY = e->pos();
 
   if (mCanvasProperties->panSelectorDown)
@@ -921,11 +1000,10 @@
 void QgsMapCanvas::setRenderFlag(bool theFlag)
 {
   mRenderFlag = theFlag;
-  // render the map
   if(mRenderFlag)
-  {
-    refresh();
-  }
+    {
+      refresh();
+    }
 }
 
 void QgsMapCanvas::connectNotify( const char * signal )
@@ -942,6 +1020,11 @@
 
 void QgsMapCanvas::panActionEnd(QPoint releasePoint)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   // move map image and other items to standard position
   moveCanvasContents(TRUE); // TRUE means reset
   
@@ -987,6 +1070,11 @@
 {
   QgsDebugMsg("panAction: entering.");
 
+  if(mDrawing)
+    {
+      return;
+    }
+
   // move all map canvas items
   moveCanvasContents();
   
@@ -996,6 +1084,11 @@
 
 void QgsMapCanvas::moveCanvasContents(bool reset)
 {
+  if(mDrawing)
+    {
+      return;
+    }
+
   QPoint pnt(0,0);
   if (!reset)
     pnt += mCanvasProperties->mouseLastXY - mCanvasProperties->rubberStartPoint;
@@ -1026,15 +1119,6 @@
 
 }
 
-
-void QgsMapCanvas::updateMap()
-{
-  // XXX updating is not possible since we're already in paint loop
-//  mCanvas->update();
-//  QApplication::processEvents();
-}
-
-
 void QgsMapCanvas::showError(QgsMapLayer * mapLayer)
 {
 //   QMessageBox::warning(
@@ -1097,7 +1181,11 @@
 
 void QgsMapCanvas::zoom(double scaleFactor)
 {
-  
+  if(mDrawing)
+    {
+      return;
+    }
+
   QgsRect r = mMapRender->extent();
   r.scale(scaleFactor);
   setExtent(r);


More information about the Qgis-developer mailing list