x11: simplify handling of X Visuals and Colormaps in VOs

Don't force VOs to pick an arbitrary default Visual and Colormap. They
still can override them if needed. This simplifies the X11 VO interface.

Always create a Colormap for simplicity. Using CopyFromParent fails if
the selected visual is not the same of that of the parent window, which
happens for me with vo_opengl.

vo_vdpau and vo_xv explicitly set CWBorderPixel, do that in x11_common
instead (it was already done for native windows, but not for slave mode
windows).

What gl_common did was incorrect in theory: freeing a colormap while a
window uses it will change the colormap of the window to "None", and
the color mapping for such windows is "undefined".

6 files changed, 71 insertions, 101 deletions

diff --git a/video/out/gl_common.c b/video/out/gl_common.c
index 887195bf18..38bd89b21d 100644
--- a/video/out/gl_common.c
+++ b/video/out/gl_common.c
@@ -1089,11 +1089,8 @@ static bool create_glx_window(struct MPGLContext *ctx, uint32_t d_width,
     if (glx_ctx->context) {
         // GL context and window already exist.
         // Only update window geometry etc.
-        Colormap colormap = XCreateColormap(vo->x11->display, vo->x11->rootwin,
-                                            glx_ctx->vinfo->visual, AllocNone);
         vo_x11_create_vo_window(vo, glx_ctx->vinfo, vo->dx, vo->dy, d_width,
-                                d_height, flags, colormap, "gl");
-        XFreeColormap(vo->x11->display, colormap);
+                                d_height, flags, "gl");
         return true;
     }
 
@@ -1144,11 +1141,8 @@ static bool create_glx_window(struct MPGLContext *ctx, uint32_t d_width,
     glXGetFBConfigAttrib(vo->x11->display, fbc, GLX_GREEN_SIZE, &ctx->depth_g);
     glXGetFBConfigAttrib(vo->x11->display, fbc, GLX_BLUE_SIZE, &ctx->depth_b);
 
-    Colormap colormap = XCreateColormap(vo->x11->display, vo->x11->rootwin,
-                                        glx_ctx->vinfo->visual, AllocNone);
     vo_x11_create_vo_window(vo, glx_ctx->vinfo, vo->dx, vo->dy, d_width,
-                            d_height, flags, colormap, "gl");
-    XFreeColormap(vo->x11->display, colormap);
+                            d_height, flags, "gl");
 
     return true;
 }
diff --git a/video/out/vo_vdpau.c b/video/out/vo_vdpau.c
index 00477716b7..85bf635b11 100644
--- a/video/out/vo_vdpau.c
+++ b/video/out/vo_vdpau.c
@@ -843,11 +843,6 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
                   uint32_t format)
 {
     struct vdpctx *vc = vo->priv;
-    struct vo_x11_state *x11 = vo->x11;
-    XVisualInfo vinfo;
-    XSetWindowAttributes xswa;
-    XWindowAttributes attribs;
-    unsigned long xswamask;
 
     if (handle_preemption(vo) < 0)
         return -1;
@@ -861,20 +856,8 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
     if (IMGFMT_IS_VDPAU(vc->image_format) && !create_vdp_decoder(vo, 2))
         return -1;
 
-    XGetWindowAttributes(x11->display, DefaultRootWindow(x11->display),
-                         &attribs);
-    XMatchVisualInfo(x11->display, x11->screen, attribs.depth, TrueColor,
-                     &vinfo);
-
-    xswa.background_pixel = 0;
-    xswa.border_pixel     = 0;
-    /* Do not use CWBackPixel: It leads to VDPAU errors after
-     * aspect ratio changes. */
-    xswamask = CWBorderPixel;
-
-    vo_x11_create_vo_window(vo, &vinfo, vo->dx, vo->dy, d_width, d_height,
-                            flags, CopyFromParent, "vdpau");
-    XChangeWindowAttributes(x11->display, x11->window, xswamask, &xswa);
+    vo_x11_create_vo_window(vo, NULL, vo->dx, vo->dy, d_width, d_height,
+                            flags, "vdpau");
 
     if (initialize_vdpau_objects(vo) < 0)
         return -1;
diff --git a/video/out/vo_x11.c b/video/out/vo_x11.c
index 09055a698d..b541cbd578 100644
--- a/video/out/vo_x11.c
+++ b/video/out/vo_x11.c
@@ -325,7 +325,6 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
 {
     struct priv *p = vo->priv;
 
-    Colormap theCmap;
     const struct fmt2Xfmtentry_s *fmte = fmt2Xfmt;
 
     mp_image_unrefp(&p->original_image);
@@ -365,12 +364,8 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
     p->image_width = (width + 7) & (~7);
     p->image_height = height;
 
-    {
-        theCmap = vo_x11_create_colormap(vo, &p->vinfo);
-
-        vo_x11_create_vo_window(vo, &p->vinfo, vo->dx, vo->dy, vo->dwidth,
-                                vo->dheight, flags, theCmap, "x11");
-    }
+    vo_x11_create_vo_window(vo, &p->vinfo, vo->dx, vo->dy, vo->dwidth,
+                            vo->dheight, flags, "x11");
 
     if (WinID > 0) {
         unsigned depth, dummy_uint;
diff --git a/video/out/vo_xv.c b/video/out/vo_xv.c
index 58e549ee16..b5fd34991e 100644
--- a/video/out/vo_xv.c
+++ b/video/out/vo_xv.c
@@ -494,10 +494,6 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
                   uint32_t format)
 {
     struct vo_x11_state *x11 = vo->x11;
-    XVisualInfo vinfo;
-    XSetWindowAttributes xswa;
-    XWindowAttributes attribs;
-    unsigned long xswamask;
     struct xvctx *ctx = vo->priv;
     int i;
 
@@ -530,23 +526,11 @@ static int config(struct vo *vo, uint32_t width, uint32_t height,
     if (!ctx->xv_format)
         return -1;
 
-    {
-        XGetWindowAttributes(x11->display, DefaultRootWindow(x11->display),
-                             &attribs);
-        XMatchVisualInfo(x11->display, x11->screen, attribs.depth, TrueColor,
-                         &vinfo);
-
-        xswa.border_pixel = 0;
-        xswamask = CWBorderPixel;
-        if (ctx->xv_ck_info.method == CK_METHOD_BACKGROUND) {
-            xswa.background_pixel = ctx->xv_colorkey;
-            xswamask |= CWBackPixel;
-        }
+    vo_x11_create_vo_window(vo, NULL, vo->dx, vo->dy, vo->dwidth,
+                            vo->dheight, flags, "xv");
 
-        vo_x11_create_vo_window(vo, &vinfo, vo->dx, vo->dy, vo->dwidth,
-                                vo->dheight, flags, CopyFromParent, "xv");
-        XChangeWindowAttributes(x11->display, x11->window, xswamask, &xswa);
-    }
+    if (ctx->xv_ck_info.method == CK_METHOD_BACKGROUND)
+        XSetWindowBackground(x11->display, x11->window, ctx->xv_colorkey);
 
     mp_msg(MSGT_VO, MSGL_V, "using Xvideo port %d for hw scaling\n",
            ctx->xv_port);
diff --git a/video/out/x11_common.c b/video/out/x11_common.c
index b30965b368..5768aaf99b 100644
--- a/video/out/x11_common.c
+++ b/video/out/x11_common.c
@@ -134,6 +134,8 @@ static void vo_x11_selectinput_witherr(Display *display, Window w,
 static void vo_x11_setlayer(struct vo *vo, Window vo_window, int layer);
 static void vo_x11_vm_switch(struct vo *vo);
 static void vo_x11_vm_close(struct vo *vo);
+static void vo_x11_create_colormap(struct vo_x11_state *x11,
+                                   XVisualInfo *vinfo);
 
 /*
  * Sends the EWMH fullscreen state event.
@@ -664,6 +666,8 @@ void vo_x11_uninit(struct vo *vo)
     }
     if (x11->xic)
         XDestroyIC(x11->xic);
+    if (x11->colormap != None)
+        XFreeColormap(vo->x11->display, x11->colormap);
     vo_fs = 0;
 
     mp_msg(MSGT_VO, MSGL_V, "vo: uninit ...\n");
@@ -961,29 +965,37 @@ static void vo_x11_update_window_title(struct vo *vo)
     vo_x11_set_property_utf8(vo, x11->XA_NET_WM_ICON_NAME, title);
 }
 
-//
-static Window vo_x11_create_smooth_window(struct vo_x11_state *x11, Window mRoot,
-                                   Visual * vis, int x, int y,
-                                   unsigned int width, unsigned int height,
-                                   int depth, Colormap col_map)
+static void setup_window_params(struct vo_x11_state *x11, XVisualInfo *vis,
+                                unsigned long *mask, XSetWindowAttributes *att)
 {
-    unsigned long xswamask = CWBorderPixel;
+    vo_x11_create_colormap(x11, vis);
+
+    *mask = CWBorderPixel | CWColormap;
+    att->border_pixel = 0;
+    att->colormap = x11->colormap;
+}
+
+static void find_default_visual(struct vo_x11_state *x11, XVisualInfo *vis)
+{
+    Display *display = x11->display;
+    XWindowAttributes attribs;
+    XGetWindowAttributes(display, DefaultRootWindow(display), &attribs);
+    XMatchVisualInfo(display, x11->screen, attribs.depth, TrueColor, vis);
+}
+
+static Window vo_x11_create_smooth_window(struct vo_x11_state *x11,
+                                          XVisualInfo *vis, int x, int y,
+                                          unsigned int width, unsigned int height)
+{
+    unsigned long xswamask;
     XSetWindowAttributes xswa;
     Window ret_win;
 
-    if (col_map != CopyFromParent)
-    {
-        xswa.colormap = col_map;
-        xswamask |= CWColormap;
-    }
-    xswa.background_pixel = 0;
-    xswa.border_pixel = 0;
-    xswa.backing_store = NotUseful;
-    xswa.bit_gravity = StaticGravity;
+    setup_window_params(x11, vis, &xswamask, &xswa);
 
     ret_win =
-        XCreateWindow(x11->display, x11->rootwin, x, y, width, height, 0, depth,
-                      CopyFromParent, vis, xswamask, &xswa);
+        XCreateWindow(x11->display, x11->rootwin, x, y, width, height, 0,
+                      vis->depth, CopyFromParent, vis->visual, xswamask, &xswa);
     XSetWMProtocols(x11->display, ret_win, &x11->XAWM_DELETE_WINDOW, 1);
     if (x11->f_gc == None)
         x11->f_gc = XCreateGC(x11->display, ret_win, 0, 0);
@@ -994,14 +1006,12 @@ static Window vo_x11_create_smooth_window(struct vo_x11_state *x11, Window mRoot
 
 /**
  * \brief create and setup a window suitable for display
- * \param vis Visual to use for creating the window
+ * \param vis Visual to use for creating the window (NULL for default)
  * \param x x position of window
  * \param y y position of window
  * \param width width of window
  * \param height height of window
  * \param flags flags for window creation.
- *              Only VOFLAG_FULLSCREEN is supported so far.
- * \param col_map Colourmap for window or CopyFromParent if a specific colormap isn't needed
  * \param classname name to use for the classhint
  *
  * This also does the grunt-work like setting Window Manager hints etc.
@@ -1009,22 +1019,24 @@ static Window vo_x11_create_smooth_window(struct vo_x11_state *x11, Window mRoot
  */
 void vo_x11_create_vo_window(struct vo *vo, XVisualInfo *vis, int x, int y,
                              unsigned int width, unsigned int height, int flags,
-                             Colormap col_map, const char *classname)
+                             const char *classname)
 {
   struct MPOpts *opts = vo->opts;
   struct vo_x11_state *x11 = vo->x11;
   Display *mDisplay = vo->x11->display;
   bool force_change_xy = opts->vo_geometry.xy_valid || xinerama_screen >= 0;
+  XVisualInfo vinfo_storage;
+  if (!vis) {
+    vis = &vinfo_storage;
+    find_default_visual(x11, vis);
+  }
   if (WinID >= 0) {
+    unsigned long xswamask;
+    XSetWindowAttributes xswa;
     vo_fs = flags & VOFLAG_FULLSCREEN;
     x11->window = WinID ? (Window)WinID : x11->rootwin;
-    if (col_map != CopyFromParent) {
-      unsigned long xswamask = CWColormap;
-      XSetWindowAttributes xswa;
-      xswa.colormap = col_map;
-      XChangeWindowAttributes(mDisplay, x11->window, xswamask, &xswa);
-      XInstallColormap(mDisplay, col_map);
-    }
+    setup_window_params(x11, vis, &xswamask, &xswa);
+    XChangeWindowAttributes(mDisplay, x11->window, xswamask, &xswa);
     if (WinID) {
         // Expose events can only really be handled by us, so request them.
         vo_x11_selectinput_witherr(mDisplay, x11->window, ExposureMask);
@@ -1043,8 +1055,7 @@ void vo_x11_create_vo_window(struct vo *vo, XVisualInfo *vis, int x, int y,
     vo_fs = 0;
     vo->dwidth = width;
     vo->dheight = height;
-    x11->window = vo_x11_create_smooth_window(x11, x11->rootwin, vis->visual,
-                      x, y, width, height, vis->depth, col_map);
+    x11->window = vo_x11_create_smooth_window(x11, vis, x, y, width, height);
     x11->window_state = VOFLAG_HIDDEN;
   }
   if (flags & VOFLAG_HIDDEN)
@@ -1663,18 +1674,22 @@ double vo_vm_get_fps(struct vo *vo)
 #endif
 
 
-Colormap vo_x11_create_colormap(struct vo *vo, XVisualInfo *vinfo)
+static void vo_x11_create_colormap(struct vo_x11_state *x11,
+                                   XVisualInfo *vinfo)
 {
-    struct vo_x11_state *x11 = vo->x11;
     unsigned k, r, g, b, ru, gu, bu, m, rv, gv, bv, rvu, gvu, bvu;
 
-    if (vinfo->class != DirectColor)
-        return XCreateColormap(x11->display, x11->rootwin, vinfo->visual,
-                               AllocNone);
+    if (x11->colormap != None)
+        return;
+
+    if (vinfo->class != DirectColor) {
+        x11->colormap = XCreateColormap(x11->display, x11->rootwin,
+                                        vinfo->visual, AllocNone);
+        return;
+    }
+
+    // DirectColor is requested by vo_x11 by default, for the equalizer
 
-    /* can this function get called twice or more? */
-    if (x11->cmap)
-        return x11->cmap;
     x11->cm_size = vinfo->colormap_size;
     x11->red_mask = vinfo->red_mask;
     x11->green_mask = vinfo->green_mask;
@@ -1713,10 +1728,9 @@ Colormap vo_x11_create_colormap(struct vo *vo, XVisualInfo *vinfo)
         gv += gvu;
         bv += bvu;
     }
-    x11->cmap = XCreateColormap(x11->display, x11->rootwin, vinfo->visual,
-                                AllocAll);
-    XStoreColors(x11->display, x11->cmap, x11->cols, x11->cm_size);
-    return x11->cmap;
+    x11->colormap = XCreateColormap(x11->display, x11->rootwin, vinfo->visual,
+                                    AllocAll);
+    XStoreColors(x11->display, x11->colormap, x11->cols, x11->cm_size);
 }
 
 static int transform_color(float val,
@@ -1752,7 +1766,7 @@ uint32_t vo_x11_set_equalizer(struct vo *vo, const char *name, int value)
      * for some reason) it is impossible to restore the setting,
      * and such behaviour could be rather annoying for the users.
      */
-    if (x11->cmap == None)
+    if (x11->colormap == None)
         return VO_NOTAVAIL;
 
     if (!strcasecmp(name, "brightness"))
@@ -1781,7 +1795,7 @@ uint32_t vo_x11_set_equalizer(struct vo *vo, const char *name, int value)
         x11->cols[k].blue  = transform_color(bf * k, brightness, contrast, gamma);
     }
 
-    XStoreColors(vo->x11->display, x11->cmap, x11->cols, x11->cm_size);
+    XStoreColors(vo->x11->display, x11->colormap, x11->cols, x11->cm_size);
     XFlush(vo->x11->display);
     return VO_TRUE;
 }
@@ -1789,7 +1803,7 @@ uint32_t vo_x11_set_equalizer(struct vo *vo, const char *name, int value)
 uint32_t vo_x11_get_equalizer(struct vo *vo, const char *name, int *value)
 {
     struct vo_x11_state *x11 = vo->x11;
-    if (x11->cmap == None)
+    if (x11->colormap == None)
         return VO_NOTAVAIL;
     if (!strcasecmp(name, "brightness"))
         *value = x11->vo_brightness;
diff --git a/video/out/x11_common.h b/video/out/x11_common.h
index 8cafd15650..47eaf1d880 100644
--- a/video/out/x11_common.h
+++ b/video/out/x11_common.h
@@ -43,6 +43,7 @@ struct vo_x11_state {
     XIC xic;
 
     GC vo_gc;
+    Colormap colormap;
 
     int wm_type;
     int fs_type;
@@ -116,14 +117,13 @@ int vo_x11_init(struct vo *vo);
 void vo_x11_uninit(struct vo *vo);
 int vo_x11_check_events(struct vo *vo);
 void vo_x11_fullscreen(struct vo *vo);
-Colormap vo_x11_create_colormap(struct vo *vo, XVisualInfo *vinfo);
 uint32_t vo_x11_set_equalizer(struct vo *vo, const char *name, int value);
 uint32_t vo_x11_get_equalizer(struct vo *vo, const char *name, int *value);
 bool vo_x11_screen_is_composited(struct vo *vo);
 void fstype_help(void);
 void vo_x11_create_vo_window(struct vo *vo, XVisualInfo *vis,
         int x, int y, unsigned int width, unsigned int height, int flags,
-	Colormap col_map, const char *classname);
+        const char *classname);
 void vo_x11_clearwindow_part(struct vo *vo, Window vo_window,
 	int img_width, int img_height);
 void vo_x11_clearwindow(struct vo *vo, Window vo_window);