Skip to content

Improve headlessness detection for backend selection. #17396

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
Oct 11, 2020
+93 −27
Merged

Improve headlessness detection for backend selection. #17396

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
13 changes: 9 additions & 4 deletions lib/matplotlib/backend_bases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2755,10 +2755,15 @@ def show(self):
warning in `.Figure.show`.
"""
# This should be overridden in GUI backends.
if cbook._get_running_interactive_framework() != "headless":
raise NonGuiException(
f"Matplotlib is currently using {get_backend()}, which is "
f"a non-GUI backend, so cannot show the figure.")
if sys.platform == "linux" and not os.environ.get("DISPLAY"):
# We cannot check _get_running_interactive_framework() ==
# "headless" because that would also suppress the warning when
# $DISPLAY exists but is invalid, which is more likely an error and
# thus warrants a warning.
return
raise NonGuiException(
f"Matplotlib is currently using {get_backend()}, which is a "
f"non-GUI backend, so cannot show the figure.")

def destroy(self):
pass
Expand Down
24 changes: 9 additions & 15 deletions lib/matplotlib/backends/backend_qt5.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
import sys
import traceback

import matplotlib

import matplotlib as mpl
from matplotlib import backend_tools, cbook
from matplotlib._pylab_helpers import Gcf
from matplotlib.backend_bases import (
Expand Down Expand Up @@ -113,11 +112,8 @@ def _create_qApp():
is_x11_build = False
else:
is_x11_build = hasattr(QtGui, "QX11Info")
if is_x11_build:
display = os.environ.get('DISPLAY')
if display is None or not re.search(r':\d', display):
raise RuntimeError('Invalid DISPLAY variable')

if is_x11_build and not mpl._c_internal_utils.display_is_valid():
raise RuntimeError('Invalid DISPLAY variable')
try:
QtWidgets.QApplication.setAttribute(
QtCore.Qt.AA_EnableHighDpiScaling)
Expand Down Expand Up @@ -574,7 +570,7 @@ def __init__(self, canvas, num):

self.window.setCentralWidget(self.canvas)

if matplotlib.is_interactive():
if mpl.is_interactive():
self.window.show()
self.canvas.draw_idle()

Expand All @@ -601,9 +597,9 @@ def _widgetclosed(self):
def _get_toolbar(self, canvas, parent):
# must be inited after the window, drawingArea and figure
# attrs are set
if matplotlib.rcParams['toolbar'] == 'toolbar2':
if mpl.rcParams['toolbar'] == 'toolbar2':
toolbar = NavigationToolbar2QT(canvas, parent, True)
elif matplotlib.rcParams['toolbar'] == 'toolmanager':
elif mpl.rcParams['toolbar'] == 'toolmanager':
toolbar = ToolbarQt(self.toolmanager, self.window)
else:
toolbar = None
Expand All @@ -619,7 +615,7 @@ def resize(self, width, height):

def show(self):
self.window.show()
if matplotlib.rcParams['figure.raise_window']:
if mpl.rcParams['figure.raise_window']:
self.window.activateWindow()
self.window.raise_()

Expand Down Expand Up @@ -792,8 +788,7 @@ def save_figure(self, *args):
sorted_filetypes = sorted(filetypes.items())
default_filetype = self.canvas.get_default_filetype()

startpath = os.path.expanduser(
matplotlib.rcParams['savefig.directory'])
startpath = os.path.expanduser(mpl.rcParams['savefig.directory'])
start = os.path.join(startpath, self.canvas.get_default_filename())
filters = []
selectedFilter = None
Expand All @@ -811,8 +806,7 @@ def save_figure(self, *args):
if fname:
# Save dir for next time, unless empty str (i.e., use cwd).
if startpath != "":
matplotlib.rcParams['savefig.directory'] = (
os.path.dirname(fname))
mpl.rcParams['savefig.directory'] = os.path.dirname(fname)
try:
self.canvas.figure.savefig(fname)
except Exception as e:
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/cbook/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def _get_running_interactive_framework():
if 'matplotlib.backends._macosx' in sys.modules:
if sys.modules["matplotlib.backends._macosx"].event_loop_is_running():
return "macosx"
if sys.platform.startswith("linux") and not os.environ.get("DISPLAY"):
if not _c_internal_utils.display_is_valid():
return "headless"
return None

Expand Down
6 changes: 4 additions & 2 deletions setupext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,10 @@ def get_extensions(self):
# c_internal_utils
ext = Extension(
"matplotlib._c_internal_utils", ["src/_c_internal_utils.c"],
libraries=({"win32": ["ole32", "shell32", "user32"]}
.get(sys.platform, [])))
libraries=({
"linux": ["dl"],
"win32": ["ole32", "shell32", "user32"],
}.get(sys.platform, [])))
yield ext
# contour
ext = Extension(
Expand Down
75 changes: 70 additions & 5 deletions src/_c_internal_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,68 @@
#define PY_SSIZE_T_CLEAN
#include <Python.h>
#ifdef __linux__
#include <dlfcn.h>
#endif
#ifdef _WIN32
#include <Objbase.h>
#include <Shobjidl.h>
#include <Windows.h>
#endif

static PyObject* mpl_GetCurrentProcessExplicitAppUserModelID(PyObject* module)
static PyObject*
mpl_display_is_valid(PyObject* module)
{
#ifdef __linux__
Copy link

@mstoeckl mstoeckl Aug 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think it would still make sense to check that environment variables DISPLAY, or WAYLAND_DISPLAY exist before hand. First, to avoid libwayland's behavior of defaulting to connect to 'wayland-0', even if no environment variables are set; second, because dlopen is very slow -- one one computer, calling dlopen followed by dlclose on libX11.so and libwayland-client.so costs between 1.4 msec (marginal cost), 2 msec (initial cost, hot cache), and 6 msec (uncached, initial cost). Importing matplotlib already costs me 240msec every script load. (Aside: It's not necessary to check env variable WAYLAND_SOCKET, because the socket named by it cannot be reused over two successive wl_display_connect calls.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you check the speed of getenv()? (I genuinely do not know whether it is much faster than dlopen().)

Copy link

@mstoeckl mstoeckl Aug 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

getenv is very fast, since the environment variables are provided to a program at the start, just like the command line arguments. My benchmark of getenv uses around 0.0002 msec to call both getenv("WAYLAND_DISPLAY") and getenv("DISPLAY").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, then.

void* libX11;
// The getenv check is redundant but helps performance as it is much faster
// than dlopen().
if (getenv("DISPLAY")
&& (libX11 = dlopen("libX11.so.6", RTLD_LAZY))) {
struct Display* display = NULL;
struct Display* (* XOpenDisplay)(char const*) =
dlsym(libX11, "XOpenDisplay");
int (* XCloseDisplay)(struct Display*) =
dlsym(libX11, "XCloseDisplay");
if (XOpenDisplay && XCloseDisplay
&& (display = XOpenDisplay(NULL))) {
XCloseDisplay(display);
}
if (dlclose(libX11)) {
PyErr_SetString(PyExc_RuntimeError, dlerror());
return NULL;
}
if (display) {
Py_RETURN_TRUE;
}
}
void* libwayland_client;
if (getenv("WAYLAND_DISPLAY")
&& (libwayland_client = dlopen("libwayland-client.so.0", RTLD_LAZY))) {
struct wl_display* display = NULL;
struct wl_display* (* wl_display_connect)(char const*) =
dlsym(libwayland_client, "wl_display_connect");
void (* wl_display_disconnect)(struct wl_display*) =
dlsym(libwayland_client, "wl_display_disconnect");
if (wl_display_connect && wl_display_disconnect
&& (display = wl_display_connect(NULL))) {
wl_display_disconnect(display);
}
if (dlclose(libwayland_client)) {
PyErr_SetString(PyExc_RuntimeError, dlerror());
return NULL;
}
if (display) {
Py_RETURN_TRUE;
}
}
Py_RETURN_FALSE;
#else
Py_RETURN_TRUE;
#endif
}

static PyObject*
mpl_GetCurrentProcessExplicitAppUserModelID(PyObject* module)
{
#ifdef _WIN32
wchar_t* appid = NULL;
Expand All @@ -22,7 +78,8 @@ static PyObject* mpl_GetCurrentProcessExplicitAppUserModelID(PyObject* module)
#endif
}

static PyObject* mpl_SetCurrentProcessExplicitAppUserModelID(PyObject* module, PyObject* arg)
static PyObject*
mpl_SetCurrentProcessExplicitAppUserModelID(PyObject* module, PyObject* arg)
{
#ifdef _WIN32
wchar_t* appid = PyUnicode_AsWideCharString(arg, NULL);
Expand All @@ -40,7 +97,8 @@ static PyObject* mpl_SetCurrentProcessExplicitAppUserModelID(PyObject* module, P
#endif
}

static PyObject* mpl_GetForegroundWindow(PyObject* module)
static PyObject*
mpl_GetForegroundWindow(PyObject* module)
{
#ifdef _WIN32
return PyLong_FromVoidPtr(GetForegroundWindow());
Expand All @@ -49,7 +107,8 @@ static PyObject* mpl_GetForegroundWindow(PyObject* module)
#endif
}

static PyObject* mpl_SetForegroundWindow(PyObject* module, PyObject *arg)
static PyObject*
mpl_SetForegroundWindow(PyObject* module, PyObject *arg)
{
#ifdef _WIN32
HWND handle = PyLong_AsVoidPtr(arg);
Expand All @@ -66,6 +125,12 @@ static PyObject* mpl_SetForegroundWindow(PyObject* module, PyObject *arg)
}

static PyMethodDef functions[] = {
{"display_is_valid", (PyCFunction)mpl_display_is_valid, METH_NOARGS,
"display_is_valid()\n--\n\n"
"Check whether the current X11 or Wayland display is valid.\n\n"
"On Linux, returns True if either $DISPLAY is set and XOpenDisplay(NULL)\n"
"succeeds, or $WAYLAND_DISPLAY is set and wl_display_connect(NULL)\n"
"succeeds. On other platforms, always returns True."},
{"Win32_GetCurrentProcessExplicitAppUserModelID",
(PyCFunction)mpl_GetCurrentProcessExplicitAppUserModelID, METH_NOARGS,
"Win32_GetCurrentProcessExplicitAppUserModelID()\n--\n\n"
Expand All @@ -83,7 +148,7 @@ static PyMethodDef functions[] = {
"always returns None."},
{"Win32_SetForegroundWindow",
(PyCFunction)mpl_SetForegroundWindow, METH_O,
"Win32_SetForegroundWindow(hwnd)\n--\n\n"
"Win32_SetForegroundWindow(hwnd, /)\n--\n\n"
Copy link
Member

Choose a reason for hiding this comment

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

What is the slash doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This marks hwnd as being positional-only (https://www.python.org/dev/peps/pep-0570/), which it is (being implemented with METH_O).

"Wrapper for Windows' SetForegroundWindow. On non-Windows platforms, \n"
"a no-op."},
{NULL, NULL}}; // sentinel.
Expand Down