Skip to content

Matplotlib blitting in Jupyter Notebooks (issue #4288) #9240

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

Closed
wants to merge 5 commits into from

Conversation

mbewley
Copy link

@mbewley mbewley commented Sep 27, 2017

PR Summary

Blitting is not currently supported by matplotlib when used in Jupyter notebooks (with the magic %matplotlib notebook).
@blink1073 wrote some code to make it work some time back, but didn't merge it in. I've taken his changes (he said he wasn't interested in doing it himself), and applied them to the latest version of matplotlib.

This is discussed in issue #4288. I was asked to do the pull request - first time contributor to matplotlib. I've tested it with a minimal example Jupyter notebook, but am unsure how to test whether blitting works in notebooks in scripted unit tests.

@jklymak
Copy link
Member

jklymak commented Sep 27, 2017

Hi, I'm unsure as well how to do a scripted notebook test as well. However, to start you could post your test code that can be pasted into a reviewers notebook.

@blink1073
Copy link
Member

There is nbval, but it only works for static notebooks. There is a UAT notebook in this repo for live testing of the notebook backend.

@tacaswell
Copy link
Member

@mbewley Thanks for taking this on! If you have any questions (code or process) please do not hesitate to ask! You will probably get faster answers asking publically (here or on https://gitter.im/matplotlib/matplotlib ) but if there is anything you do not feel comfortable asking publicly email me.

@mbewley
Copy link
Author

mbewley commented Sep 27, 2017

Here's the minimal example that works inside a notebook... just click on the generated 49 MP image, and it will rapidly toggle a little red/green box in the top left.

%matplotlib notebook
import numpy as np
import matplotlib.pyplot as plt
import matplotlib.patches as mpatches
import time


IMSIZE = 7000
RED = (1,0,0,1)
GREEN = (0,1,0,1)

class ExampleToggle:
    
    def __init__(self):
        plt.ioff()
        self.fig = plt.figure()
        self.ax = self.fig.add_subplot(111)
        self.canvas = self.ax.figure.canvas

        self.im = np.random.randint(0, 255, [IMSIZE,IMSIZE, 3]).astype('uint8')
        self.tmp_colour = RED
        self.rect = mpatches.Rectangle([0, 0], 200, 200, edgecolor='yellow', facecolor=self.tmp_colour, linewidth=0.5, animated=True)
        self.ax.add_patch(self.rect)
        
        plt.show()
        self.canvas.mpl_connect('button_press_event', self.clicked)
        
        self.ax.imshow(self.im)
        self.canvas.draw()
        self.background = self.canvas.copy_from_bbox(self.ax.bbox)    

    def clicked(self, event):
        if self.tmp_colour == RED:
            self.tmp_colour = GREEN
        elif self.tmp_colour == GREEN:
            self.tmp_colour = RED
        self.rect.set_facecolor(self.tmp_colour)
        self.canvas.restore_region(self.background)
        self.ax.draw_artist(self.rect)
        self.canvas.blit(self.ax.bbox)
et = ExampleToggle()

@pelson
Copy link
Member

pelson commented Oct 11, 2017

Looks like a sensible change (untested). To be clear, the blitting you are adding is on the server side - you aren't getting any blitting through the javascript and into the notebook. That's fine (and a hard problem to solve), but we should be careful not to call it full-blown blitting in the nbagg backend.

I was asked to do the pull request - first time contributor to matplotlib

Wow! Really impressive stuff - there is a lot of connective tissue you've had to get your head around, and you've done a great job of it as far as I can see! Keep up the awesome work - looking forward to seeing your future mpl contributions 👍

@mbewley
Copy link
Author

mbewley commented Oct 12, 2017

Ha, the "connective tissue" was all blink1073 - I just managed to copy/paste correctly (and do a neat minimal example). Yes, the server side blitting works perfectly on local host for fast access. It also works quite quickly on EC2 (tested at work where we have a pretty fast connection), but chews up monstrous amounts of bandwidth - so maybe should come with some sort of warning?

@dstansby dstansby added this to the v2.2 milestone Oct 19, 2017
@mbewley
Copy link
Author

mbewley commented Oct 19, 2017

Hey guys, is there anything we need to do to make sure this gets into 2.2? (it was slated for 2.1, but got bumped).

@dopplershift dopplershift added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Oct 20, 2017
@dopplershift
Copy link
Contributor

I labelled it release critical so that we're sure to take a look at it before the next feature release.

@mbewley
Copy link
Author

mbewley commented Oct 20, 2017

Thanks!

@tacaswell tacaswell closed this Jan 3, 2018
@tacaswell tacaswell reopened this Jan 3, 2018
@tacaswell
Copy link
Member

power cycled to re-trigger CI.

@tacaswell
Copy link
Member

Do you see the issues with #4290 with this patch?

Can you copy (or re-write) the UAT in #4290 ?

@tacaswell
Copy link
Member

Or put your example in the UAT notebook.

@mbewley
Copy link
Author

mbewley commented Jan 4, 2018

Hmm, I didn't see those issues. I've now:

  • Rebased on the latest merge in matplotlib, and checked it works in my use case.
  • Copied in the UAT notebook from ENH : Nbagg blit #4290 to the same location in my pull request (only change is letting jupyter upgrade the notebook format).
  • Ran the notebook. Tests seem to work fine, but exhibit the same flickering issue as your previous ones.

Anything else I need to do?

@mbewley mbewley mentioned this pull request Jan 4, 2018
@mbewley
Copy link
Author

mbewley commented Jan 4, 2018

That's odd - careful not to get pull requests mixed up - I got a bit confused at first. This one on blitting we're currently trying to get in is #9240, the old one on blitting from a couple of years ago is just the numbers rearranged - #4290.

@mbewley
Copy link
Author

mbewley commented Jan 17, 2018

Hey, any feedback on the updates?

@tacaswell
Copy link
Member

Can you update the UAT notebook as part of this PR?

Any theories on the flickering?

@mbewley
Copy link
Author

mbewley commented Jan 24, 2018

What did you want updated on the UAT notebook? I added it into the repo and checked it ran - the tests all still seemed sensible.
In terms of the flickering, I don't know the matplotlib codebase well at all, but it seems to be a pixel width on the right and bottom edges of the axis (when you have xticks/yticks set to none). Is there some inconsistency between the area that gets saved/restored as the background image, and the border/tickmarks of an axis?

@tacaswell
Copy link
Member

@mbewley just for the changes to be check-in as part of this PR so that the next person to work on this can run them.

@mbewley
Copy link
Author

mbewley commented Jan 24, 2018

Ah, sorry, thought I'd done that - forgot to push. Will do...

@jklymak
Copy link
Member

jklymak commented Jan 28, 2018

oops, looks like you accidentally rebased in a bunch of other commits...

@mbewley
Copy link
Author

mbewley commented Jan 28, 2018

Hmm, I was trying to update to the latest head on master, as it was getting pretty old. What's the best way to fix it?

@jklymak
Copy link
Member

jklymak commented Jan 29, 2018

To rebase, I always squash my own commits

git rebase -i HEAD~X  # where X is the last commit *not* made by me

Then I rebase with master:

git fetch upstream master
git rebase upstream/master

See:
https://matplotlib.org/devel/gitwash/index.html

When I've gotten in the state that you are in, I copy the files I edited somewhere else and then in my branch:

git reset --hard upstream/master

and then re-add the changes.

@mbewley
Copy link
Author

mbewley commented Jan 29, 2018

Ok, I thought I followed this: https://matplotlib.org/devel/gitwash/development_workflow.html#development-workflow ("Rebasing on Trunk"), but must've stuffed it up. Will fix and push again.

@jklymak jklymak added the stale label Feb 10, 2019
@jklymak jklymak modified the milestones: v3.1.0, unassigned Feb 10, 2019
@astrofrog
Copy link
Contributor

While investigating this I ran into #13971, which may be related - essentially any mouse click on the figure will result in a draw, which might interfere with the use of blit locally. I think it would be worth solving #13971 first just in case.

@@ -215,6 +223,11 @@ def get_diff_image(self):
# Swap the renderer frames
self._renderer, self._last_renderer = (
self._last_renderer, renderer)

self.figure._cachedRenderer = self._renderer
for ax in self.figure.axes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't cache the renderer on the axes anymore, so these two lines should go away

@tacaswell tacaswell modified the milestones: unassigned, v3.2.0 Apr 16, 2019
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 16, 2019
@jklymak jklymak removed the stale label Jul 16, 2019
@jklymak
Copy link
Member

jklymak commented Jul 16, 2019

ping @mbewley - sometimes takes a while to get things through matplotlib - hopefully the process hasn't scared you off!

@tacaswell
Copy link
Member

This still does not deal with the double-buffering correctly. For example in

%matplotlib notebook
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation

fig, ax = plt.subplots()
xdata, ydata = [], []
ln, = plt.plot([], [], 'ro')

def init():
    ax.set_xlim(0, 2*np.pi)
    ax.set_ylim(-1, 1)
    return ln,

def update(frame):
    xdata.append(frame)
    ydata.append(np.sin(frame))
    ln.set_data(xdata, ydata)
    return ln,

ani = FuncAnimation(fig, update, frames=np.linspace(0, 2*np.pi, 128),
                    init_func=init, blit=True)
plt.show()

every other frame has the incorrect tick labels. I think instead of using two render objects, we need a double-buffered renderer sub-class.

@tacaswell tacaswell modified the milestones: v3.2.0, v3.3.0 Sep 5, 2019
@tacaswell tacaswell removed the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 30, 2020
@tacaswell tacaswell modified the milestones: v3.3.0, v3.4.0 Apr 30, 2020
@jklymak jklymak marked this pull request as draft September 12, 2020 19:54
ianhi added a commit to ianhi/matplotlib that referenced this pull request Dec 3, 2020
Replaced the second Agg renderer by storing the previous buffer in a private attribute. Removing the second renderer eliminates the the flickering issues noted in: matplotlib#9240 (comment)

Also, did not need to implement copy_from_bbox as that is inherited from backend_agg.FigureCanvasAgg
ianhi added a commit to ianhi/matplotlib that referenced this pull request Dec 3, 2020
Replaced the second Agg renderer by storing the previous buffer in a private attribute. Removing the second renderer eliminates the the flickering issues noted in: matplotlib#9240 (comment)

Also, did not need to implement copy_from_bbox as that is inherited from backend_agg.FigureCanvasAgg
@ianhi ianhi mentioned this pull request Dec 3, 2020
4 tasks
ianhi added a commit to ianhi/matplotlib that referenced this pull request Dec 3, 2020
…g figure

taking the example from matplotlib#9240

Co-Authored-By: Thomas A Caswell <tcaswell@gmail.com>
ianhi added a commit to ianhi/matplotlib that referenced this pull request Dec 3, 2020
…g figure

taking the example from matplotlib#9240

Co-Authored-By: Thomas A Caswell <tcaswell@gmail.com>
ianhi added a commit to ianhi/matplotlib that referenced this pull request Dec 5, 2020
…g figure

taking the example from matplotlib#9240

Co-Authored-By: Thomas A Caswell <tcaswell@gmail.com>
@QuLogic QuLogic modified the milestones: v3.4.0, unassigned Jan 21, 2021
@jklymak
Copy link
Member

jklymak commented Jun 14, 2021

I'll closed as abandoned; thanks for the PR, and feel free to re-open if you want to come back to this!

@jklymak jklymak closed this Jun 14, 2021
@ianhi
Copy link
Contributor

ianhi commented Jun 28, 2021

I'll closed as abandoned; thanks for the PR, and feel free to re-open if you want to come back to this!

For any future finders of this PR: This was subsumed and then superceded by #19059

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

Successfully merging this pull request may close these issues.