Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qt backend draw_idle doesn't work #4944

Merged
merged 1 commit into from Aug 20, 2015
Merged

Conversation

jrevans
Copy link

@jrevans jrevans commented Aug 17, 2015

Restored 'draw_idle' method to allow for mpl callbacks to finish processing before a Qt draw happens.

This addresses an issue in #4897.

@tacaswell tacaswell added this to the next point release milestone Aug 17, 2015
@tacaswell
Copy link
Member

attn @pwuertz

@tacaswell
Copy link
Member

attn @mfitzp

@tacaswell tacaswell modified the milestone: next point release Aug 18, 2015
@pwuertz
Copy link
Contributor

pwuertz commented Aug 18, 2015

I don't see why this is should be necessary. Calling update() is designed to do the exact same thing, it posts a paintEvent to the event loop and returns immediately to the event processing code. If the render thread determines that it has time to draw another frame, it draws another frame.

Is there an example where the mpl callbacks do not get processed in time and are causing an event pile up? After we eliminated all synchronous draw calls I haven't seen this behaviour any more.

@mfitzp
Copy link
Member

mfitzp commented Aug 19, 2015

I might be missing something here but the patch:

def draw_idle(self):
    d = self._idle
    self._idle = False

    def idle_draw(*args):
        try:
            self.draw()
        finally:
            self._idle = True
    if d:
        QtCore.QTimer.singleShot(0, idle_draw)

...replaces the call to self.update() (the Qt function) with one to self.draw() which is implemented on FigureCanvasBase as a no-op:

class FigureCanvasBase(object):
    def draw(self, *args, **kwargs):
        """
        Render the :class:`~matplotlib.figure.Figure`
        """
        pass

...this looks to me as if it would just block the update() (Qt refresh) from occurring. Is there are test-case for the issue this is trying to resolve (the need for a longer delay from zoom, pan, lasso)?

I'm wondering if postponing the call to update() until idle would accomplish the same thing:

def draw_idle(self):
    if self._idle:
        self.update()

@pwuertz
Copy link
Contributor

pwuertz commented Aug 19, 2015

@mfitzp The draw() call is reimplemented in the qt backend and will force an agg redraw + emit a qt update. This however eliminates the recent optimization where the agg redraw is done in paintEvent.

As discussed in another issue, there is a problem when using interactive mode where stale callbacks are emitting draw_idle calls from within a agg draw call. As I understand it, this is a bug in matplotlib. The self._idle flag in this pull request acts as a workaround for that.

@mfitzp
Copy link
Member

mfitzp commented Aug 19, 2015

@pwuertz ah, thanks. I was looking up the inheritance rather than down:

class FigureCanvasQTAggBase(object):            
    def draw(self):
        FigureCanvasAgg.draw(self)
        self.update()    

I was confused as the self._idle flag was already present before this commit (but is apparently unused).

@jrevans is the purpose of this to postpone Qt redraw? Why use self.draw() rather than self.update()?

@tacaswell
Copy link
Member

You need to consider this with the qt.paintEvent pr for it to make sense.

The _idle flash was there for a long time and the removed.

I will have an alternate pr to this pair in soon.

On Wed, Aug 19, 2015, 7:46 AM Martin Fitzpatrick notifications@github.com
wrote:

@pwuertz https://github.com/pwuertz ah, thanks. I was looking up the
inheritance rather than down:

class FigureCanvasQTAggBase(object):

def draw(self):
FigureCanvasAgg.draw(self)
self.update()

I was confused as the self._idle flag was already present before this
commit (but is apparently unused).

@jrevans https://github.com/jrevans is the purpose of this to postpone
Qt redraw? Why use self.draw() rather than self.update()?


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

tacaswell added a commit that referenced this pull request Aug 20, 2015
FIX: revert changes to qt draw_idle
@tacaswell tacaswell merged commit 5a23a21 into matplotlib:master Aug 20, 2015
@tacaswell
Copy link
Member

tightly linked with #4943

Please move this discussion to #4962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants