This was changed in 0870c50 in address performance in qt5. Can you please merge this with #4944 as they collectively reverting #4612 ?

Can you please provide an example of what these two PRs are fixing?

At least merging both of these does not re-introduce the previous bug.

The example in #4947 is sufficient to show the extra draws (as something was masking the restore_region failure).

I suspect that what we want to do here is to defer the call to draw until the paint event so draw_idle should just set a flag that paintEvent checks to see if it should trigger the re-draw or not. I think that will get the best of both solutions, but I am too tired right now to fully trace through the Qt draw logic.

attn @pwuertz

I don't really understand what this change does, maybe because I don't have any in-deep knowledge about the Agg renderer. For me it looks like the agg image is drawn exactly once in the lifetime of the canvas, and after self.renderer exists it is never updated again.. at least not in paintEvent. However, paintEvent is the only routine where the agg draw should be happening.

@tacaswell While paintEvent needs to draw the latest figure-image onto the widget surface, its true that the agg canvas might not need a redraw each time. I don't see a lot of scenarios for this though, other than the use of the rubberband tool. Who else but draw_idle() is calling the widgets update() routine?

I ran the example from #4947 on current master. On my system everything seems to work as expected. There are no duplicate red ellipses and the red ellipse is removed correctly after the second break. Note that the draw_artist/restore_region calls are not required at the moment since blit->redraw->paintEvent will redraw the figure anyways.

The ion() mode however causes lots of extra draws. For instance, there is a looped draw_idle() call that is caused from within the figure.draw(). Backtrace:

  /home/pwuertz/Software/matplotlib/lib/matplotlib/backends/backend_qt5agg.py(74)paintEvent()
-> FigureCanvasAgg.draw(self)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/backends/backend_agg.py(474)draw()
-> self.figure.draw(self.renderer)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/artist.py(60)draw_wrapper()
-> draw(artist, renderer, *args, **kwargs)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/figure.py(1134)draw()
-> func(*args)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/artist.py(60)draw_wrapper()
-> draw(artist, renderer, *args, **kwargs)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/axes/_base.py(2072)draw()
-> self.apply_aspect()
  /home/pwuertz/Software/matplotlib/lib/matplotlib/axes/_base.py(1201)apply_aspect()
-> self.set_position(position, which='active')
  /home/pwuertz/Software/matplotlib/lib/matplotlib/axes/_base.py(759)set_position()
-> self.stale = True
  /home/pwuertz/Software/matplotlib/lib/matplotlib/artist.py(266)stale()
-> self.stale_callback(self, val)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/figure.py(56)_stale_figure_callback()
-> self.figure.stale = val
  /home/pwuertz/Software/matplotlib/lib/matplotlib/artist.py(266)stale()
-> self.stale_callback(self, val)
  /home/pwuertz/Software/matplotlib/lib/matplotlib/pyplot.py(559)_auto_draw_if_interactive()
-> fig.canvas.draw_idle()
> /home/pwuertz/Software/matplotlib/lib/matplotlib/backends/backend_qt5.py(419)draw_idle()
-> self.update()

I don't see a reason why figure.draw should be emitting another draw_idle call.

@jrevans The agg.draw in paintEvent is not intended as a cover up for missing canvas.draw calls. It is actually the other way around. The superfluous agg.draw call is the one in canvas.draw, but we cannot get rid of it since other code might rely on the figure being drawn immediately for having some figure elements initialized in the process.
In a GUI environment canvas.draw should never be invoked directly. If the GUI needs updating draw_idle is called for scheduling a redraw, while the actual drawing is happening only if the render process is able to display another frame (in paintEvent).

The stale callbacks causing draw_idle loops are currently under investigation? Is this related to #4738?

@pwuertz You are missing the blit workflow. We have 3 layers of state: scene graph -> renderer buffer -> Qt screen buffer and any changes on the left need to propagate right. The standard way is:

canvas.draw()  # moves state change from scene graph -> renderer buffer
Qt.update() -> canvas.paintEvent() # moves state from Agg buffer -> Qt screen buffer

What @pwuertz change did was to merge the draw and update steps to prevent a case that looks like

canvas.draw()
canvas.draw()
canvas.draw()
Qt.update() -> canvas.paintEvent()

where the first two draws are just wasted cycles as only the last draw matters.

The problem with this is that the blit work flow (which is to avoid draw calls) is

canvas.restore_region(bg)
canvas.draw_artist(art)
Qt.update() -> canvas.paintEvent()

If paintEvent is calling draw the entire blit process is dead (run the example under tkagg, it is 3x as fast) so an un-guarded (by a stale flag of some sort) can not stay in paintEvent. If that animation example is working correctly the total number of calls to draw should be 1.

The cascading draw_idle calls are because stale state propagates all the way up the draw tree, even if the parent is already stale, to work around a limitation of the plain python repl, that @mdehoon pointed out. They should be harmless as the point of draw_idle is to allow multiple calls to accumulate. The draw-during-draw can be prevented by being smarter in all of the setters to only trigger if they in fact changed, I was hoping to punt that work to be merged with the Traitlets work that is in progress.

That is all I have time to type as I need to catch a train to work 😄 .

@tacaswell I didn't know that there was something like a general blit/draw_artist/restore workflow, and if I wrap my head around it I'm not really sure if it is such a useful thing to do. It seems like a workaround for not having a scene graph on the backend-side.
Of course the qt backend currently uses an Agg buffer you could manipulate directly in order to get some speed-up for specific situations, but the Agg buffer is an implementation detail of the qt backend that you cannot rely on. For example, the backend might as well just render the primitives emitted from the scene graph directly and skip the additional buffer altogether. Saving/restoring a region of pixels could be difficult and inefficient then, not to mention that modern frameworks don't allow synchronous drawing commands from the main thread at all.

For the current situation I added a flag for determining if paintEvent needs to update the agg buffer or not. The buffer is updated when draw_idle triggers the paintEvent and the buffer is considered valid after using draw and when using blit.

Another feature that is important to keep track of here is that artists can have an animation flag set on them that has the effect of

• excluding them from a non-save call to draw
• prevents their stale state from propagating up to their parents

This property should probably be rename from 'animated' -> 'blitted'

On current master if you set animated=True on the red ellipse in #4947 then it never shows up (because it gets blitted in and then a draw is called which re-renders the canvas excluding the animated ellipse).

I should have made the #4943 PR and #4944 PR the same one, I saw them as two separate issues at the time, but now I see that they are really closely tied.

It seems that some clarification needs to be given. The current design of matplotlib (up to this point) has been that when matplotlib deems a figure needs to be drawn (to the screen, to a file, whatever) a call to 'FigureCanvas.draw' is made. This is where the magic happens. Since the internal design of matplotlib is really geared towards an Agg-like renderer, this is typically where the Agg buffers get rendered to.

In the case of Qt it will draw a widget and its contents to the screen in a 'paintEvent'. This can happen for any number of reasons. Reasons can include showing/hiding the widget, resizing the widget (which happens at least one time during widget creation), etc. GUIs (like Qt) typically only want to draw the least amount possible when they can for speed purposes, so ideally they will only redraw the areas of a window that need it. This is where Qt can decide when it calls 'paintEvent' based on their own internal decision making processes. If you are using Qt, you are typically using it because it has these types of optimizations.

When rendering any matplotlib figure to the screen, the slowest part is the rendering of the artists (to the Agg buffers). This is not something that you want happening every time Qt issues a 'paintEvent'. The original designed behavior relies on the blitting functionality of transferring the contents of the Agg buffers to the widget. A process which is fairly fast and efficient. This is using the same idea behind what makes the animation system, lassoing, label dragging, etc work. By updating the Agg buffers during every 'paintEvent' will not only make the Qt GUIs very slow, but will make the above mentioned features not work. We should not short-circuit the matplotlib design of the artists being rendered when matplotlib deems necessary, which it enacts by calling 'FigureCanvas.draw'

The changes introduced in #4612 appear to be addressing the Qt bug identified in #4897. Those changes, when applied do not redraw the canvas properly when dragging during a pan when there is alot of artists present on the canvas until after the mouse is released.

The reason for the timer in draw_idle, is in fact to delay a teeny bit longer so that matplotlib can finish processing its events before the redraw actually happens, otherwise when you select a region for zooming or use the lasso and release the mouse button, they will not disappear. Ideally an optimized mechanism for matplotlib signal processing, where you could 'flush' the signals might be an alternative solution, but the timer delay was much easier and less invasive of a fix.

I also need to mention that in our use cases we can have some plots that have thousands of data points and potentially thousands of artists on a canvas at a time. When you are dealing with the more complicated plots, then little things can sometime turn into bigger things.

@pwuertz That bilt/draw/restore workflow has been is place for longer than I have been working on the project and is absolutely essential to making animations/complex figures go fast. I consider this ability to be one of the core features of mpl.

@tacasawell, it has been around for awhile, but it was implemented in the
days prior to the animation module and the timer system. Perhaps someone
else can chime in on its history (it predates me as well), but it never
seemed to me to be properly implemented. I note several limitations of it
in my book and downplay it importance/impact (going so far as to not
include any examples).

Some of the blitting bugs have been fixed in recent weeks, so maybe I'll
revise its importance in a new edition of my book (if that ever happens).
But three major limitations still exists. 1) zorder is not respected (we
don't keep a buffer for each contiguous static portions of artists sorted
by zorder). This is a huge problem with things like legends and annotations
in an animation. 2) I have yet to see panning/zooming always work properly
when blitting (supposedly, it should work, but...). 3) You can't do
blitting with multiple animation objects in play (not all static artists
may have been drawn by the time one of the animation objects grabs a
buffer).

On Wed, Aug 19, 2015 at 1:53 PM, Thomas A Caswell notifications@github.com
wrote:

@pwuertz https://github.com/pwuertz That bilt/draw/restore workflow has
been is place for longer than I have been working on the project and is
absolutely essential to making animations/complex figures go fast. I
consider this ability to be one of the core features of mpl.

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

I never claimed it was easy, just that it was very important where it is used 😜

To get blitting to work correctly with pan/zoom you need to hook up a callback to the Axes/Figure draw_event which tells it to re-grab the background (which of course relies on the blitted artists having 'animation=True' set so that they don't get captured in the draw).  

@blink1073 has a 'blit manager' which can take care of much of this (and the animation framework does take care of this for the most part).  

I have also seen demos from @joferkington that are amazingly responsive, I think that glueviz relies on blitting (and obviously the jpl folks rely on it).

On my internal to-do list is to make using blitting much easier. I have notions that we can use this blitting + stale information to do auto-blitting

@tacaswell The problems concerning the blitting solution listed by @WeatherGod are what I was thinking about. Efficient segmentation of the scene graph, setting up and maintaining multiple agg buffers and figuring out the order in which to blend them is a tough job. For such tasks there are already well optimized solutions like QtQuick. But I see the point in not having to completely redesign the matplotlib scenegraph-to-backend framework, despite the idea of letting the backend do all drawing optimizations - possibly using multiple threads and hardware acceleration.

After sending us off topic, I would like to keep this discussion on the
topic of fixing what used to work and stay away from the rabbit hole of my
possibly ill thought out future plans.

On Thu, Aug 20, 2015 at 9:45 AM Peter Würtz notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell The problems concerning the
blitting solution listed by @WeatherGod https://github.com/WeatherGod
are what I was thinking about. Efficient segmentation of the scene graph,
setting up and maintaining multiple agg buffers and figuring out the order
in which to blend them is a tough job. For such tasks there are already
well optimized solutions like QtQuick. But I see the point in not having to
completely redesign the matplotlib scenegraph-to-backend framework, despite
the idea of letting the backend do all drawing optimizations - possibly
using multiple threads and hardware acceleration.

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

Keep in mind, one of the main reasons why we don't normally delegate much
to the backends is because 1) AGG has been producing better quality results
for a long time, and 2) using AGG produces consistent results across all
the backends. I have a cache of emails conversations between John Hunter
and Perry Greenfield from about 14 years ago when matplotlib was in its
infancy and Chaco was king. One of the most important design requirements
was to work with many different GUIs and to have identical results in every
one of them. Not Chaco or any of the other tools could do that (and most of
them still can't).

As for hardware accelerations, at the SciPy 2015 conference, there were
some very exciting conversations with the VisPy people to put together a
roadmap to get matplotlib and vispy working together (essentially, a
vispy-based backend). Very exciting stuff, but will take quite some time,
though.

On Thu, Aug 20, 2015 at 9:45 AM, Peter Würtz notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell The problems concerning the
blitting solution listed by @WeatherGod https://github.com/WeatherGod
are what I was thinking about. Efficient segmentation of the scene graph,
setting up and maintaining multiple agg buffers and figuring out the order
in which to blend them is a tough job. For such tasks there are already
well optimized solutions like QtQuick. But I see the point in not having to
completely redesign the matplotlib scenegraph-to-backend framework, despite
the idea of letting the backend do all drawing optimizations - possibly
using multiple threads and hardware acceleration.

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

The near term plan for this is that this two PRs clearly fix a problem so I am going to merge them so master is less broken, we can then work on improving that state in #4962

Just found a possible problem in moving the agg draw into paintEvent. I'm closing #4962, please continue on #4972