dix/glx/composite: consolidate visual resize in one place.

The previous code was copied and in both cases incorrectly fixed
up the colormaps after resizing the visuals, this patch consolidates
the visual resize + colormaps fixups in one place. This version
also consolidates the vid allocation for the DepthPtr inside the
function.

I'm not 100% sure colormap.[ch] is the correct place for this but
visuals are mostly created in fb and I know thats not the place to
be resizing them.

Fixes fd.o bug #19470.

Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 6ffda5aae7)

composite/compinit.c

@@ -248,15 +248,9 @@ static Bool
 compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
 		       CompAlternateVisual *alt)
 {
-    VisualPtr	    visual, visuals;
-    int		    i;
-    int		    numVisuals;
-    XID		    *installedCmaps;
-    ColormapPtr	    installedCmap;
-    int		    numInstalledCmaps;
+    VisualPtr	    visual;
     DepthPtr	    depth;
     PictFormatPtr   pPictFormat;
-    VisualID	    *vid;
     unsigned long   alphaMask;
 
     /*
@@ -277,54 +271,13 @@ compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
 	pPictFormat->direct.red != pScreen->visuals[0].offsetRed)
 	return FALSE;
 
-    vid = xalloc(sizeof(VisualID));
-    if (!vid)
-	return FALSE;
-
-    /* Find the installed colormaps */
-    installedCmaps = xalloc (pScreen->maxInstalledCmaps * sizeof (XID));
-    if (!installedCmaps) {
-	xfree(vid);
-	return FALSE;
-    }
-    numInstalledCmaps = pScreen->ListInstalledColormaps(pScreen, 
-	    installedCmaps);
-
-    /* realloc the visual array to fit the new one in place */
-    numVisuals = pScreen->numVisuals;
-    visuals = xrealloc(pScreen->visuals, (numVisuals + 1) * sizeof(VisualRec));
-    if (!visuals) {
-	xfree(vid);
-	xfree(installedCmaps);
-	return FALSE;
+    if (ResizeVisualArray(pScreen, 1, depth) == FALSE) {
+        return FALSE;
     }
 
-    /*
-     * Fix up any existing installed colormaps -- we'll assume that
-     * the only ones created so far have been installed.  If this
-     * isn't true, we'll have to walk the resource database looking
-     * for all colormaps.
-     */
-    for (i = 0; i < numInstalledCmaps; i++) {
-	int j, rc;
-
-	rc = dixLookupResourceByType((pointer *)&installedCmap,
-				     installedCmaps[i], RT_COLORMAP,
-				     serverClient, DixReadAccess);
-	if (rc != Success)
-	    continue;
-	j = installedCmap->pVisual - pScreen->visuals;
-	installedCmap->pVisual = &visuals[j];
-    }
-
-    xfree(installedCmaps);
-
-    pScreen->visuals = visuals;
-    visual = visuals + pScreen->numVisuals; /* the new one */
-    pScreen->numVisuals++;
+    visual = pScreen->visuals + (pScreen->numVisuals - 1); /* the new one */
 
     /* Initialize the visual */
-    visual->vid = FakeClientID (0);
     visual->bitsPerRGBValue = 8;
     if (PICT_FORMAT_TYPE(alt->format) == PICT_TYPE_COLOR) {
 	visual->class = PseudoColor;
@@ -357,10 +310,6 @@ compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
     /* remember the visual ID to detect auto-update windows */
     compRegisterAlternateVisuals(cs, &visual->vid, 1);
 
-    /* Fix up the depth */
-    *vid = visual->vid;
-    depth->numVids = 1;
-    depth->vids = vid;
     return TRUE;
 }
 

dix/colormap.c

@@ -2690,3 +2690,67 @@ IsMapInstalled(Colormap map, WindowPtr pWin)
     xfree(pmaps);
     return (found);
 }
+
+struct colormap_lookup_data {
+    ScreenPtr pScreen;
+    VisualPtr visuals;
+};
+
+static void _colormap_find_resource(pointer value, XID id,
+				    pointer cdata)
+{
+    struct colormap_lookup_data *cmap_data = cdata;
+    VisualPtr visuals = cmap_data->visuals;
+    ScreenPtr pScreen = cmap_data->pScreen;
+    ColormapPtr cmap = value;
+    int j;
+
+    j = cmap->pVisual - pScreen->visuals;
+    cmap->pVisual = &visuals[j];
+}
+
+/* something has realloced the visuals, instead of breaking
+   ABI fix it up here - glx and compsite did this wrong */
+Bool
+ResizeVisualArray(ScreenPtr pScreen, int new_visual_count,
+		  DepthPtr depth)
+{
+    struct colormap_lookup_data cdata;
+    int numVisuals;
+    VisualPtr visuals;
+    XID *vids, vid;
+    int first_new_vid, first_new_visual, i;
+
+    first_new_vid = depth->numVids;
+    first_new_visual = pScreen->numVisuals;
+
+    vids = xrealloc(depth->vids, (depth->numVids + new_visual_count) * sizeof(XID));
+    if (!vids)
+        return FALSE;
+
+    /* its realloced now no going back if we fail the next one */
+    depth->vids = vids;
+
+    numVisuals = pScreen->numVisuals + new_visual_count;
+    visuals = xrealloc(pScreen->visuals, numVisuals * sizeof(VisualRec));
+    if (!visuals) {
+	return FALSE;
+    }
+
+    cdata.visuals = visuals;
+    cdata.pScreen = pScreen;
+    FindClientResourcesByType(serverClient, RT_COLORMAP, _colormap_find_resource, &cdata);
+
+    pScreen->visuals = visuals;
+
+    for (i = 0; i < new_visual_count; i++) {
+	vid = FakeClientID(0);
+	pScreen->visuals[first_new_visual + i].vid = vid;
+	vids[first_new_vid + i] = vid;
+    }
+
+    depth->numVids += new_visual_count;
+    pScreen->numVisuals += new_visual_count;
+
+    return TRUE;
+}

glx/glxscreens.c

@@ -250,12 +250,8 @@ GLint glxConvertToXVisualType(int visualType)
 static VisualPtr
 AddScreenVisuals(ScreenPtr pScreen, int count, int d)
 {
-    XID		*installedCmaps, *vids, vid;
-    int		 numInstalledCmaps, numVisuals, i, j;
-    VisualPtr	 visuals;
-    ColormapPtr	 installedCmap;
+    int		 i;
     DepthPtr	 depth;
-    int		 rc;
 
     depth = NULL;
     for (i = 0; i < pScreen->numDepths; i++) {
@@ -267,56 +263,8 @@ AddScreenVisuals(ScreenPtr pScreen, int count, int d)
     if (depth == NULL)
 	return NULL;
 
-    /* Find the installed colormaps */
-    installedCmaps = xalloc (pScreen->maxInstalledCmaps * sizeof (XID));
-    if (!installedCmaps)
-	return NULL;
-
-    numInstalledCmaps = pScreen->ListInstalledColormaps(pScreen, installedCmaps);
-
-    /* realloc the visual array to fit the new one in place */
-    numVisuals = pScreen->numVisuals;
-    visuals = xrealloc(pScreen->visuals, (numVisuals + count) * sizeof(VisualRec));
-    if (!visuals) {
-	xfree(installedCmaps);
-	return NULL;
-    }
-
-    vids = xrealloc(depth->vids, (depth->numVids + count) * sizeof(XID));
-    if (vids == NULL) {
-	xfree(installedCmaps);
-	xfree(visuals);
-	return NULL;
-    }
-
-    /*
-     * Fix up any existing installed colormaps -- we'll assume that
-     * the only ones created so far have been installed.  If this
-     * isn't true, we'll have to walk the resource database looking
-     * for all colormaps.
-     */
-    for (i = 0; i < numInstalledCmaps; i++) {
-	rc = dixLookupResourceByType((pointer *)&installedCmap,
-				     installedCmaps[i], RT_COLORMAP,
-				     serverClient, DixReadAccess);
-	if (rc != Success)
-	    continue;
-	j = installedCmap->pVisual - pScreen->visuals;
-	installedCmap->pVisual = &visuals[j];
-    }
-
-    xfree(installedCmaps);
-
-    for (i = 0; i < count; i++) {
-	vid = FakeClientID(0);
-	visuals[pScreen->numVisuals + i].vid = vid;
-	vids[depth->numVids + i] = vid;
-    }
-
-    pScreen->visuals = visuals;
-    pScreen->numVisuals += count;
-    depth->vids = vids;
-    depth->numVids += count;
+    if (ResizeVisualArray(pScreen, count, depth) == FALSE)
+        return NULL;
 
     /* Return a pointer to the first of the added visuals. */ 
     return pScreen->visuals + pScreen->numVisuals - count;

include/colormap.h

@@ -179,4 +179,9 @@ extern _X_EXPORT int IsMapInstalled(
     Colormap /*map*/,
     WindowPtr /*pWin*/);
 
+extern _X_EXPORT Bool ResizeVisualArray(
+    ScreenPtr /* pScreen */,
+    int /* new_vis_count */,
+    DepthPtr /* depth */);
+
 #endif /* CMAP_H */