[Qgis-developer] Incremental rendering update

[Hugentobler Marco]

Hi Martin,

>I remember it was doing problems and crashes also on my linux box.

Ok, in this case it is possible we will have similar problems again. I'll do some tests about that. Also some sections of code need to be protected, e.g. the 'open table' button and the info tool may only be called if the canvas is not rendering. Maybe there are others.

>We should probably investigate what's better - whether to render to
>QImage and then pay for the conversion to QPixmap or whether to draw
>directly to QPixmap. From my tests some time ago I've noticed that
>drawing to QImage is faster, however with some older qt4 versions
>(surely all qt4.1.x) rendering to QImage wasn't without bugs... maybe
>it's better with qt4.2 and qt4.3.

Yes, we need to investigate that again. If QPixmap or QImage is faster may also depend on the hardware, as pixmap renders on the graphic card and image on the processor. On my computer, I have the impression that drawing to QPixmap is faster. But does QPixmap already provide antialiasing on X11? Because until now, I preferred QImage because of the nice antialiasing.

>As you write "repainRequested signal is still emitted when the layer
>thinks it should be re-rendered" - I think that's the problem - when
>that signal is used to signal both need to re-render and to signal
>need to just update the image in canvas... do I make sense?

Ah, I see what you mean. Afaik at the moment, only the need for a screen update is signaled with 'repaintRequested'. Maybe we need a different name for the signal, like 'screenUpdateRequested'. The need to re-rendering would occure if something in the layer has changed, e.g. an editing action or maybe a symbology change, right? At the moment, the dialogs trigger a complete canvas re-rendering in these cases. Do you think it would be good to have a dedicated signal for this and to trigger the update from within the maplayer?   

>I think that would be really bad to use a member like
>"mDrawingIsCancelled" again. What sounds more reasonable to me is to
>create a class called e.g. QgsRenderContext which will contain all
>information about rendering that we now pass to draw() functions as
>arguments. Moreover it could get also the information about whether
>the drawing should continue. QgsMapRender or QgsMapCanvas would store
>reference to this context, so they could modify the context to tell
>the layers to stop rendering. How does it sound to you?

And a reference to the rendering context would be passed to the draw() methods of maplayers? Sounds good to me at first glance.

>Probably it would be better to update the layer after some time
>intervals (e.g. every 1/2 second) to save some cpu time.

Makes sense. I just need to check what the most efficient solution is to insert the time query into raster layer code. Because calling functions in the pixel loops could be slow.

Regards,
Marco


-----Ursprüngliche Nachricht-----
Von: Martin Dobias [mailto:wonder.sk at gmail.com]
Gesendet: Do 18.10.2007 15:08
An: Hugentobler  Marco
Cc: qgis-developer at lists.qgis.org
Betreff: Re: [Qgis-developer] Incremental rendering update
 
On 10/17/07, Hugentobler  Marco <marco.hugentobler at karto.baug.ethz.ch> wrote:
> Hi Martin
>
> >- I remember that after Qt4 port Tom disabled processEvents() calls
> >during the rendering due because they were producing UI problems and
> >even crashes... now it seems that it works again (Qt4.2.3 here) - or
> >did you have to do something special to avoid those troubles?
>
> No, it just worked (qt4.3). Maybe the problem was only on Mac?

I remember it was doing problems and crashes also on my linux box.


> >- I've noticed you've disabled usage of QPixmap in QgsMapCanvasMap ...
> >was there any particular reason for that or just because of being a
> >proof of concept?
>
> Yes, it was just the quick and dirty way. I think it is possible to keep the QPixmap and to convert to image when doing a screen refresh.
>

We should probably investigate what's better - whether to render to
QImage and then pay for the conversion to QPixmap or whether to draw
directly to QPixmap. From my tests some time ago I've noticed that
drawing to QImage is faster, however with some older qt4 versions
(surely all qt4.1.x) rendering to QImage wasn't without bugs... maybe
it's better with qt4.2 and qt4.3.

> >- if I understand it correctly, QgsMapCanvas::refresh() has changed
> >its semantics from being trigger of rendering to just being updater of
> >the drawn map. Some of the refresh() calls have been changed to
> >drawIncrementally() and some not, which produces some bugs when
> >working with the map canvas. (wouldn't it be better to preserve
> >original meaning of refresh() function?)
>
> That's a good point. It would probably be better to make a switch into refresh() function that calls QgsMapCanvasMap::update() if canvas is not dirty and QgsMapCanvasMap::render() if dirty.

Yes, I would prefer this.


> >- the usual purpose of layer's repaintRequested signal is different -
> >it's emitted when the layer thinks it should be re-rendered, while you
> >use it to signal that there is some progress in drawing...
>
> The repaintRequested signal is still emitted when the layer thinks it should be re-rendered. Maybe you mean the signal 'drawingProgress'? It is emitted in the same place (also before my changes). I think the reason for this is more of practical nature in that it is the place where qApp->processEvents() is done and so a progress bar would have the change to update.

As you write "repainRequested signal is still emitted when the layer
thinks it should be re-rendered" - I think that's the problem - when
that signal is used to signal both need to re-render and to signal
need to just update the image in canvas... do I make sense?


> >- an important thing what's missing so far is lack of mechanism which
> >would stop rendering - e.g. when you hit escape key or when you move
> >the map it should stop   and start rendering again
>
> Yes, that is still to do. The way this has been done in the qt3 times was to set QgsMapLayer::mDrawingIsCancelled to true for all the layers. We could do the same to stop rendering. Althought it is not very nice to have QgsMapLayer::mDrawingIsCancelled member, as it does not represent a real maplayer property.

I think that would be really bad to use a member like
"mDrawingIsCancelled" again. What sounds more reasonable to me is to
create a class called e.g. QgsRenderContext which will contain all
information about rendering that we now pass to draw() functions as
arguments. Moreover it could get also the information about whether
the drawing should continue. QgsMapRender or QgsMapCanvas would store
reference to this context, so they could modify the context to tell
the layers to stop rendering. How does it sound to you?


> > - raster layers could also do some updates during rendering...
>
> Good point. What do you think should be the policy for a raster layers to update? In fixed Intervals, e.g. once in the middle of the image or in fixed time intervals?

Probably it would be better to update the layer after some time
intervals (e.g. every 1/2 second) to save some cpu time.

Martin