@anntzer : would you mind reviewing this one?
You have more experience with all the backends.
If I'm right, then draw() needs to call Refresh as it can be called by the user or from show(). On the other hand, if _isDrawn is False, then _OnPaint is calling draw to render the figure before painting. So at the moment Refresh is probably called too often.
Maybe it would be best to move the rendering into a separate method, e.g. _draw() or add an argument draw(refresh=True).
What do you think?

Tagged this as 3.1 for now. Please re-milestone if it needs to be backported.

I don't know the details of the wx drawing model, but for Qt we do call update() from draw() (and we do want fig.canvas.draw() to update the actual widget; that's a very common way of doing it), and http://doc.qt.io/qt-5/qwidget.html#update states that

This function does not cause an immediate repaint; instead it schedules a paint event for processing when Qt returns to the main event loop. This permits Qt to optimize for more speed and less flicker than a call to repaint() does.

Calling update() several times normally results in just one paintEvent() call.

so we're not overdoing things. How are things on wx's side?

@anntzer
Thanks a lot.
With the implementation as it is now, Refresh gets called from the EVT_PAINT handler, so I'm quite sure it's being painted twice if _isDrawn is False.
It's probably best to separate into the API function draw() and the internal rendering function _draw().

About the rubberband:
Currently the wx backends use the coloured box with 50% transparency, except on Mac OS due to the high dpi problems of the overlays.
RubberbandWx draws just a black border on Mac OS. I'm not sure about the NavigationToolbar2Wx
Of course, it would be possible to unify this and it would save a few lines. On the other hand, I like the semi-transparent blue box.
I will change to a dotted black line on Mac OS, though. That's probably more common than a solid blue or black line.

I will add notes about the API changes.
I thought about keeping retinaFix, but it would have to be moved to the canvas anyway.

I will come back to you when I've done the changes. (I need to push the commits to be able to test on Mac OS.)

retinaFix can be made private if it moves somewhere else, it's not as if it was ever intended as a public API anyways.

RubberbandWx draws just a black border on Mac OS.
I will change to a dotted black line on Mac OS, though. That's probably more common than a solid blue or black line.

If it can be a dashed line drawn with XOR -- that's ideal for making it visible against any backround. Now suer that that interacts with Overlay, though

@ChrisBarker-NOAA : thanks, but this does not seem to work. I think that this also did not work with ClientDC on Mac OS. The only way to get this behaviour would be to modify the canvas' bitmap, as it was done with the previous implementation of NavigationToolbar2Wx on Mac OS. This is something that I would like to avoid, though.

@ChrisBarker-NOAA : using anything than a simple drawing function would also cause problems on Retina displays, as the overlay background is scaled wrongly. I think that was the reason why overlays were not used on Mac OS previously.
With a simple nontransparent pen, this does not matter as the underlay does not shine through.

I just looked at some old overlay code of mine -- and indeed, not XOR. So what I did is draw a dashed black and white line:

            # a black and white line so you can see it over any color.
            dc.SetPen(wx.Pen('WHITE', 2))
            dc.SetBrush(wx.TRANSPARENT_BRUSH)
            dc.DrawLine(self.StartMove, pos)
            dc.SetPen(wx.Pen('BLACK', 2, wx.SHORT_DASH))
            dc.DrawLine(self.StartMove, pos)

Ah, thanks, that's probably the best option. I will take this one for Mac OS.

1. Zooming doesn't actually work once you release the mouse window (I get a gray background with nothing).
2. I got extremely confused at how to adapt mplcairo's wx integration to the new code (currently at https://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/wx.py, new version at https://github.com/anntzer/mplcairo/blob/wx/lib/mplcairo/wx.py, would appreciate if you could advise...
3. Should the definition of draw (which just calls _draw and Refresh) be moved to _FigureCanvasWxBase instead of being duplicated? Also perhaps rename _draw to something clearer, e.g. _update_bitmap or similar (if I understood its goal correctly).

@anntzer :
On what platform and backend does zooming fail?

Doesn't the supplied code for backend_wxcairo.py work?
Basically, the backend should render into self.bitmap, call Refresh and then _FigureCanvasWxBase._OnPaint will paint the bitmap to the screen, using BufferedPaintDC (which is the standard approach for double-buffering on wx).

    def _draw(self):
        width = int(self.figure.bbox.width)
        height = int(self.figure.bbox.height)
        surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, width, height)
        self._renderer.set_ctx_from_surface(surface)
        self._renderer.set_width_height(width, height)
        self.figure.draw(self._renderer)
        self.bitmap = wxcairo.BitmapFromImageSurface(surface)
        self._isDrawn = True

    def draw(self):
        """
        Render the figure and trigger a screen refresh.
        """
        self._draw()
        self.Refresh()

Unfortunately, draw can't be centralized at _FigureCanvasWxBase as then it would be called recursively from FigureCanvasAgg.draw as super().draw() resolves to _FigureCanvasWxBase.draw.
I think I commented some time ago that I don't like the implementation of FigureCanvasAgg.draw, as it is very intransparent what gets called...

On what platform and backend does zooming fail?

Fedora 28 (in case anyone wonders, my Arch machine is momentarily out of service...).

Doesn't the supplied code for backend_wxcairo.py work?

Actually a basic pop-up-a-plot work, it's animation that doesn't work (e.g. examples/animation/strip_chart.py), sorry, should have clarified that.

I can't (don't want to) use the codepath from backend_wxcairo because that disables blitting (you redraw the whole figure at every iteration). backend_wxcairo correctly inherits supports_blitting = False from backend_cairo (even though technically it's only gtk3cairo that doesn't support blitting (because it needs to draw directly on the X surface that gtk3 handles to it, whereas wxcairo could just stash the ImageSurface and blit normally from it), but that's an unrelated point...), but for mplcairo.wx I have no reason not to support blitting.

OK, so in mplcairo/wx.py just replace the gui_repaint call with a call to Refresh.
With backend_wxcairo the strip_chart animation seems to work for me on Ubuntu with Wayland.

I don't have a Fedora 28 VM. So I need some time to install one.

Replacing gui_repaint by Refresh works in the old codebase (before this PR). After this PR, if I replace gui_repaint by Refresh, it looks like something is preventing wx from running its event loop, unless I try to throw events at it: the plot stays blank (except for the axes), but if I click on the window, or on one of the buttons (let's say the mouse is initially out of the window) then I get one or two updates to the animation, which then freezes again, and so on.
(As a comparison, things work just fine on qt and tk, see https://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/qt.py and https://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/tk.py for how they are done.)

OK, maybe you can attach a script that uses mplcairo for animation. If I got it right, then I need to place the contents from mplcairo somewhere next to such a script and then just run the script, right? I could then try at least on Ubuntu with and without Wayland.
Or would I need some more complicated installation?

That behavior of Refresh() is expected:

https://wxpython.org/Phoenix/docs/html/wx.Window.html#wx.Window.Refresh

.Refesh() tell wx that the Window needs to be repainted, but it doesn't trigger a Paint event right then.

The usual way to get it done NOW is:

.Refresh()

followed by

.Update()

Update(self)
Calling this method immediately repaints the invalidated area of the window and all of its children recursively (this normally only happens when the flow of control returns to the event loop).

Notice that this function doesn’t invalidate any area of the window so nothing happens if nothing has been invalidated (i.e. marked as requiring a redraw). Use Refresh first if you want to immediately redraw the window unconditionally.

BTW, not sure if there is any point to using a BufferedDC. The idea behind double buffering is that you only repaint the screen once the drawing is complete on an offscreen buffer. But int his case, all the drawing is being done by AGG or Cairo to an offscreen buffer anyway, and the Draw() call is simply blitting that image to the Screen. -- no need to blit it to an offscreen buffer first, only to have to transfer it again.

NOTE: I'm working off ten year(or more!) old information here, back when I did a lot with wx drawing, (And BufferedDC didn't exist, so I wrote my own) but I suspect this part is the same.

Honestly, I have never used Update. Would be worth a try, but in my tests the Refresh with later repaint was enough. It could be that Update has the same problems as ClientDC, but that would need testing.

I would not expect BufferedDC to make an additional copy, as it just receives the buffer bitmap that was filled from AGG or Cairo: dc = wx.BufferedPaintDC(self, bmp).
The wx backend code is somewhat inefficient, as it makes copies of the bitmap on e.g. Windows, but that's a different story...

No, Update is not quite as aggressive as ClientDC about "NOW!" -- At least on OS-X, Client DC does it really right now, which may interfere with other drawing the OS is doing, whereas Update() puts it on the queue for the OS right away, but still lets the OS schedule it.

Due to this, on the past, on OS-X, using Refresh(), Update() and drawing in a PaintDC was not fast enough to follow the mouse...

As for the BufferedDC -- if it's not writing to a buffer, then what's the point ???

Note also that OS-X (and some others?) is already double-buffered by the OS anyway.

Maybe wx.Window.IsDoubleBuffered() would help here?

Hmm -- not sure the point of wxBufferedPaintDC if you aren't drawing to it anyway... (as opposed to simply doing a PaintDC.DrawBitmap() call, but it looks like you are right. I did see this note in the BufferedDC docs:

"Finally, please note that GTK+ 2.0 as well as OS X provide double buffering themselves natively. You can either use wx.Window.IsDoubleBuffered to determine whether you need to use buffering or not, or use wx.AutoBufferedPaintDC to avoid needless double buffering on the systems which already do it automatically."

(unrelated point: the timer bug was fixed in #12023, you can just rebase on top of it)

re-milestoned to 3.4 and removed the release critical label.

(Neither of my examples are using Wayland.)

@anntzer Thanks. Could you please check whether these are all gtk2 builds? From the recipe file, I would guess that the conda build is gtk2.

it's gtk2/wxWidgets 3.0.5 on fedora, gtk3/wxWidgets 3.0.4 on arch.

I have just spent too much time again with the ArchLinux2018.09 VM. The platform upgrade command pacman -Syu is broken. The guest extensions do not work properly. The python-wxpython package installs a wxPython for Python3.8 while the platform Python is 3.7.

I'm not willing to work on fixing platform problems after all the experiences with the Arch and Fedora VMs.
If nobody wants to debug on his platform(s), then I would suggest to close this PR and declare Wayland unsupported.

Is it possible to resurrect the non-Wayland parts of this PR? If not, what is the todo here?

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.