Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Qt5: Eliminate slow path when showing messages#4894
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -493,8 +497,7 @@ def notify_axes_change(fig): | |||
| @QtCore.Slot() | |||
| def _show_message(self, s): | |||
| # Fixes a PySide segfault. | |||
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.
Do you know what this comment is referring to?
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.
Looking at the history it seems that connecting the showMessage slot directly resulted in a segfault in PySide. The_show_message wrapper appears to be a workaround.
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.
Can we drop this helper function and just hook upself.statusbar_label.setText to the proper signal?
Seems reasonable to me. Still not sure I understand the underlying cause of the regression. attn@mfitzp |
Looks good to me. Using the QLabel in the statusbar should also work around the PySide bug that the comment @tacaswell the I think the regression is a combination of the increased mouse events in Qt5, possibly some slowness in matplotlib and using a slower (blocking) method to update the statusbar. Any of those individually probably wasn't an issue, but the combination leads to noticeable lag. Relevant (unanswered) post about slowdown caused by |
The cause of the regression is the increase in mouse events from Qt5. They are emitted at rates at which you cannot synchronously update the GUI any more - which one isn't supposed to do in the first place. Again, I think this problem was present in the code all along, it's just that you won't notice it if the event rates are low enough (i.e. not like a typical regression caused by a specific code change). In the debugger you can see that when the |
@mfitzp Please do the honors of merging if/when you are happy with this? [edit typing, reading mid-sentence, and then typing again is hard] |
Tested with PyQt4, 5 and PySide without problems. |
Qt5: Eliminate slow path when showing messages
It would be interesting to see how a QtQuick based approach performs, since event handling and scene drawing are decoupled more rigorous. I'd expect selectionbox/rubberband and the xy-coordinate text to perform better with this framework. |
Thanks both of you. @mfitzp I think this is your first merge to master, congratulations! |
@tacaswell It is, thanks!waits for things to explode |
FYI Qt5 has a change (vs Qt4) in the way they report mouse events which considered a bug. The patch in the bug report does restore the Qt4 behavior which is much better. See:https://bugreports.qt.io/browse/QTBUG-40889 |
Let's quickly fix all slow-paths then while they are so extremely noticeable ;) |
For every single mouse-move event the Qt5 backend displays a message (x, y coordinates) via
QStatusBar.showMessage(). This function forces an immediate update of the GUI and causes the application to lag behind mouse movements in case the update process cannot keep up with the event-rate.As an alternative approach I added a QLabel to the QStatusBar and changed its text property for displaying messages. The redraws for message updates are now handled asynchronously and thus result in much better GUI performance, at least on the systems I tested.