xcb screengrab fixes

Tests/test_imagegrab.py

@@ -2,13 +2,15 @@ import subprocess
 import sys
 
 import pytest
+from PIL import Image, ImageGrab
 
 from .helper import assert_image
 
-try:
-    from PIL import ImageGrab
 
 class TestImageGrab:
+    @pytest.mark.skipif(
+        sys.platform not in ("win32", "darwin"), reason="requires Windows or macOS"
+    )
     def test_grab(self):
         for im in [
             ImageGrab.grab(),
@@ -20,13 +22,38 @@ try:
         im = ImageGrab.grab(bbox=(10, 20, 50, 80))
         assert_image(im, im.mode, (40, 60))
 
+    @pytest.mark.skipif(not Image.core.HAVE_XCB, reason="requires XCB")
+    def test_grab_x11(self):
+        try:
+            if sys.platform not in ("win32", "darwin"):
+                im = ImageGrab.grab()
+                assert_image(im, im.mode, im.size)
+
+            im2 = ImageGrab.grab(xdisplay="")
+            assert_image(im2, im2.mode, im2.size)
+        except IOError as e:
+            pytest.skip(str(e))
+
+    @pytest.mark.skipif(not Image.core.HAVE_XCB, reason="requires XCB")
+    def test_grab_invalid_xdisplay(self):
+        exception = None
+
+        try:
+            ImageGrab.grab(xdisplay="error.test:0.0")
+        except Exception as e:
+            exception = e
+
+        assert isinstance(exception, IOError)
+        assert str(exception).startswith("X connection failed")
+
+    @pytest.mark.skipif(
+        sys.platform not in ("win32", "darwin"), reason="requires Windows or macOS"
+    )
     def test_grabclipboard(self):
         if sys.platform == "darwin":
             subprocess.call(["screencapture", "-cx"])
-            else:
+        elif sys.platform == "win32":
-                p = subprocess.Popen(
+            p = subprocess.Popen(["powershell", "-command", "-"], stdin=subprocess.PIPE)
-                    ["powershell", "-command", "-"], stdin=subprocess.PIPE
-                )
             p.stdin.write(
                 b"""[Reflection.Assembly]::LoadWithPartialName("System.Drawing")
 [Reflection.Assembly]::LoadWithPartialName("System.Windows.Forms")
@@ -34,35 +61,9 @@ $bmp = New-Object Drawing.Bitmap 200, 200
 [Windows.Forms.Clipboard]::SetImage($bmp)"""
             )
             p.communicate()
+        else:
+            pytest.skip("ImageGrab.grabclipboard() is macOS and Windows only")
+            return
 
         im = ImageGrab.grabclipboard()
         assert_image(im, im.mode, im.size)
-
-
-except ImportError:
-
-    class TestImageGrab:
-        @pytest.mark.skip(reason="ImageGrab ImportError")
-        def test_skip(self):
-            pass
-
-
-class TestImageGrabImport:
-    def test_import(self):
-        # Arrange
-        exception = None
-
-        # Act
-        try:
-            from PIL import ImageGrab
-
-            ImageGrab.__name__  # dummy to prevent Pyflakes warning
-        except Exception as e:
-            exception = e
-
-        # Assert
-        if sys.platform in ["win32", "darwin"]:
-            assert exception is None
-        else:
-            assert isinstance(exception, ImportError)
-            assert str(exception) == "ImageGrab is macOS and Windows only"

setup.py

@@ -823,6 +823,7 @@ class pil_build_ext(build_ext):
             (feature.lcms, "LITTLECMS2"),
             (feature.webp, "WEBP"),
             (feature.webpmux, "WEBPMUX"),
+            (feature.xcb, "XCB (X protocol)"),
         ]
 
         all = 1

src/PIL/ImageGrab.py

@@ -22,12 +22,12 @@ import tempfile
 
 from . import Image
 
-if sys.platform == "win32":
+if sys.platform == "darwin":
-    pass
-elif sys.platform == "darwin":
     import os
     import tempfile
     import subprocess
+elif sys.platform == "win32":
+    pass
 elif not Image.core.HAVE_XCB:
     raise ImportError("ImageGrab requires Windows, macOS, or the XCB library")
 
@@ -65,17 +65,9 @@ def grab(bbox=None, include_layered_windows=False, all_screens=False, xdisplay=N
             return im
     # use xdisplay=None for default display on non-win32/macOS systems
     if not Image.core.HAVE_XCB:
-        raise OSError("XCB support not included")
+        raise AttributeError("XCB support not present")
     size, data = Image.core.grabscreen_x11(xdisplay)
-    im = Image.frombytes(
+    im = Image.frombytes("RGB", size, data, "raw", "BGRX", size[0] * 4, 1)
-        "RGB",
-        size,
-        data,
-        "raw",
-        "BGRX",
-        size[0] * 4,
-        1,
-    )
     if bbox:
         im = im.crop(bbox)
     return im

src/display.c

@@ -845,8 +845,8 @@ PyImaging_GrabScreenX11(PyObject* self, PyObject* args)
 
     connection = xcb_connect(display_name, &screen_number);
     if (xcb_connection_has_error(connection)) {
+        PyErr_Format(PyExc_IOError, "X connection failed: error %i", xcb_connection_has_error(connection));
         xcb_disconnect(connection);
-        PyErr_SetString(PyExc_IOError, "X connection failed");
         return NULL;
     }
 
@@ -857,7 +857,8 @@ PyImaging_GrabScreenX11(PyObject* self, PyObject* args)
             break;
         }
     }
-    if (screen == NULL) {
+    if (screen == NULL || screen->root == 0) {
+        // this case is usually caught with "X connection failed: error 6" above
         xcb_disconnect(connection);
         PyErr_SetString(PyExc_IOError, "X screen not found");
         return NULL;
@@ -873,16 +874,21 @@ PyImaging_GrabScreenX11(PyObject* self, PyObject* args)
                                               0, 0, width, height, 0x00ffffff),
                                 &error);
     if (reply == NULL) {
+        PyErr_Format(PyExc_IOError, "X get_image failed: error %i (%i, %i, %i)",
+                     error->error_code, error->major_code, error->minor_code, error->resource_id);
         free(error);
         xcb_disconnect(connection);
-        PyErr_SetString(PyExc_IOError, "X get_image failed");
         return NULL;
     }
 
     /* store data in Python buffer */
 
+    if (reply->depth == 24) {
         buffer = PyBytes_FromStringAndSize((char*)xcb_get_image_data(reply),
                                            xcb_get_image_data_length(reply));
+    } else {
+        PyErr_Format(PyExc_IOError, "usupported bit depth: %i", reply->depth);
+    }
 
     free(reply);
     xcb_disconnect(connection);