Only set the wait cursor if the last draw was >1s ago. #9672
Conversation
9b81bee to
e9b7c67
Compare
| toolbar.set_cursor(toolbar._lastCursor) | ||
| return False # finish event propagation? | ||
| with (self.toolbar._wait_cursor_for_draw_cm() if self.toolbar | ||
| else ExitStack()): |
There was a problem hiding this comment.
self.toolbar._wait_cursor_for_draw_cm() if self.toolbar else ExitStack() happens here the second time, ExitStack is overkill for this usage, I think it would be nice to move this to a function and do not bring a contextlib2 dependency.
There was a problem hiding this comment.
It's not a second time; this is gtk3cairo which goes through a different rendering path than the various agg backends.
Using ExitStack as a "do-nothing" context manager to optionally enter a context is explicitly mentioned in the docs: https://docs.python.org/3/library/contextlib.html#simplifying-support-for-single-optional-context-managers. contextlib2 is a new dependency only on Py2; I think this is perfectly idiomatic Py3 (and again Py2 is very low on my priority list...). It's also a pure python dependency so it's not as if installing it was particularly difficult.
There was a problem hiding this comment.
In Python 3.7, there will be an explicit nullcontext() for that purpose, which is semantically more clear.
https://docs.python.org/3/library/contextlib.html#simplifying-support-for-single-optional-context-managers
Please add a comment concerning this so that we can replace it once we'll depend on 3.7+ only.
Alternatively:
try:
from contextlib import nullcontext
except ImportError: # python 3.6 backward compatibility
from contextlib import ExitStack as nullcontext
| @@ -399,8 +396,6 @@ def expose_event(self, widget, event): | |||
| x, y, w, h = event.area | |||
| self.window.draw_drawable (self.style.fg_gc[self.state], | |||
| self._pixmap, x, y, x, y, w, h) | |||
| if toolbar: | |||
| toolbar.set_cursor(toolbar._lastCursor) | |||
| return False # finish event propagation? | |||
There was a problem hiding this comment.
I removed the wait cursor on GTK. I honestly have zero interest in fixing a Py2-only backend.
285f715 to
986840c
Compare
|
I did actually forget to declare the dependency in a few places (docs + conda meta.yaml). |
986840c to
01fe73d
Compare
5ca2e45 to
6b26215
Compare
|
I think this is borderline release critical for 2.1.2 (because the wait cursor turned out to be quite more an annoyance than I expected). Feel free to untag if you disagree. |
|
It'll have to go to 3.0 if it can't pass the Py2.7 test 😉 |
d8a4539 to
faa9f54
Compare
|
Can we do this without adding a dependency? I am 👎 on adding a dependency on a bug-fix release. Maybe we should just add a a 'turn it off' escape hatch? |
|
I'm fine with reverting the original wait cursor PR for now. Or we can just vendor exitstack. |
|
@anntzer this should simplify now that we are py3 only. Interested in a rebase? |
faa9f54 to
a520fa3
Compare
|
thanks, rebased |
a520fa3 to
87272b6
Compare
|
Still conflicts though. |
|
re-rebased :) |
87272b6 to
3e9b8dc
Compare
| toolbar.set_cursor(toolbar._lastCursor) | ||
| return False # finish event propagation? | ||
| with (self.toolbar._wait_cursor_for_draw_cm() if self.toolbar | ||
| else ExitStack()): |
There was a problem hiding this comment.
In Python 3.7, there will be an explicit nullcontext() for that purpose, which is semantically more clear.
https://docs.python.org/3/library/contextlib.html#simplifying-support-for-single-optional-context-managers
Please add a comment concerning this so that we can replace it once we'll depend on 3.7+ only.
Alternatively:
try:
from contextlib import nullcontext
except ImportError: # python 3.6 backward compatibility
from contextlib import ExitStack as nullcontext
This avoids setting the wait cursor when panning or zooming (that behavior was somewhat annoying), or during an animation that renders at least one frame per second. Rename `_set_cursor` to `_update_cursor` as it is fairly different from `set_cursor` and the previous name could cause confusion. Move the defintion of cursor-related methods to a single place.
3e9b8dc to
0944dcb
Compare
|
sure, switched to nullcontext() |
This avoids setting the wait cursor when panning or zooming (that
behavior was somewhat annoying), or during an animation that renders at
least one frame per second.
Rename
_set_cursorto_update_cursoras it is fairly different fromset_cursorand the previous name could cause confusion. Move thedefintion of cursor-related methods to a single place.
In the case of gtk, just strip out the offending cursor related code as
the backend is deprecated and slated for removal anyways.
Fixes #9546.
PR Summary
PR Checklist