Skip to content

PRF: only check some artists on mousemove #4878

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

Merged
merged 5 commits into from
Aug 16, 2015
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions lib/matplotlib/artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def __init__(self):
self._contains = None
self._rasterized = None
self._agg_filter = None

self._mouseover = False
self.eventson = False # fire events only if eventson
self._oid = 0 # an observer id
self._propobservers = {} # a dict from oids to funcs
Expand Down Expand Up @@ -147,6 +147,23 @@ def remove(self):
# callback has one parameter, which is the child to be removed.
if self._remove_method is not None:
self._remove_method(self)
Copy link
Member

Choose a reason for hiding this comment

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

Surely the logic for removing from the axes should live in the remove method itself. Is there a reason we can't do that?

# clear stale callback
self.stale_callback = None
_ax_flag = False
if hasattr(self, 'axes') and self.axes:
# remove from the mouse hit list
self.axes.mouseover_set.discard(self)
# mark the axes as stale
self.axes.stale = True
# decouple the artist from the axes
self.axes = None
_ax_flag = True

if self.figure:
self.figure = None
if not _ax_flag:
self.figure = True

else:
raise NotImplementedError('cannot remove artist')
# TODO: the fix for the collections relim problem is to move the
Expand Down Expand Up @@ -214,7 +231,9 @@ def axes(self):

@axes.setter
def axes(self, new_axes):
if self._axes is not None and new_axes != self._axes:

if (new_axes is not None and
(self._axes is not None and new_axes != self._axes)):
raise ValueError("Can not reset the axes. You are "
"probably trying to re-use an artist "
"in more than one Axes which is not "
Expand Down Expand Up @@ -961,6 +980,21 @@ def format_cursor_data(self, data):
data = [data]
return ', '.join('{:0.3g}'.format(item) for item in data)

@property
def mouseover(self):
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering, have you ever measured the performance of descriptor access vs attribute lookup. You might be quite shocked 😱

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 have not, but I made this a property for the setter functionality.

return self._mouseover

@mouseover.setter
def mouseover(self, val):
val = bool(val)
self._mouseover = val
ax = self.axes
if ax:
if val:
ax.mouseover_set.add(self)
else:
ax.mouseover_set.discard(self)


class ArtistInspector(object):
"""
Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/axes/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,8 @@ def _set_artist_props(self, a):
a.set_transform(self.transData)

a.axes = self
if a.mouseover:
self.mouseover_set.add(a)
Copy link
Member

Choose a reason for hiding this comment

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

Alarm bells... 🔔
What if the artist is subsequently removed from the axes? This redundancy is problematic.

Copy link
Member Author

Choose a reason for hiding this comment

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

The remove method takes care of this.


def _gen_axes_patch(self):
"""
Expand Down Expand Up @@ -916,6 +918,7 @@ def cla(self):
self.tables = []
self.artists = []
self.images = []
self.mouseover_set = set()
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 not a huge fan of the name. Is this really "artists who have mouseover enabled"? Can we come up with a better name?

self._current_image = None # strictly for pyplot via _sci, _gci
self.legend_ = None
self.collections = [] # collection.Collection instances
Expand Down
4 changes: 3 additions & 1 deletion lib/matplotlib/backend_bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -2815,9 +2815,11 @@ def mouse_move(self, event):
except (ValueError, OverflowError):
pass
else:
artists = event.inaxes.hitlist(event)
artists = [a for a in event.inaxes.mouseover_set
if a.contains(event)]

if artists:

a = max(enumerate(artists), key=lambda x: x[1].zorder)[1]
if a is not event.inaxes.patch:
data = a.get_cursor_data(event)
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def __init__(self, ax,
"""
martist.Artist.__init__(self)
cm.ScalarMappable.__init__(self, norm, cmap)

self._mouseover = True
if origin is None:
origin = rcParams['image.origin']
self.origin = origin
Expand Down
26 changes: 26 additions & 0 deletions lib/matplotlib/tests/test_artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import matplotlib.collections as mcollections
from matplotlib.testing.decorators import image_comparison, cleanup

from nose.tools import (assert_true, assert_false)


@cleanup
def test_patch_transform_of_none():
Expand Down Expand Up @@ -144,6 +146,30 @@ def test_cull_markers():
assert len(svg.getvalue()) < 20000


@cleanup
def test_remove():
fig, ax = plt.subplots()
im = ax.imshow(np.arange(36).reshape(6, 6))

assert_true(fig.stale)
assert_true(ax.stale)

fig.canvas.draw()
assert_false(fig.stale)
assert_false(ax.stale)

assert_true(im in ax.mouseover_set)
assert_true(im.axes is ax)

im.remove()

assert_true(im.axes is None)
assert_true(im.figure is None)
assert_true(im not in ax.mouseover_set)
assert_true(fig.stale)
assert_true(ax.stale)


if __name__ == '__main__':
import nose
nose.runmodule(argv=['-s', '--with-doctest'], exit=False)