8000 Drop pgi support for the GTK3 backend by lazka · Pull Request #13014 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Drop pgi support for the GTK3 backend #13014

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 23, 2018
Merged

Drop pgi support for the GTK3 backend #13014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ addons:
- graphviz
- inkscape
- libcairo2
- libcairo2-dev
- libffi-dev
- libgeos-dev
- libgirepository-1.0.1
- libgirepository1.0-dev
- lmodern
- otf-freefont
- pgf
- pkg-config
- texlive-fonts-recommended
- texlive-latex-base
- texlive-latex-extra
Expand Down Expand Up @@ -132,10 +135,11 @@ install:
# install was successful by trying to import the toolkit (sometimes, the
# install appears to be successful but shared libraries cannot be loaded at
# runtime, so an actual import is a better check).
python -mpip install --upgrade cairocffi>=0.8 pgi>=0.0.11.2 &&
python -c 'import pgi as gi; gi.require_version("Gtk", "3.0"); from pgi.repository import Gtk' &&
echo 'pgi is available' ||
echo 'pgi is not available'
python -mpip install --upgrade pycairo cairocffi>=0.8
python -mpip install --upgrade PyGObject &&
python -c 'import gi; gi.require_version("Gtk", "3.0"); from gi.repository import Gtk' &&
echo 'PyGObject is available' ||
echo 'PyGObject is not available'
python -mpip install --upgrade pyqt5 &&
python -c 'import PyQt5.QtCore' &&
echo 'PyQt5 is available' ||
Expand Down
3 changes: 1 addition & 2 deletions INSTALL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ optional Matplotlib backends and the capabilities they provide.
`PySide <https://pypi.org/project/PySide>`_ (>= 1.0.3): for the Qt4-based
backends;
* `PyQt5 <https://pypi.org/project/PyQt5>`_: for the Qt5-based backends;
* `PyGObject <https://pypi.org/project/PyGObject/>`_ or
`pgi <https://pypi.org/project/pgi/>`_ (>= 0.0.11.2): for the GTK3-based
* `PyGObject <https://pypi.org/project/PyGObject/>`_: for the GTK3-based
backends;
* :term:`wxpython` (>= 4): for the WX-based backends;
* `cairocffi <https://cairocffi.readthedocs.io/en/latest/>`_ (>= 0.8) or
Expand Down
4 changes: 4 additions & 0 deletions doc/api/next_api_changes/2018-12-19-gtk-drop-pgi.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Drop support for ``pgi`` in the GTK3 backends
`````````````````````````````````````````````
``pgi``, an alternative implementation to PyGObject, is no longer supported in
the GTK3 backends. PyGObject should be used instead.
7 changes: 0 additions & 7 deletions doc/glossary/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,6 @@ Glossary
channel. PDF was designed in part as a next-generation document
format to replace postscript

pgi
`pgi <https://pypi.python.org/pypi/pgi/>` exists as a relatively
new Python wrapper to GTK3 and acts as a pure python alternative to
PyGObject. pgi still exists in its infancy, currently missing many
features of PyGObject. However Matplotlib does not use any of these
missing features.

PyGObject
`PyGObject <http://www.pygtk.org/>`_ provides Python wrappers for the
:term:`GTK` widgets library
Expand Down
3 changes: 1 addition & 2 deletions lib/matplotlib/backends/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ def _get_running_interactive_framework():
or sys.modules.get("PySide.QtGui"))
if QtGui and QtGui.QApplication.instance():
return "qt4"
Gtk = (sys.modules.get("gi.repository.Gtk")
or sys.modules.get("pgi.repository.Gtk"))
Gtk = sys.modules.get("gi.repository.Gtk")
if Gtk and Gtk.main_level():
return "gtk3"
wx = sys.modules.get("wx")
Expand Down
55 changes: 0 additions & 55 deletions lib/matplotlib/backends/_gtk3_compat.py
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions lib/matplotlib/backends/backend_cairo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@

import numpy as np

# cairocffi is more widely compatible than pycairo (in particular pgi only
# works with cairocffi) so try it first.
# cairocffi is more widely compatible than pycairo so try it first.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to prefer pycairo now? (as pygobject will go through pycairo regardless).
In fact, does pygobject even work (in this use case) without pycairo?

Copy link
Contributor Author
@lazka lazka Dec 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that there is code to convert pycairo objects to cairocffi ones for some fastpath implementations, so I assumed it is still preferred.

If there is anything I can do in pycairo to make it the preferred one I'm happy to help.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, does pygobject even work (in this use case) without pycairo?

No, it can be build without pycairo but then any function using cairo types will not work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means in particular on_draw_event (connected to the "draw" event), no?
IOW, can we just assume that gtk3 is built with cairo support and that cairo is a dependency, and error out early if not?

I don't think keeping the cairocffi optional dependency just for the sake of a not-even-twofold speedup is worth it (but if you want to make expose something similar in pycairo's side what would be needed is a way to create a cairo Path from a list of cairo_path_data_t (or better, from a buffer (in the memoryview sense), so that no copy at all is done). (Effectively, this runs afoul of the last paragraph of https://cairographics.org/manual/bindings-path.html ("You should not present an API for mutating or for creating new cairo_path_t objects. In the future, these guidelines may be extended to present an API for creating a cairo_path_t from scratch for use with cairo_append_path() but the current expectation is that cairo_append_path() will mostly be used with paths from cairo_copy_path()."), but performance wise it does make some difference.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means in particular on_draw_event (connected to the "draw" event), no?

yeah

IOW, can we just assume that gtk3 is built with cairo support and that cairo is a dependency, and error out early if not?

We have https://pygobject.readthedocs.io/en/latest/guide/api/api.html?highlight=require_foreign#gi.require_foreign to check for this.

I don't think keeping the cairocffi optional dependency just for the sake of a not-even-twofold speedup is worth it [...]

Thanks, I'll have a look.

Either way I think any changes to the cairo part should be another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was originally done to support py3 but I guess yeah I don't know the situation with pypy right now either, @lazka?

Copy link
Contributor Author
@lazka lazka Dec 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was originally done to support py3

Yeah, the pycairo in Ubuntu 16.04 for example (the distro provided package) is lacking various API compared to the Python 2 version, and also has a few bugs/crashers.

I don't know the situation with pypy right now either,

PyGObject and Pycairo do work with PyPy now: https://lazka.github.io/posts/2018-04_pypy-pygobject/index.html but pycairo is probably slower than cairocffi with PyPy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xenial has pycairo 1.10 but we require 1.11 indeed.

Copy link
Contributor Author
@lazka lazka Dec 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer if I just take the code in _append_paths_fast() that computes codes/vertices and loop over them and call the path functions I get the same FPS boost as with the buffer passing approach (unless I'm missing something). So I think the whole cairo_path_data_t thing is not needed and the fast path would also works with pycairo as is.

(tested with wire3d_animation_sgskip)

Copy link
Contributor Author
@lazka lazka Dec 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, the "fast path" cleans up the path first, so they aren't doing the same thing. The main speedup comes from removing the curve segments. So assuming that shortcut is OK this would be faster than the current fast path for both libraries: (turned out to be wrong, see my PRs)

# Mapping from Matplotlib Path codes to cairo path functions.
_MPL_TO_CAIRO_PATH_TYPE = [lambda ctx, *args: None] * (Path.CLOSEPOLY + 1)
_MPL_TO_CAIRO_PATH_TYPE[Path.MOVETO] = cairo.Context.move_to
_MPL_TO_CAIRO_PATH_TYPE[Path.LINETO] = cairo.Context.line_to
_MPL_TO_CAIRO_PATH_TYPE[Path.CLOSEPOLY] = \
    lambda ctx, *args: cairo.Context.close_path(ctx)

def _append_path(ctx, path, transform, clip=None):
    cleaned = path.cleaned(transform, remove_nans=True, clip=clip)
    for points, code in zip(cleaned.vertices, cleaned.codes):
        _MPL_TO_CAIRO_PATH_TYPE[code](ctx, *points)

try:
import cairocffi as cairo
except ImportError:
Expand Down
17 changes: 16 additions & 1 deletion lib/matplotlib/backends/backend_gtk3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,22 @@
from matplotlib.backend_managers import ToolManager
from matplotlib.figure import Figure
from matplotlib.widgets import SubplotTool
from ._gtk3_compat import GLib, GObject, Gtk, Gdk

try:
import gi
except ImportError:
raise ImportError("The GTK3 backends require PyGObject")

try:
# :raises ValueError: If module/version is already loaded, already
# required, or unavailable.
gi.require_version("Gtk", "3.0")
except ValueError as e:
# in this case we want to re-raise as ImportError so the
# auto-backend selection logic correctly skips.
raise ImportError from e

from gi.repository import GLib, GObject, Gtk, Gdk


_log = logging.getLogger(__name__)
Expand Down
5 changes: 2 additions & 3 deletions lib/matplotlib/tests/test_backends_interactive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
def _get_testable_interactive_backends():
backends = []
for deps, backend in [
# gtk3agg fails on Travis, needs to be investigated.
# (["cairocffi", "pgi"], "gtk3agg"),
(["cairocffi", "pgi"], "gtk3cairo"),
(["cairo", "gi"], "gtk3agg"),
(["cairo", "gi"], "gtk3cairo"),
(["PyQt5"], "qt5agg"),
(["PyQt5", "cairocffi"], "qt5cairo"),
(["tkinter"], "tkagg"),
Expand Down
0