Set macOS icon when using Qt backend #21784
Conversation
On most platforms, windows have their own icons that are displayed in conjunction with windows. On macOS, icons are used differently. Individual windows should not have an icon unless they are associated with a file. On the other hand, processes ("Apps") have icons that can be set.
If Matplotlib is running a FigureManagerBase, then we can assume that Matplotlib can be responsible for setting the process icon.
|
@jklymak Oh, no, it definitely stomps on the icon if you run it via something that uses a FigureManagerBase. If you want to verify it doesn't always stomp on the icon, you just need to run an example of embedding. I'll see if I can find something... |
| @@ -121,6 +120,10 @@ def _create_qApp(): | |||
| except AttributeError: # Only for Qt>=5.14. | |||
| pass | |||
| qApp = QtWidgets.QApplication(["matplotlib"]) | |||
| if sys.platform == "darwin": | |||
There was a problem hiding this comment.
Is there a harm in setting this regardless of platform? I just tested on a Linux box and it didn't seem to have an impact.
Ditto to the block below, where even if we are on mac, I don't think it hurts anything to call setWindowIcon on the windows.
There was a problem hiding this comment.
I don't have a problem removing this one.
With the other one, theoretically window icons on macOS are supposed to only be set along with a file path when the window represents a file. However, if you don't set the file path, the icon won't show. So while it's an unnecessary call that's semantically incorrect, it's effectively a noop.
So I guess it's really up to your priorities: semantics? Performance? Less code? Up to you!
There was a problem hiding this comment.
Here's the documentation from Qt re: QWidget::setWindowIcon():
https://doc.qt.io/qt-5/qwidget.html#windowIcon-prop
There was a problem hiding this comment.
Does it make sense to keep Darwin for now, and if someone wants this on another system they can add it?
PR Summary
On most platforms, windows have their own icons that are displayed in conjunction with windows. On macOS, icons are used differently. Individual windows should not have an icon unless they are associated with a file. On the other hand, processes ("Apps") have icons that can be set.
@greglucas suggested applying these fixes within derivations of FigureManagerBase. FigureManagerBase should only be instantiated when Matplotlib is responsible for running the GUI and its event loop. In this case, it makes sense for Matplotlib to control the App icon. (see #14930 (comment))
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand 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).