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

Fixed the differencing of images for the webagg/nbagg backends. #3567

Merged
merged 1 commit into from
Sep 27, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions lib/matplotlib/backends/backend_nbagg.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,20 @@ def __call__(self, block=None):
if not managers:
return

interactive = is_interactive()

for manager in managers:
manager.show()

if not is_interactive() and manager in Gcf._activeQue:
Gcf._activeQue.remove(manager)
# plt.figure adds an event which puts the figure in focus
# in the activeQue. Disable this behaviour, as it results in
# figures being put as the active figure after they have been
# shown, even in non-interactive mode.
if hasattr(manager, '_cidgcf'):
manager.canvas.mpl_disconnect(manager._cidgcf)

if not interactive and manager in Gcf._activeQue:
Gcf._activeQue.remove(manager)

show = Show()

Expand Down
43 changes: 35 additions & 8 deletions lib/matplotlib/backends/backend_webagg_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ def __init__(self, *args, **kwargs):
# sent to the clients will be a full frame.
self._force_full = True

# Store the current image mode so that at any point, clients can
# request the information. This should be changed by calling
# self.set_image_mode(mode) so that the notification can be given
# to the connected clients.
self._current_image_mode = 'full'

def show(self):
# show the figure window
from matplotlib.pyplot import show
Expand All @@ -86,24 +92,41 @@ def draw(self):
def draw_idle(self):
self.send_event("draw")

def set_image_mode(self, mode):
Copy link
Member

Choose a reason for hiding this comment

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

Should this be done as a property?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could do. There is a performance cost to using properties over methods, but I agree, it would be a more pleasant interface. Having said that, I was very tempted to put a leading underscore in the name as it is a pretty useless interface unless you are developing some code in get_diff_image...

"""
Set the image mode for any subsequent images which will be sent
to the clients. The modes may currently be either 'full' or 'diff'.

Note: diff images may not contain transparency, therefore upon
draw this mode may be changed if the resulting image has any
transparent component.

"""
if mode not in ['full', 'diff']:
Copy link
Member

Choose a reason for hiding this comment

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

throw in a mode = mode.lower() to make it a bit more forgiving?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that a good thing? Personally, I'd sooner let the user know that they haven't given the right parameter so that they can change it as they are developing.

Copy link
Member

Choose a reason for hiding this comment

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

There are only two options here so we are using this more-or-less as an enum, 'mode', 'MoDe' and 'MODE' all seem equivalent to me. I don't see the benefit of being picky here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see the benefit of being picky here.

Its such as moot point, I'm happy to change it, but I equally don't see any benefit of accepting MoDe as a valid input. It adds interface/code complexity (admittedly the complexity gain is trivial) for zero benefit IMHO.

Copy link
Member

Choose a reason for hiding this comment

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

We have now spent more time on this than it is worth, I am fine either way.

raise ValueError('image mode must be either full or diff.')
if self._current_image_mode != mode:
self._current_image_mode = mode
self.handle_send_image_mode(None)

def get_diff_image(self):
if self._png_is_old:
renderer = self.get_renderer()

# The buffer is created as type uint32 so that entire
# pixels can be compared in one numpy call, rather than
# needing to compare each plane separately.
renderer = self.get_renderer()
buff = np.frombuffer(renderer.buffer_rgba(), dtype=np.uint32)

buff.shape = (renderer.height, renderer.width)

# If any pixels have transparency, we need to force a full draw
# as we cannot overlay new on top of old.
# If any pixels have transparency, we need to force a full
# draw as we cannot overlay new on top of old.
pixels = buff.view(dtype=np.uint8).reshape(buff.shape + (4,))
some_transparency = np.any(pixels[:, :, 3] != 255)

output = buff

if not self._force_full and not some_transparency:
if self._force_full or np.any(pixels[:, :, 3] != 255):
self.set_image_mode('full')
output = buff
else:
self.set_image_mode('diff')
last_buffer = np.frombuffer(self._last_renderer.buffer_rgba(),
dtype=np.uint32)
last_buffer.shape = (renderer.height, renderer.width)
Expand Down Expand Up @@ -230,6 +253,10 @@ def handle_resize(self, event):
self._png_is_old = True
self.manager.resize(w, h)

def handle_send_image_mode(self, event):
# The client requests notification of what the current image mode is.
self.send_event('image_mode', mode=self._current_image_mode)

def send_event(self, event_type, **kwargs):
self.manager._send_event(event_type, **kwargs)

Expand Down
13 changes: 12 additions & 1 deletion lib/matplotlib/backends/web_backend/mpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ mpl.figure = function(figure_id, websocket, ondownload, parent_element) {
this.format_dropdown = undefined;

this.focus_on_mousover = false;
this.image_mode = 'full';

this.root = $('<div/>');
this.root.attr('style', 'display: inline-block');
Expand All @@ -56,11 +57,17 @@ mpl.figure = function(figure_id, websocket, ondownload, parent_element) {

this.ws.onopen = function () {
fig.send_message("supports_binary", {value: fig.supports_binary});
fig.send_message("send_image_mode", {});
fig.send_message("refresh", {});
}

this.imageObj.onload = function() {
fig.context.clearRect(0, 0, fig.canvas.width, fig.canvas.height);
if (fig.image_mode == 'full') {
// Full images could contain transparency (where diff images
// almost always do), so we need to clear the canvas so that
// there is no ghosting.
fig.context.clearRect(0, 0, fig.canvas.width, fig.canvas.height);
}
fig.context.drawImage(fig.imageObj, 0, 0);
fig.waiting = false;
};
Expand Down Expand Up @@ -302,6 +309,10 @@ mpl.figure.prototype.handle_draw = function(fig, msg) {
fig.send_draw_message();
}

mpl.figure.prototype.handle_image_mode = function(fig, msg) {
fig.image_mode = msg['mode'];
}

mpl.figure.prototype.updated_canvas_event = function() {
// Called whenever the canvas gets updated.
this.send_message("ack", {});
Expand Down