This needs a rebase, sorry.

This PR contains two fixes.

The first one moves the agg draw back to the main thread. Qt uses a dedicated render thread for paintEvent. Although the agg buffer should be updated as closely to the qt repaint as possible in order to take all the latest events into account, accessing the scenegraph from the render thread is dangerous since the scenegraph has no locking mechanism.

The second and third commit address problems with the rubberband in qt. The second adds a remove_rubberband() handler to backend_bases that is invoked when the zoom rectangle is released.

The last commit changes the rubberband handling in the qt backend to utilize the rubberband removal handler. The previous solution was to simply remove the rectangle after each draw. This method fails when there is an additional paintEvent between two draw_rubberband() calls because it clears the rubberband while it is still being dragged (flicker). Also, there is the chance that the paintEvent requested by the last draw_rubberband() call before releasing the mouse might get delayed until after the draw_idle() call for zooming in. This causes the rubberband to be drawn on top of the zoomed-in figure (bad) and since there are no more events after the release the rubberband will stay on screen (worse).
Also, changing the state of the rubberband doesn't trigger an agg buffer redraw anymore, freeing some CPU cycles while dragging.

@jrevans Do you have an example for the situation you described where paintEvent is called while self.renderer doesn't exist yet?

My reading of @jrevans comments is that was his guess as to why the draw was there. That conditional can probably be removed.

I would like confirmation from @jrevans that this does not break any of their use cases.

I gave this a read over and it seems reasonable, but need sink more time into it. I am slightly worried about the MI re-ordering and making sure all of our classes are correctly cooperating when they need to be (one of the big changes between pyQt4 and pyQt5 is that their classes are now cooperating) so everything works with both qt4 and qt5.

attn @mfitzp (you clearly need a distraction from thesis writing 😈 )

@astrofrog Can you have a look at how this impacts glueviz?

@tacaswell - we don't yet use Qt5 in glueviz, so I'm not able to test this yet (unless this impacts Qt4 in some way?)

It does, qt4 is implemented as a shim on to of qt5.

On Sun, Aug 23, 2015, 11:27 AM Thomas Robitaille notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell - we don't yet use Qt5 in
glueviz, so I'm not able to test this yet (unless this impacts Qt4 in some
way?)

—
Reply to this email directly or view it on GitHub
#4972 (comment)
.

@tacaswell - ok, in that case I will test this out tomorrow with glueviz

@tacaswell - sorry for the delay. Unfortunately this does break glueviz:

Before (master):

After (this PR):

Any idea why this would have an impact on glueviz while the regular gui operates normally?

glueviz does some monkey patching to try and control the draw cadence (if I recall correctly). I suspect that someplace they are calling draw instead of draw_idle.

Strange, that shouldnt stop the Widget from painting. Anything hardcoded to the order of the base and Mixin-classes? I will probably look into installing glueviz also...

Yea, seems to be a problem with the python MRO. The glueviz problem is caused by making FigureCanvasQT the first class instead of FigureCanvasQTAggBase, although making the QObject the first class this is the preferred layout according to the PyQt documentation.

Reverted to original class order and rebased to current master.

@astrofrog @jrevans Can you both please check the impact of this branch on your applications?

@tacaswell @pwuertz - just confirming that this now works with glueviz