Improve testing and comments

Author: jholveck

CHANGELOG.md

@@ -5,6 +5,7 @@ See Git checking messages for full history.
 ## 10.1.1.dev0 (2025-xx-xx)
 - Linux: check the server for Xrandr support version (#417)
 - Linux: improve typing and error messages for X libraries (#418)
+- Linux: add a new XCB backend for better thread safety, error-checking, and future development (#425)
 - :heart: contributors: @jholveck
 
 ## 10.1.0 (2025-08-16)

src/mss/linux/__init__.py

@@ -4,8 +4,7 @@
 from mss.exception import ScreenShotError
 
 
-# This factory function is named in upper-case for backwards compatibility.
-def MSS(backend: str = "default", **kwargs: Any) -> MSSBase:  # noqa: N802
+def mss(backend: str = "default", **kwargs: Any) -> MSSBase:
     backend = backend.lower()
     if backend in {"default", "xlib"}:
         from . import xlib  # noqa: PLC0415
@@ -17,3 +16,8 @@ def MSS(backend: str = "default", **kwargs: Any) -> MSSBase:  # noqa: N802
         return xgetimage.MSS(**kwargs)
     msg = f"Backend {backend!r} not (yet?) implemented."
     raise ScreenShotError(msg)
+
+
+# Alias in upper-case for backward compatibility.  This is a supported name in the docs.
+def MSS(*args, **kwargs) -> MSSBase:  # type: ignore[no-untyped-def] # noqa: N802, ANN002, ANN003
+    return mss(*args, **kwargs)

src/mss/linux/xcb.py

@@ -120,18 +120,23 @@
 }
 
 
+def initialize() -> None:
+    LIB.initialize(callbacks=[xcbgen.initialize])
+
+
 def connect(display: str | bytes | None = None) -> tuple[Connection, int]:
     if isinstance(display, str):
         display = display.encode("utf-8")
 
-    LIB.initialize(callbacks=[xcbgen.initialize])
-
+    initialize()
     pref_screen_num = c_int()
     conn_p = LIB.xcb.xcb_connect(display, pref_screen_num)
 
     # We still get a connection object even if the connection fails.
     conn_err = LIB.xcb.xcb_connection_has_error(conn_p)
     if conn_err != 0:
+        # XCB won't free its connection structures until we disconnect, even in the event of an error.
+        LIB.xcb.xcb_disconnect(conn_p)
         msg = "Cannot connect to display: "
         conn_errmsg = XCB_CONN_ERRMSG.get(conn_err)
         if conn_errmsg:

src/mss/linux/xcbhelpers.py

@@ -35,13 +35,11 @@ def __init__(self, /, **kwargs: Any) -> None:  # noqa: PLR0912
         self.conn: xcb.Connection | None
         self.conn, pref_screen_num = xcb.connect(display)
 
-        # Let XCB pre-populate its internal cache regarding the
-        # extensions we might use, while we finish setup.
+        # Let XCB pre-populate its internal cache regarding the extensions we might use, while we finish setup.
         LIB.xcb.xcb_prefetch_extension_data(self.conn, LIB.randr_id)
         LIB.xcb.xcb_prefetch_extension_data(self.conn, LIB.xfixes_id)
 
-        # Get the connection setup information that was included when we
-        # connected.
+        # Get the connection setup information that was included when we connected.
         xcb_setup = LIB.xcb.xcb_get_setup(self.conn).contents
         screens = xcb.setup_roots(xcb_setup)
         pref_screen = screens[pref_screen_num]
@@ -128,8 +126,7 @@ def _monitors_impl(self) -> None:
             msg = "Cannot identify monitors while the connection is closed"
             raise ScreenShotError(msg)
 
-        # The first entry is the whole X11 screen that the root is
-        # on.  That's the one that covers all the monitors.
+        # The first entry is the whole X11 screen that the root is on.  That's the one that covers all the monitors.
         root_geom = xcb.get_geometry(self.conn, self.root)
         self._monitors.append(
             {
@@ -140,52 +137,47 @@ def _monitors_impl(self) -> None:
             }
         )
 
-        # After that, we have one for each monitor on that X11 screen.
-        # For decades, that's been handled by Xrandr.  We don't
-        # presently try to work with Xinerama.  So, we're going to
-        # check the different outputs, according to Xrandr.  If that
-        # fails, we'll just leave the one root covering everything.
+        # After that, we have one for each monitor on that X11 screen.  For decades, that's been handled by Xrandr.
+        # We don't presently try to work with Xinerama.  So, we're going to check the different outputs, according to
+        # Xrandr.  If that fails, we'll just leave the one root covering everything.
 
-        # Make sure we have the Xrandr extension we need.  This will
-        # query the cache that we started populating in __init__.
+        # Make sure we have the Xrandr extension we need.  This will query the cache that we started populating in
+        # __init__.
         randr_ext_data = LIB.xcb.xcb_get_extension_data(self.conn, LIB.randr_id).contents
         if not randr_ext_data.present:
             return
 
-        # We ask the server to give us anything up to the version we
-        # support (i.e., what we expect the reply structs to look
-        # like).  If the server only supports 1.2, then that's what
-        # it'll give us, and we're ok with that, but we also use a
-        # faster path if the server implements at least 1.3.
+        # We ask the server to give us anything up to the version we support (i.e., what we expect the reply structs
+        # to look like).  If the server only supports 1.2, then that's what it'll give us, and we're ok with that, but
+        # we also use a faster path if the server implements at least 1.3.
         randr_version_data = xcb.randr_query_version(self.conn, xcb.RANDR_MAJOR_VERSION, xcb.RANDR_MINOR_VERSION)
         randr_version = (randr_version_data.major_version, randr_version_data.minor_version)
         if randr_version < (1, 2):
             return
 
         screen_resources: xcb.RandrGetScreenResourcesReply | xcb.RandrGetScreenResourcesCurrentReply
-        # Check to see if we have the xcb_randr_get_screen_resources_current
-        # function in libxcb-randr, and that the server supports it.
+        # Check to see if we have the xcb_randr_get_screen_resources_current function in libxcb-randr, and that the
+        # server supports it.
         if hasattr(LIB.randr, "xcb_randr_get_screen_resources_current") and randr_version >= (1, 3):
             screen_resources = xcb.randr_get_screen_resources_current(self.conn, self.drawable.value)
             crtcs = xcb.randr_get_screen_resources_current_crtcs(screen_resources)
         else:
-            # Either the client or the server doesn't support the _current
-            # form.  That's ok; we'll use the old function, which forces
-            # a new query to the physical monitors.
+            # Either the client or the server doesn't support the _current form.  That's ok; we'll use the old
+            # function, which forces a new query to the physical monitors.
             screen_resources = xcb.randr_get_screen_resources(self.conn, self.drawable)
             crtcs = xcb.randr_get_screen_resources_crtcs(screen_resources)
 
         for crtc in crtcs:
-            crtc_info = xcb.randr_get_crtc_info(self.conn, crtc, screen_resources.timestamp)
+            crtc_info = xcb.randr_get_crtc_info(self.conn, crtc, screen_resources.config_timestamp)
             if crtc_info.num_outputs == 0:
                 continue
             self._monitors.append(
                 {"left": crtc_info.x, "top": crtc_info.y, "width": crtc_info.width, "height": crtc_info.height}
             )
 
         # Extra credit would be to enumerate the virtual desktops; see
-        # https://specifications.freedesktop.org/wm/latest/ar01s03.html
-        # But I don't know how widely-used that style is.
+        # https://specifications.freedesktop.org/wm/latest/ar01s03.html.  But I don't know how widely-used that style
+        # is.
 
     def _grab_impl(self, monitor: Monitor, /) -> ScreenShot:
         """Retrieve all pixels from a monitor. Pixels have to be RGBX."""
@@ -205,11 +197,9 @@ def _grab_impl(self, monitor: Monitor, /) -> ScreenShot:
             ALL_PLANES,
         )
 
-        # Now, save the image.  This is a reference into the img_reply
-        # structure.
+        # Now, save the image.  This is a reference into the img_reply structure.
         img_data_arr = xcb.get_image_data(img_reply)
-        # Copy this into a new bytearray, so that it will persist after
-        # we clear the image structure.
+        # Copy this into a new bytearray, so that it will persist after we clear the image structure.
         img_data = bytearray(img_data_arr)
 
         if img_reply.depth != self.drawable_depth or img_reply.visual.value != self.drawable_visual_id:
@@ -233,9 +223,8 @@ def _cursor_impl_check_xfixes(self) -> bool:
             return False
 
         reply = xcb.xfixes_query_version(self.conn, xcb.XFIXES_MAJOR_VERSION, xcb.XFIXES_MINOR_VERSION)
-        # We can work with 2.0 and later, but not sure about the
-        # actual minimum version we can use.  That's ok; everything
-        # these days is much more modern.
+        # We can work with 2.0 and later, but not sure about the actual minimum version we can use.  That's ok;
+        # everything these days is much more modern.
         return (reply.major_version, reply.minor_version) >= (2, 0)
 
     def _cursor_impl(self) -> ScreenShot:
@@ -261,9 +250,7 @@ def _cursor_impl(self) -> ScreenShot:
 
         data_arr = xcb.xfixes_get_cursor_image_cursor_image(cursor_img)
         data = bytearray(data_arr)
-        # We don't need to do the same array slice-and-dice work as
-        # the Xlib-based implementation: Xlib has an unfortunate
-        # historical accident that makes it have to return the cursor
-        # image in a different format.
+        # We don't need to do the same array slice-and-dice work as the Xlib-based implementation: Xlib has an
+        # unfortunate historical accident that makes it have to return the cursor image in a different format.
 
         return self.cls_image(data, region)

src/mss/linux/xgetimage.py

@@ -152,9 +152,19 @@ def test_xrandr_extension_exists_but_is_not_enabled(display: str) -> None:
 
 
 def test_unsupported_depth(backend: str) -> None:
+    # 8-bit is normally PseudoColor.  If the order of testing the display support changes, this might raise a
+    # different message; just change the match= accordingly.
     with (
         pyvirtualdisplay.Display(size=(WIDTH, HEIGHT), color_depth=8) as vdisplay,
-        pytest.raises(ScreenShotError),
+        pytest.raises(ScreenShotError, match=r"\b8\b"),
+        mss.mss(display=vdisplay.new_display_var, backend=backend) as sct,
+    ):
+        sct.grab(sct.monitors[1])
+
+    # 16-bit is normally TrueColor, but still just 16 bits.
+    with (
+        pyvirtualdisplay.Display(size=(WIDTH, HEIGHT), color_depth=16) as vdisplay,
+        pytest.raises(ScreenShotError, match=r"\b16\b"),
         mss.mss(display=vdisplay.new_display_var, backend=backend) as sct,
     ):
         sct.grab(sct.monitors[1])

src/tests/test_gnu_linux.py

@@ -97,6 +97,7 @@ def test_sdist() -> None:
         f"mss-{__version__}/src/tests/test_setup.py",
         f"mss-{__version__}/src/tests/test_tools.py",
         f"mss-{__version__}/src/tests/test_windows.py",
+        f"mss-{__version__}/src/tests/test_xcb.py",
         f"mss-{__version__}/src/tests/third_party/__init__.py",
         f"mss-{__version__}/src/tests/third_party/test_numpy.py",
         f"mss-{__version__}/src/tests/third_party/test_pil.py",