Fix Cursor widget drawing behavior with blitting support #29971
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
|
Hello, it's just that I haven't heard from my contribution yet and it's been almost 2 weeks. The github bot suggested I leave a comment here so it would bring attention to any reviewers who are willing to check it and give me some feedback. So I would just like to ask if it is possible to review my PR soon so I could try to fix anything if needed. Hope to get some feedback soon! |
|
Hi @livlutz thank you for your contribution and sorry you did not get a response sooner. Is this PR in response to issue #29731? If so, please edit your summary above to include "closes #29731". I am not familiar with this module but I think the suggestion at #29731 (comment) was to make this method consistent with the one on |
|
Thanks for the feedback! Yes, it is related to #29731! I'll change it on my summary shortly. I did not try to fix it using MultiCursor though, I used self.canvas.blit() instead...should I take another look and try to use MultiCursor this time? |
|
As I said I'm not familiar with this module so definitely no expert. So I had a play with the code examples Cursor and MultiCursor to teach myself something about it. For I also tried the |
There was a problem hiding this comment.
I made a few comments to help you think about the change. I am inexperienced with blitting concepts so feel free to say so if you think I'm mistaken. I have just been looking at this example to try to understand what is happening.
lib/matplotlib/widgets.py
Outdated
| self.canvas.restore_region(self.background) | ||
| else: | ||
| self.canvas.draw() | ||
| self.canvas.restore_region(self.background) |
There was a problem hiding this comment.
self.background may be None here, in which case this fails. I think it is always None if useblit is False.
lib/matplotlib/widgets.py
Outdated
| else: | ||
| self.canvas.draw() | ||
| self.canvas.restore_region(self.background) | ||
| self.canvas.blit(self.ax.bbox) |
There was a problem hiding this comment.
Since useblit may be False here, I think it does not make sense to use blit.
lib/matplotlib/widgets.py
Outdated
| @@ -1934,8 +1934,15 @@ def onmove(self, event): | ||
| self.linev.set_visible(False) | ||
| self.lineh.set_visible(False) | ||
| if self.needclear: | ||
| if self.useblit and self.background is not None: |
There was a problem hiding this comment.
If both needclear and useblit are True, is it possible for background to be None? Looking at the code I do not think so, but maybe I missed something.
|
Hello, I tried to apply the suggested changes and it looks a bit better, could you try to take a look again and see if it is alright? |
|
I'm afraid I do not understand your changes. Can you explain how you chose what goes in the different branches of the if-loops? Also, please test your changes locally. Unfortunately there are no unit tests for this class, but you can run some example code that uses it. When you run an example against your branch, does the behaviour look right? Is it the same as without your changes? If not, fix it. I'm going to mark this as "draft" for now. Please mark it as "ready for review" when you are ready. In the meantime, if you need help, feel free to ask questions here. Or you may prefer to ask them in our incubator gitter room. |
PR summary
In class cursor,when the mouse moves out of the ax, redrawing takes a long time,
it solves the issue with the redrawing method, which takes too long when the mouse moves out of the ax, after fixing it should be more efficient
closes #29731
Summarising changes :
"Fix Cursor widget drawing behavior with blitting support"
PR checklist