XCB: merge GetWindowSize() into GetWindow()

This saves one geometry request and some code.

modules/video_output/xcb/common.c

@@ -41,125 +41,6 @@
 
 #include "xcb_vlc.h"
 
-/**
- * Connect to the X server.
- */
-static xcb_connection_t *Connect (vlc_object_t *obj, const char *display)
-{
-    xcb_connection_t *conn = xcb_connect (display, NULL);
-    if (xcb_connection_has_error (conn) /*== NULL*/)
-    {
-        msg_Err (obj, "cannot connect to X server (%s)",
-                 display ? display : "default");
-        xcb_disconnect (conn);
-        return NULL;
-    }
-
-    const xcb_setup_t *setup = xcb_get_setup (conn);
-    msg_Dbg (obj, "connected to X%"PRIu16".%"PRIu16" server",
-             setup->protocol_major_version, setup->protocol_minor_version);
-    char *vendor = strndup (xcb_setup_vendor (setup), setup->vendor_len);
-    if (vendor)
-    {
-        msg_Dbg (obj, " vendor : %s", vendor);
-        free (vendor);
-    }
-    msg_Dbg (obj, " version: %"PRIu32, setup->release_number);
-    return conn;
-}
-
-/**
- * Find screen matching a given root window.
- */
-static const xcb_screen_t *FindScreen (vlc_object_t *obj,
-                                       xcb_connection_t *conn,
-                                       xcb_window_t root)
-{
-    /* Find the selected screen */
-    const xcb_setup_t *setup = xcb_get_setup (conn);
-    const xcb_screen_t *screen = NULL;
-    for (xcb_screen_iterator_t i = xcb_setup_roots_iterator (setup);
-         i.rem > 0 && screen == NULL; xcb_screen_next (&i))
-    {
-        if (i.data->root == root)
-            screen = i.data;
-    }
-
-    if (screen == NULL)
-    {
-        msg_Err (obj, "parent window screen not found");
-        return NULL;
-    }
-    msg_Dbg (obj, "using screen 0x%"PRIx32, root);
-    return screen;
-}
-
-static const xcb_screen_t *FindWindow (vlc_object_t *obj,
-                                       xcb_connection_t *conn,
-                                       xcb_window_t xid,
-                                       uint8_t *restrict pdepth)
-{
-    xcb_get_geometry_reply_t *geo =
-        xcb_get_geometry_reply (conn, xcb_get_geometry (conn, xid), NULL);
-    if (geo == NULL)
-    {
-        msg_Err (obj, "parent window not valid");
-        return NULL;
-    }
-
-    const xcb_screen_t *screen = FindScreen (obj, conn, geo->root);
-    *pdepth = geo->depth;
-    free (geo);
-    return screen;
-}
-
-
-/**
- * Create a VLC video X window object, connect to the corresponding X server,
- * find the corresponding X server screen.
- */
-vout_window_t *GetWindow (vout_display_t *vd,
-                          xcb_connection_t **restrict pconn,
-                          const xcb_screen_t **restrict pscreen,
-                          uint8_t *restrict pdepth)
-{
-    /* Get window */
-    vout_window_cfg_t wnd_cfg;
-
-    memset( &wnd_cfg, 0, sizeof(wnd_cfg) );
-    wnd_cfg.type = VOUT_WINDOW_TYPE_XID;
-    wnd_cfg.x = var_InheritInteger (vd, "video-x");
-    wnd_cfg.y = var_InheritInteger (vd, "video-y");
-    wnd_cfg.width  = vd->cfg->display.width;
-    wnd_cfg.height = vd->cfg->display.height;
-
-    vout_window_t *wnd = vout_display_NewWindow (vd, &wnd_cfg);
-    if (wnd == NULL)
-    {
-        msg_Err (vd, "parent window not available");
-        return NULL;
-    }
-
-    xcb_connection_t *conn = Connect (VLC_OBJECT(vd), wnd->display.x11);
-    if (conn == NULL)
-        goto error;
-    *pconn = conn;
-
-    *pscreen = FindWindow (VLC_OBJECT(vd), conn, wnd->handle.xid, pdepth);
-    if (*pscreen == NULL)
-    {
-        xcb_disconnect (conn);
-        goto error;
-    }
-
-    RegisterMouseEvents (VLC_OBJECT(vd), conn, wnd->handle.xid);
-    return wnd;
-
-error:
-    vout_display_DeleteWindow (vd, wnd);
-    return NULL;
-}
-
 /** Check MIT-SHM shared memory support */
 bool CheckSHM (vlc_object_t *obj, xcb_connection_t *conn)
 {

modules/video_output/xcb/events.c

@@ -56,39 +56,30 @@ int CheckError (vout_display_t *vd, xcb_connection_t *conn,
 }
 
 /**
- * Gets the size of an X window.
+ * Connect to the X server.
  */
-int GetWindowSize (struct vout_window_t *wnd, xcb_connection_t *conn,
-                   unsigned *restrict width, unsigned *restrict height)
+static xcb_connection_t *Connect (vlc_object_t *obj, const char *display)
 {
-    xcb_get_geometry_cookie_t ck = xcb_get_geometry (conn, wnd->handle.xid);
-    xcb_get_geometry_reply_t *geo = xcb_get_geometry_reply (conn, ck, NULL);
-
-    if (!geo)
-        return -1;
-
-    *width = geo->width;
-    *height = geo->height;
-    free (geo);
-    return 0;
-}
-
-/**
- * Create a blank cursor.
- * Note that the pixmaps are leaked (until the X disconnection). Hence, this
- * function should be called no more than once per X connection.
- * @param conn XCB connection
- * @param scr target XCB screen
- */
-xcb_cursor_t CreateBlankCursor (xcb_connection_t *conn,
-                                const xcb_screen_t *scr)
-{
-    xcb_cursor_t cur = xcb_generate_id (conn);
-    xcb_pixmap_t pix = xcb_generate_id (conn);
+    xcb_connection_t *conn = xcb_connect (display, NULL);
+    if (xcb_connection_has_error (conn) /*== NULL*/)
+    {
+        msg_Err (obj, "cannot connect to X server (%s)",
+                 (display != NULL) ? display : "default");
+        xcb_disconnect (conn);
+        return NULL;
+    }
 
-    xcb_create_pixmap (conn, 1, pix, scr->root, 1, 1);
-    xcb_create_cursor (conn, cur, pix, pix, 0, 0, 0, 0, 0, 0, 1, 1);
-    return cur;
+    const xcb_setup_t *setup = xcb_get_setup (conn);
+    msg_Dbg (obj, "connected to X%"PRIu16".%"PRIu16" server",
+             setup->protocol_major_version, setup->protocol_minor_version);
+    char *vendor = strndup (xcb_setup_vendor (setup), setup->vendor_len);
+    if (vendor)
+    {
+        msg_Dbg (obj, " vendor : %s", vendor);
+        free (vendor);
+    }
+    msg_Dbg (obj, " version: %"PRIu32, setup->release_number);
+    return conn;
 }
 
 /**
@@ -112,6 +103,107 @@ void RegisterMouseEvents (vlc_object_t *obj, xcb_connection_t *conn,
     }
 }
 
+/**
+ * Find screen matching a given root window.
+ */
+static const xcb_screen_t *FindScreen (vlc_object_t *obj,
+                                       xcb_connection_t *conn,
+                                       xcb_window_t root)
+{
+    /* Find the selected screen */
+    const xcb_setup_t *setup = xcb_get_setup (conn);
+    for (xcb_screen_iterator_t i = xcb_setup_roots_iterator (setup);
+         i.rem > 0; xcb_screen_next (&i))
+    {
+        if (i.data->root == root)
+        {
+            msg_Dbg (obj, "using screen 0x%"PRIx32, root);
+            return i.data;
+        }
+    }
+    msg_Err (obj, "window screen not found");
+    return NULL;
+}
+
+/**
+ * Create a VLC video X window object, connect to the corresponding X server,
+ * find the corresponding X server screen.
+ */
+vout_window_t *GetWindow (vout_display_t *vd,
+                          xcb_connection_t **restrict pconn,
+                          const xcb_screen_t **restrict pscreen,
+                          uint8_t *restrict pdepth,
+                          uint16_t *restrict pwidth,
+                          uint16_t *restrict pheight)
+{
+    vout_window_cfg_t cfg = {
+        .type = VOUT_WINDOW_TYPE_XID,
+        .x = var_InheritInteger (vd, "video-x"),
+        .y = var_InheritInteger (vd, "video-y"),
+        .width  = vd->cfg->display.width,
+        .height = vd->cfg->display.height,
+    };
+
+    vout_window_t *wnd = vout_display_NewWindow (vd, &cfg);
+    if (wnd == NULL)
+    {
+        msg_Err (vd, "window not available");
+        return NULL;
+    }
+
+    xcb_connection_t *conn = Connect (VLC_OBJECT(vd), wnd->display.x11);
+    if (conn == NULL)
+        goto error;
+    *pconn = conn;
+
+    /* Events must be registered before the window geometry is queried, so as
+     * to avoid missing impeding resize events. */
+    RegisterMouseEvents (VLC_OBJECT(vd), conn, wnd->handle.xid);
+
+    xcb_get_geometry_reply_t *geo =
+        xcb_get_geometry_reply (conn, xcb_get_geometry (conn, wnd->handle.xid),
+                                NULL);
+    if (geo == NULL)
+    {
+        msg_Err (vd, "window not valid");
+        goto error;
+    }
+    *pdepth = geo->depth;
+    *pwidth = geo->width;
+    *pheight = geo->height;
+
+    const xcb_screen_t *screen = FindScreen (VLC_OBJECT(vd), conn, geo->root);
+    free (geo);
+    if (screen == NULL)
+        goto error;
+    *pscreen = screen;
+    return wnd;
+
+error:
+    if (conn != NULL)
+        xcb_disconnect (conn);
+    vout_display_DeleteWindow (vd, wnd);
+    return NULL;
+}
+
+/**
+ * Create a blank cursor.
+ * Note that the pixmaps are leaked (until the X disconnection). Hence, this
+ * function should be called no more than once per X connection.
+ * @param conn XCB connection
+ * @param scr target XCB screen
+ */
+xcb_cursor_t CreateBlankCursor (xcb_connection_t *conn,
+                                const xcb_screen_t *scr)
+{
+    xcb_cursor_t cur = xcb_generate_id (conn);
+    xcb_pixmap_t pix = xcb_generate_id (conn);
+
+    xcb_create_pixmap (conn, 1, pix, scr->root, 1, 1);
+    xcb_create_cursor (conn, cur, pix, pix, 0, 0, 0, 0, 0, 0, 1, 1);
+    return cur;
+}
+
 /* NOTE: we assume no other thread will be _setting_ our video output events
  * variables. Afterall, only this plugin is supposed to know when these occur.
   * Otherwise, we'd var_OrInteger() and var_NandInteger() functions...

modules/video_output/xcb/x11.c

@@ -115,7 +115,8 @@ static int Open (vlc_object_t *obj)
     /* Get window, connect to X server */
     xcb_connection_t *conn;
     const xcb_screen_t *scr;
-    sys->embed = GetWindow (vd, &conn, &scr, &(uint8_t){ 0 });
+    uint16_t width, height;
+    sys->embed = GetWindow (vd, &conn, &scr, &(uint8_t){ 0 }, &width, &height);
     if (sys->embed == NULL)
     {
         free (sys);
@@ -249,10 +250,6 @@ found_format:;
         cmap = scr->default_colormap;
 
     /* Create window */
-    unsigned width, height;
-    if (GetWindowSize (sys->embed, conn, &width, &height))
-        goto error;
-
     sys->window = xcb_generate_id (conn);
     sys->gc = xcb_generate_id (conn);
     xcb_pixmap_t pixmap = xcb_generate_id (conn);

modules/video_output/xcb/xcb_vlc.h

@@ -42,12 +42,9 @@ void DestroyKeyHandler (key_handler_t *);
 int ProcessKeyEvent (key_handler_t *, xcb_generic_event_t *);
 
 /* common.c */
-struct vout_window_t *GetWindow (vout_display_t *obj,
-                                 xcb_connection_t **restrict pconn,
-                                 const xcb_screen_t **restrict pscreen,
-                                 uint8_t *restrict pdepth);
-int GetWindowSize (struct vout_window_t *wnd, xcb_connection_t *conn,
-                   unsigned *restrict width, unsigned *restrict height);
+struct vout_window_t *GetWindow (vout_display_t *obj, xcb_connection_t **,
+                                 const xcb_screen_t **, uint8_t *depth,
+                                 uint16_t *width, uint16_t *height);
 bool CheckSHM (vlc_object_t *obj, xcb_connection_t *conn);
 xcb_cursor_t CreateBlankCursor (xcb_connection_t *, const xcb_screen_t *);
 void RegisterMouseEvents (vlc_object_t *, xcb_connection_t *, xcb_window_t);

modules/video_output/xcb/xvideo.c

@@ -386,8 +386,9 @@ static int Open (vlc_object_t *obj)
     /* Connect to X */
     xcb_connection_t *conn;
     const xcb_screen_t *screen;
+    uint16_t width, height;
     uint8_t depth;
-    p_sys->embed = GetWindow (vd, &conn, &screen, &depth);
+    p_sys->embed = GetWindow (vd, &conn, &screen, &depth, &width, &height);
     if (p_sys->embed == NULL)
     {
         free (p_sys);
@@ -597,9 +598,7 @@ static int Open (vlc_object_t *obj)
     if (is_fullscreen && vout_window_SetFullScreen (p_sys->embed, true))
         is_fullscreen = false;
     vout_display_SendEventFullscreen (vd, is_fullscreen);
-    unsigned width, height;
-    if (!GetWindowSize (p_sys->embed, conn, &width, &height))
-        vout_display_SendEventDisplaySize (vd, width, height, is_fullscreen);
+    vout_display_SendEventDisplaySize (vd, width, height, is_fullscreen);
 
     return VLC_SUCCESS;