Skip to content

Support blitting in webagg backend #19059

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 15, 2020
Merged

Support blitting in webagg backend #19059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78c182d
support blitting in webagg backend
ianhi Dec 3, 2020
2fbbec9
add blitting to nbagg UAT notebook + correct the numbering of stoppin…
ianhi Dec 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions lib/matplotlib/backends/backend_webagg_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def _handle_key(key):


class FigureCanvasWebAggCore(backend_agg.FigureCanvasAgg):
supports_blit = False
supports_blit = True

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand Down Expand Up @@ -153,6 +153,10 @@ def draw(self):
finally:
self.manager.refresh_all() # Swap the frames.

def blit(self, bbox=None):
self._png_is_old = True
self.manager.refresh_all()
Comment on lines +156 to +158
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this from #4290 so I assume it was done correctly by @tacaswell, but I find it odd that this accepts a bbox argument but doesn't do anything with it. Is that a mistake?

Copy link
Member

Choose a reason for hiding this comment

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

That's to match the super class; it should blit only the section in the bbox if it's not None. Just re-blitting everything is fine, but perhaps a bit under-optimized. I think you would have to add to the current wire protocol to be able to do bbox'd blitting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense. Should a docstring be added explaining something to that effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I see how to implement more selective blitting for the ipympl frontend, but would/can the backend Agg renderer also needing blitting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other consideration is that if any part of the image is transparent it will be forced to be a full image. That could also be optimized a bit with frontend blitting. Discussed far in the past here: #5419 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I'm sure it could be possible, but would require sending a different message probably. Doesn't have to be done in this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed I'm happy with this PR as is. Better blitting seems like a good 2021 goal.


def draw_idle(self):
self.send_event("draw")

Expand Down Expand Up @@ -189,18 +193,14 @@ def get_diff_image(self):
output = buff
else:
self.set_image_mode('diff')
last_buffer = (np.frombuffer(self._last_renderer.buffer_rgba(),
dtype=np.uint32)
.reshape((renderer.height, renderer.width)))
diff = buff != last_buffer
diff = buff != self._last_buff
output = np.where(diff, buff, 0)

buf = BytesIO()
Copy link

@thomasaarholt thomasaarholt Dec 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I notice that there are both buff and buf in this code (I realise you didn't introduce or touch the last one). Do you think you could attempt to clear up what the difference between them is, and perhaps make the variable names self explanatory?

Making it clearer now will make it easier for the next developer who looks at this to understand what's going on :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't left a comment because I'm not 100% sure this is the reason. But here's the idea of what I might leave:

We differentiate between buff and buf because we cannot directly return buff as that is a reference to a location in memory that may change later.

There may also be something happening with the save where the buffer gets converted from whatever the renderer uses into a png buffer.

Copy link
Member

Choose a reason for hiding this comment

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

See #19117.

data = output.view(dtype=np.uint8).reshape((*output.shape, 4))
Image.fromarray(data).save(buf, format="png")
# Swap the renderer frames
self._renderer, self._last_renderer = (
self._last_renderer, renderer)
# store the current buffer so we can compute the next diff
np.copyto(self._last_buff, buff)
self._force_full = False
self._png_is_old = False
return buf.getvalue()
Expand All @@ -220,9 +220,10 @@ def get_renderer(self, cleared=None):
if need_new_renderer:
self._renderer = backend_agg.RendererAgg(
w, h, self.figure.dpi)
self._last_renderer = backend_agg.RendererAgg(
w, h, self.figure.dpi)
self._lastKey = key
self._last_buff = np.copy(np.frombuffer(
self._renderer.buffer_rgba(), dtype=np.uint32
).reshape((self._renderer.height, self._renderer.width)))

elif cleared:
self._renderer.clear()
Expand Down
Loading