-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: Update blitting and drawing on the macosx backend #21790
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
Conversation
5c10202
to
fce4431
Compare
Can you give a bit of a guide for how to test this worked? I think the general idea is great - to make the mac backend as similar as possible to the others.... |
I was testing on the examples from the linked issues as they had pretty good minimal code samples. All of them basically didn't show any updates (redraw the entire canvas background which was white, rather than blitting and updating only the new portion of the figure). #10980 (Should rotate around) Code for reproductionfrom mpl_toolkits.mplot3d import axes3d
import matplotlib.pyplot as plt
fig = plt.figure()
ax = fig.add_subplot(111, projection='3d')
# load some test data for demonstration and plot a wireframe
X, Y, Z = axes3d.get_test_data(0.1)
ax.plot_wireframe(X, Y, Z, rstride=5, cstride=5)
# rotate the axes and update
for angle in range(0, 360):
ax.view_init(30, angle)
plt.draw()
plt.pause(.001) #17445 (Should not print "Drawing!" 40+ times and also update the arrow animation) Code for reproductionimport matplotlib.pyplot as plt
from matplotlib import animation
import numpy as np
class BaseVisualizer():
def __init__(self, **kwargs):
self.__iteration_number = kwargs.get('iteration_number', 1)
self.__intervals = 1
self.__interval_ms = kwargs.get('interval', 2000)
self._marker_size = 4
self._index = 0
self._positions = np.zeros(2)
self._velocities = np.ones(2)
self.__frame_interval = 50 # ms
def replay(self):
self._fig = plt.figure()
self._fig.canvas.mpl_connect('draw_event', lambda ev: print('Drawing!'))
self._fig.stale_callback = None
ax = self._fig.add_subplot(1, 1, 1, label='BaseAxis')
# Plot all particle pos
self.__particles = ax.scatter([], [], marker='o', zorder=2, color='red')
# Plot all velocities
self.__particle_vel = ax.quiver([], [], [], [], angles='xy', scale_units='xy', scale=1)
self.__rectangle = plt.Rectangle([0,0], 1, 1, ec='none', lw=2, fc='none')
ax.add_patch(self.__rectangle)
# Amount of frames to play considering one interval should last __interval ms.
frames = int(self.__intervals*self.__interval_ms/self.__frame_interval)
# iteration_number+1 for initialization frame
_ = animation.FuncAnimation(self._fig, self._animate, frames=frames, interval=self.__frame_interval,
blit=True, repeat=False, fargs=[frames])
plt.show()
def _animate(self, i: int, frames: int):
"""
Animation function for animations. Only used for FuncAnimation
"""
self._marker_size = int(50 * self._fig.get_figwidth()/self._fig.dpi)
# Calculate the scale to apply to the data in order to generate a more dynamic visualization
scale = i / (frames/self.__intervals)
# Calculate scaled position and velocity
pos_scaled = self._positions + scale * self._velocities
vel_scaled = (1-scale)*self._velocities
# Update the particle position
self.__particles.set_offsets(pos_scaled)
self.__particles.set_sizes([self._marker_size**2])
# Update the velocities
ax = self._fig.gca(label='BaseAxis')
self.__particle_vel = ax.quiver(pos_scaled[0], pos_scaled[1], vel_scaled[0], vel_scaled[1], angles='xy', scale_units='xy', scale=1, color='#CFCFCF', width=self._marker_size*0.001)
return self.__particles, self.__particle_vel
if __name__ == '__main__':
vis = BaseVisualizer()
vis.replay() #17642 (A sine wave should move across the screen) Code for reproductionimport matplotlib.pyplot as plt
import numpy as np
x = np.linspace(0, 2 * np.pi, 100)
fig, ax = plt.subplots()
# animated=True tells matplotlib to only draw the artist when we
# explicitly request it
(ln,) = ax.plot(x, np.sin(x), animated=True)
# make sure the window is raised, but the script keeps going
plt.show(block=False)
# stop to admire our empty window axes and ensure it is rendered at
# least once.
#
# We need to fully draw the figure at its final size on the screen
# before we continue on so that :
# a) we have the correctly sized and drawn background to grab
# b) we have a cached renderer so that ``ax.draw_artist`` works
# so we spin the event loop to let the backend process any pending operations
plt.pause(0.1)
# get copy of entire figure (everything inside fig.bbox) sans animated artist
bg = fig.canvas.copy_from_bbox(fig.bbox)
# draw the animated artist, this uses a cached renderer
ax.draw_artist(ln)
# show the result to the screen, this pushes the updated RGBA buffer from the
# renderer to the GUI framework so you can see it
fig.canvas.blit(fig.bbox)
for j in range(100):
# reset the background back in the canvas state, screen unchanged
fig.canvas.restore_region(bg)
# update the artist, neither the canvas state nor the screen have changed
ln.set_ydata(np.sin(x + (j / 100) * np.pi))
# re-render the artist, updating the canvas state, but not the screen
ax.draw_artist(ln)
# copy the image to the GUI state, but screen might not changed yet
fig.canvas.blit(fig.bbox)
# flush any pending GUI events, re-painting the screen if needed
fig.canvas.flush_events()
# you can put a pause in if you want to slow things down
# plt.pause(.1) |
I can confirm this fixes the attached examples. I don't think I have the expertise to thoroughly review the code changes, but within what I know they look fine. Is there a way to add a blitting test for the macosx backend to the test suite? |
This is definitely a good idea, but as far as I can tell if I try to save anything during the process, the buffer gets updated and the draw does actually happen, so the old versions would actually still appear to work from a test perspective, even though they don't on the screen. I tried grabbing the buffer during one of the animations, and the buffer is getting updated I believe, it is just Mac isn't drawing the buffer to the screen. If anyone has a suggestion for how to do the final screen buffer grab I'm happy to implement that. Here is the quick check I was trying comparing new/old buffer bytes before/after blitting or animating: new_bytes = bytes(fig.canvas.buffer_rgba())
print(new_bytes == orig_bytes) # False on both main and this PR @dstansby, did the final example with the sine wave work for you? When I go back to that today I don't get the flush_events to work as expected, so I may have had something crossed up in my environments. I think I may also need to bring over the commit from #21768 after all. |
self._draw_pending = True | ||
with self._idle_draw_cntx(): | ||
self._draw_pending = False | ||
self.draw() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not call draw
as draw_idle
should always be "cheap" .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I took this from the qt_backend, which appears to do the same self.draw()
call from within draw_idle
? I'm not entirely clear where the "pending" of draw_idle() should take place here, whether it should be within the Python context managers, or if perhaps I'm just missing putting this into the Mac-specific event loop similar to the singleShot call from QT?
matplotlib/lib/matplotlib/backends/backend_qt.py
Lines 450 to 461 in ec99c15
def _draw_idle(self): | |
with self._idle_draw_cntx(): | |
if not self._draw_pending: | |
return | |
self._draw_pending = False | |
if self.height() < 0 or self.width() < 0: | |
return | |
try: | |
self.draw() | |
except Exception: | |
# Uncaught exceptions are fatal for PyQt5, so catch them. | |
traceback.print_exc() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic with Qt is that every call to canvas.draw_idle
set self._draw_pending
to True
and then schedules a single-shot timer to run canvas._draw_idle
. Thus many calls to draw_idle
within a block of Python (which runs without letting the event loop spin) will stack up a bunch of single-shot timers. When ever the main loop gets to run (which is the earliest we would be able to paint the screen anyway!) the timers start to expire. The first one actually does the update and the rest short-circuit out.
I suspect that you can do the same single-shot trick with OSX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the clear explanation! That makes sense to me (stacking a bunch of callbacks in the event loop, and those callbacks depend on whether the draw has already happened or not). Let me know if you think the implementation is wrong here, or if I misunderstood.
OK, I must have had something mixed up in my test cases before and this does need the update to flush_events so that the macosx event loop has time to process the updates. I brought the commit from #21790 over here, but I'm happy to reopen that one again if it would be easier consider them separately. |
ad04a33
to
ecd8c84
Compare
I just pushed up a new commit that I think addresses the I was able to get around this by stopping the timer at the Python level after firing. I'm not sure that is the most elegant way around this, so I'm open to suggestions if anyone has other ideas for how to approach the runloop singleshot timers here. |
335355b
to
72a028a
Compare
I have now added a test to verify that we aren't getting multiple draw_events emitted from the backends. I added it into |
src/_macosx.m
Outdated
@@ -392,6 +385,9 @@ static CGFloat _get_device_scale(CGContextRef cr) | |||
static PyObject* | |||
FigureCanvas_flush_events(FigureCanvas* self) | |||
{ | |||
// We need to interrupt the runloop very briefly to allow the view to be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this interrupting the runloop (which is what I assume that OSX calls the event loop?) or letting it actually run (and interrupting the Python interpreter)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain... Is there a way to debug that and see what is going on? I think you are right though, the text I had was misleading about "interrupting". I'm pretty sure we are doing the latter, so I updated the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one comment about a comment on the runloop, but otherwise looks good 👍🏻
b5afb9e
to
b130b75
Compare
It looks like the test I added is not happy on some architectures, before I go banging on CI I figured I would see what others would prefer. I see a few options:
|
A possible idea: |
|
0fd4022
to
0c92c89
Compare
Fully avoiding the animation framework and just testing the thing we want to test is a much better idea than inviting generators to the party! |
@dstansby, I think this is good to go for a check from you again. i.e. can you double check me on those examples to make sure I didn't screw something up? Don't apologize for the test request! It is always good to add tests when we can. @tacaswell, This steals most of your other blitting example for the test, so thanks for that :) Unfortunately, the actual reason for going this direction was because macosx has an issue with single-shot timers and not quitting out of the test on the runners when using the generator/animation approach. Locally it hangs until I move my mouse or do something else interactive on the system, so there is some other issue with macosx event loops and the handoff from Python <-> NSApp which I will try and make a reproducible example for and open an issue. |
0c92c89
to
25f9b92
Compare
25f9b92
to
6293cf0
Compare
This inherits some of the drawing and blitting code from the AGG canvas and leverages that to handle when a full AGG re-render should happen. The macosx code always grabs the full AGG buffer when doing the draw to the GUI window.
The macosx backend would not update on flush_events calls due to the loop being run too fast for the view to update properly in the NSApp. Fix that by adding an unnoticeable RunLoop timer slowdown that allows the app to see the changes.
This adds a singleshot timer to the draw_idle method. This has a callback that triggers the draw upon firing the timer if the draw has not occurred yet, otherwise it short-circuits out of the draw and removes the timer.
6293cf0
to
8536c7a
Compare
Any mac users want to test this out and push it through the finish line? |
8536c7a
to
a4c007e
Compare
This adds a test that only one draw_event is called when using blitting with animations. There will be more than one draw_event if blitting is not implemented properly and instead calls a full redraw of the canvas each time.
a4c007e
to
5415418
Compare
ping @dstansby who recently reported owning a M1 machine and volunteering to test mac things ;) |
Of the examples listed in #21790 (comment), the 1st and 3rd work for me with this branch, but the 2nd does not. It prints "Drawing!" once, and nothing changes in the empty plot that appears. |
Thanks for checking @dstansby! That one has a very small marker and thin grey line that are updated, so maybe it just didn't show up as obviously (especially with HiDPI screen)? It does work for me locally copy-pasting that example. Can you try changing the marker size to make it more obvious: If that doesn't work, my guess is this would be an M1 issue then :-/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spotted the marker - clearly wasn't looking closely enough first time!
Thanks for checking! (It was not the easiest example to see) I'm going to self-merge based on the two approvals and this isn't targeting 3.5.x |
PR Summary
This moves some of the drawing and blitting code to the AGG canvas (instead of trying to use the objc routines) and leverages that to handle when a full AGG re-render should happen. The macosx code always grabs the full AGG buffer when drawing to the GUI window at the objc level (inside drawRect). I removed
_draw()
and added anupdate()
routine in objc that requests a screen draw within the main run loop.closes #10980
closes #17445 (Only does a full redraw once now!)
Fixes the example from #17642, but I'm not sure this is still feature-parity with what "flush_events" means on the other backends, so I guess it probably shouldn't be closed.
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).