Update background in SpanSelector._press whenever useblit=True.

[anntzer]

Previously, the canvas would only get redrawn when useblit=True only if
span_stays is set. This causes e.g.

import sys
from PyQt5.QtWidgets import *
from matplotlib.backends.backend_qt5agg import *
from matplotlib.figure import Figure
from matplotlib.widgets import SpanSelector

class App(QMainWindow):
    def __init__(self):
        super().__init__()
        self.setCentralWidget(QWidget())
        layout = QVBoxLayout()
        self.centralWidget().setLayout(layout)
        self._fig = Figure()
        self._fig.subplots()
        layout.addWidget(FigureCanvas(self._fig))
        layout.addWidget(QPushButton("span", clicked=self._span_cb))

    def _span_cb(self):
        self._span = SpanSelector(
            self._fig.axes[0], print, "horizontal", useblit=True)

qapp = QApplication(sys.argv)
app = App()
app.show()
qapp.exec_()

to use an incorrect background(?) when clicking and starting a span for
the first time.

To be honest I'm not sure this is exactly the correct solution, although
it does fix the issue...

PR Summary

PR Checklist

• Has Pytest style unit tests
• Code is PEP 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[lkjell]

That is because self.background is None before any draw is being made. The red patch you see is actually the dragging line because background (None) is not restored. The "correct" way perhaps to fix this is to add a draw() and copy the background in onmove event. This is because the benefit of blitting is used here. Adding a moving flag to just update the background before blitting. That is what I do in my implementation.

However, with the current implementation I would suggest to add self.update_background(None) in self.update() for the implementation of Spanselector.

    def update(self):
        """draw using newfangled blit or oldfangled draw depending on
        useblit

        """
        if not self.ax.get_visible():
            return False

        if self.useblit:
            if self.background is not None:
                self.canvas.restore_region(self.background)
            else:
                self.update_background(None)  # ADD ME
            for artist in self.artists:
                self.ax.draw_artist(artist)

            self.canvas.blit(self.ax.bbox)

        else:
            self.canvas.draw_idle()
        return False 

[tacaswell]

I think @lkjell 's suggestion is more correct. The background is updated automatically in a callback on draw_event and in the example @anntzer posted the the figure is never drawn before the first time the span selector is used. If span_stays is False, the selector never leaves non animated=True artists in the draw tree so it should be safe to just grab the current state.

Might be worth adding a check if the figure is stale and forcing a redraw and if not just grabbing the background.

[anntzer]

I believe that as suggested by @tacaswell that checking for staleness is probably the correct thing to do.

[lkjell]

Actually you cannot check for staleness because the figure will mostly be staled anyway. The current implementation is very clever and too complex if you do not understand what should be done thus making it very prone for bugs. The problem occur because of blitting. And blitting is only useful during move event. For blitting to function properly it needs a clean background. Thus to make it sane and guard against retarded code that change the background one should before a move event force a redraw with span invisible, copy the background and turn span visible on. The question is where to put this redraw on. Should it be in press event or move event. If you put it in press then it is a bit redundant because you do not need it when there is no movement.

Putting it in move event will let you have a move flag to fix some inconsistency with span_stay is visible but the next time you only click and do not move. The old span will still be visible but select callback gives different number.

I only suggest to update the background in update because of the fancy implementation but it does not guard against people make strange code like change the background or deactivate spanselector do something with the figure without redrawing and activate the spanselector.

[tacaswell]

Actually you cannot check for staleness because the figure will mostly be staled anyway.

Why is it most likely stale? Changes to animated artists should not be marking the figure as stale and for people in IPython should end up with a non-stale figure everytime they get the prompt back.

We should be biased towards putting logic in press over move and move is a hot-path. The only moves we care about here are between a press and a release so it is safe to put all of the setup logic in 'press'.

[lkjell]

If I add a print(self.ax.stale)right before return False in _press it will result in True all the time.
And if I put it above everything else it still result in True. I really do not know where else you want to put the stale test on.

Anyway, I do believe that my initial suggestion on just updating the background in def update would be the best solution for now. I just tested with RectangleSelector and found that you have the same problem. Therefore I assume that all widgets that are based on _SelectorWidget would have the same problem.

[lkjell]

Perhaps a new bug. Tested in ipython and python. Jupyter does not seem to have interactive.
onpress will always result in axes being stale. The only time it returns false is zooming and panning.

import matplotlib.pyplot as plt

plt.plot(range(10))
axes = plt.gca()
canvas = axes.figure.canvas

cid = canvas.mpl_connect("button_press_event", lambda x: print(axes.stale))
cid = canvas.mpl_connect("button_release_event", lambda x: canvas.draw())
plt.show()

[QuLogic]

With update_background being nicer from #17480, maybe it can be used here?

[anntzer]

TBH I sort of lost track of the discussion above. I can confirm that this PR still works and still fixes the originally mentionned issue (which is still present), but if you have a better solution, go for it :)