[PATCH] glx: Replace broken GLX visual setup with a fixed "all" mode.

Eric Anholt eric at anholt.net
Sun Feb 8 04:00:15 PST 2009


With trying to match depths so that you didn't end up with a depth 24
fbconfig for the 32-bit composite visual, I broke the alpha bits on the depth
24 X visual, which angered other applications.  But in fixing that, the
pickFBconfigs code for "minimal" also could end up breaking GLX visuals if
the same FBconfig was chosen for more than one X visual.
We have no reason to not expose as many visuals as possible, but the old
"all" mode didn't match any existing X visuals to GLX visuals, so normal
GL apps didn't work at all.

Instead, replace it with a simple combination of the two modes: Create GLX
visuals by picking unique FBconfigs with as many features as possible for
each X visual in order.  Then, for all remaining FBconfigs that are
appropriate for display, add a corresponding X and GLX visual.

This gets all applications (even ones that aren't smart enough to do FBconfigs)
get all the options to get the visual configuration they want.  The only
potential downside is that the composite ARGB visual is unique and gets a
nearly full-featured GLX visual (except that the root visual might have taken
the tastiest FBconfig), which means that a dumb compositing manager could
waste resources. Write compositing managers using FBconfigs instead, please.
---
 glx/glxscreens.c               |  233 +++++++++++++--------------------------
 glx/glxserver.h                |    2 -
 hw/xfree86/dixmods/glxmodule.c |   15 ---
 3 files changed, 78 insertions(+), 172 deletions(-)

diff --git a/glx/glxscreens.c b/glx/glxscreens.c
index 2c8432e..2656355 100644
--- a/glx/glxscreens.c
+++ b/glx/glxscreens.c
@@ -242,44 +242,6 @@ GLint glxConvertToXVisualType(int visualType)
 	? x_visual_types[ visualType - GLX_TRUE_COLOR ] : -1;
 }
 
-
-static void
-filterOutNativeConfigs(__GLXscreen *pGlxScreen)
-{
-    __GLXconfig *m, *next, **last;
-    ScreenPtr pScreen = pGlxScreen->pScreen;
-    int i, depth;
-
-    last = &pGlxScreen->fbconfigs;
-    for (m = pGlxScreen->fbconfigs; m != NULL; m = next) {
-	next = m->next;
-	depth = m->redBits + m->blueBits + m->greenBits;
-
-	for (i = 0; i < pScreen->numVisuals; i++) {
-	    if (pScreen->visuals[i].nplanes == depth) {
-		*last = m;
-		last = &m->next;
-		break;
-	    }
-	}
-    }
-
-    *last = NULL;
-}
-
-static XID
-findVisualForConfig(ScreenPtr pScreen, __GLXconfig *m)
-{
-    int i;
-
-    for (i = 0; i < pScreen->numVisuals; i++) {
-	if (glxConvertToXVisualType(m->visualType) == pScreen->visuals[i].class)
-	    return pScreen->visuals[i].vid;
-    }
-
-    return 0;
-}
-
 /* This code inspired by composite/compinit.c.  We could move this to
  * mi/ and share it with composite.*/
 
@@ -387,125 +349,52 @@ initGlxVisual(VisualPtr visual, __GLXconfig *config)
     visual->offsetBlue = findFirstSet(config->blueMask);
 }
 
-typedef struct {
-    GLboolean doubleBuffer;
-    GLboolean depthBuffer;
-    GLboolean stencilBuffer;
-} FBConfigTemplateRec, *FBConfigTemplatePtr;
-
 static __GLXconfig *
-pickFBConfig(__GLXscreen *pGlxScreen, FBConfigTemplatePtr template,
-	     VisualPtr visual)
+pickFBConfig(__GLXscreen *pGlxScreen, VisualPtr visual)
 {
-    __GLXconfig *config;
+    __GLXconfig *best = NULL, *config;
+    int best_score;
 
     for (config = pGlxScreen->fbconfigs; config != NULL; config = config->next) {
+	int score = 0;
+
 	if (config->redMask != visual->redMask ||
 	    config->greenMask != visual->greenMask ||
-	    config->blueMask != visual->blueMask ||
-	    config->rgbBits != visual->nplanes)
+	    config->blueMask != visual->blueMask)
 	    continue;
 	if (config->visualRating != GLX_NONE)
 	    continue;
 	if (glxConvertToXVisualType(config->visualType) != visual->class)
 	    continue;
-	if ((config->doubleBufferMode > 0) != template->doubleBuffer)
-	    continue;
-	if ((config->depthBits > 0) != template->depthBuffer)
+	/* If it's the 32-bit RGBA visual, demand a 32-bit fbconfig. */
+	if (visual->nplanes == 32 && config->rgbBits != 32)
 	    continue;
-	if ((config->stencilBits > 0) != template->stencilBuffer)
-	    continue;
-
-	return config;
-    }
-
-    return NULL;
-}
-
-static void
-addMinimalSet(__GLXscreen *pGlxScreen)
-{
-    __GLXconfig *config;
-    VisualPtr visuals;
-    int i, j;
-    FBConfigTemplateRec best = { GL_TRUE, GL_TRUE, GL_TRUE };
-    FBConfigTemplateRec good = { GL_TRUE, GL_TRUE, GL_FALSE };
-    FBConfigTemplateRec minimal = { GL_FALSE, GL_FALSE, GL_FALSE };
-
-    pGlxScreen->visuals = xcalloc(pGlxScreen->pScreen->numVisuals,
-				  sizeof (__GLXconfig *));
-    if (pGlxScreen->visuals == NULL) {
-	ErrorF("Failed to allocate for minimal set of GLX visuals\n");
-	return;
-    }
-
-    visuals = pGlxScreen->pScreen->visuals;
-    for (i = 0, j = 0; i < pGlxScreen->pScreen->numVisuals; i++) {
-	if (visuals[i].nplanes == 32)
-	    config = pickFBConfig(pGlxScreen, &minimal, &visuals[i]);
-	else {
-	    config = pickFBConfig(pGlxScreen, &best, &visuals[i]);
-	    if (config == NULL)
-		config = pickFBConfig(pGlxScreen, &good, &visuals[i]);
-        }
-	if (config == NULL)
-	    config = pGlxScreen->fbconfigs;
-	if (config == NULL)
+	/* Can't use the same FBconfig for multiple X visuals.  I think. */
+	if (config->visualID != 0)
 	    continue;
 
-	pGlxScreen->visuals[j] = config;
-	config->visualID = visuals[i].vid;
-	j++;
-    }
-
-    pGlxScreen->numVisuals = j;
-}
-
-static void
-addTypicalSet(__GLXscreen *pGlxScreen)
-{
-    addMinimalSet(pGlxScreen);
-}
-
-static void
-addFullSet(__GLXscreen *pGlxScreen)
-{
-    __GLXconfig *config;
-    VisualPtr visuals;
-    int i, depth;
-
-    pGlxScreen->visuals =
-	xcalloc(pGlxScreen->numFBConfigs, sizeof (__GLXconfig *));
-    if (pGlxScreen->visuals == NULL) {
-	ErrorF("Failed to allocate for full set of GLX visuals\n");
-	return;
-    }
-
-    config = pGlxScreen->fbconfigs;
-    depth = config->redBits + config->greenBits + config->blueBits;
-    visuals = AddScreenVisuals(pGlxScreen->pScreen, pGlxScreen->numFBConfigs, depth);
-    if (visuals == NULL) {
-	xfree(pGlxScreen->visuals);
-	return;
-    }
-
-    pGlxScreen->numVisuals = pGlxScreen->numFBConfigs;
-    for (i = 0, config = pGlxScreen->fbconfigs; config; config = config->next, i++) {
-	pGlxScreen->visuals[i] = config;
-	initGlxVisual(&visuals[i], config);
+	if (config->doubleBufferMode > 0)
+	    score += 8;
+	if (config->depthBits > 0)
+	    score += 4;
+	if (config->stencilBits > 0)
+	    score += 2;
+	if (config->alphaBits > 0)
+	    score++;
+
+	if (score > best_score) {
+	    best = config;
+	    best_score = score;
+	}
     }
-}
 
-static int glxVisualConfig = GLX_ALL_VISUALS;
-
-void GlxSetVisualConfig(int config)
-{
-    glxVisualConfig = config;
+    return best;
 }
 
 void __glXScreenInit(__GLXscreen *pGlxScreen, ScreenPtr pScreen)
 {
     __GLXconfig *m;
+    __GLXconfig *config;
     int i;
 
     pGlxScreen->pScreen       = pScreen;
@@ -517,32 +406,66 @@ void __glXScreenInit(__GLXscreen *pGlxScreen, ScreenPtr pScreen)
     pGlxScreen->CloseScreen = pScreen->CloseScreen;
     pScreen->CloseScreen = glxCloseScreen;
 
-    filterOutNativeConfigs(pGlxScreen);
-
     i = 0;
     for (m = pGlxScreen->fbconfigs; m != NULL; m = m->next) {
 	m->fbconfigID = FakeClientID(0);
-	m->visualID = findVisualForConfig(pScreen, m);
+	m->visualID = 0;
 	i++;
     }
     pGlxScreen->numFBConfigs = i;
 
-    /* Select a subset of fbconfigs that we send to the client when it
-     * asks for the glx visuals.  All the fbconfigs here have a valid
-     * value for visual ID and each visual ID is only present once.
-     * This runs before composite adds its extra visual so we have to
-     * remember the number of visuals here.*/
-
-    switch (glxVisualConfig) {
-    case GLX_MINIMAL_VISUALS:
-	addMinimalSet(pGlxScreen);
-	break;
-    case GLX_TYPICAL_VISUALS:
-	addTypicalSet(pGlxScreen);
-	break;
-    case GLX_ALL_VISUALS:
-	addFullSet(pGlxScreen);
-	break;
+    pGlxScreen->visuals =
+	xcalloc(pGlxScreen->numFBConfigs, sizeof (__GLXconfig *));
+
+    /* First, try to choose featureful FBconfigs for the existing X visuals.
+     * Note that if multiple X visuals end up with the same FBconfig being
+     * chosen, the later X visuals don't get GLX visuals (because we want to
+     * prioritize the root visual being GLX).
+     */
+    for (i = 0; i < pScreen->numVisuals; i++) {
+	VisualPtr visual = &pScreen->visuals[i];
+
+	config = pickFBConfig(pGlxScreen, visual);
+	if (config) {
+	    pGlxScreen->visuals[pGlxScreen->numVisuals++] = config;
+	    config->visualID = visual->vid;
+	}
+    }
+
+    /* Then, add new visuals corresponding to all FBconfigs that didn't have
+     * an existing, appropriate visual.
+     */
+    for (config = pGlxScreen->fbconfigs; config != NULL; config = config->next) {
+	int depth;
+
+	VisualPtr visual;
+
+	if (config->visualID != 0)
+	    continue;
+
+	/* Only count RGB bits and not alpha, as we're not trying to create
+	 * visuals for compositing (that's what the 32-bit composite visual
+	 * set up above is for.
+	 */
+	depth = config->redBits + config->greenBits + config->blueBits;
+
+	/* Make sure that our FBconfig's depth can actually be displayed
+	 * (corresponds to an existing visual).
+	 */
+	for (i = 0; i < pScreen->numVisuals; i++) {
+	    if (depth == pScreen->visuals[i].nplanes)
+		break;
+	}
+	if (i == pScreen->numVisuals)
+	    continue;
+
+	/* Create a new X visual for our FBconfig. */
+	visual = AddScreenVisuals(pScreen, 1, depth);
+	if (visual == NULL)
+	    continue;
+
+	pGlxScreen->visuals[pGlxScreen->numVisuals++] = config;
+	initGlxVisual(visual, config);
     }
 
     dixSetPrivate(&pScreen->devPrivates, glxScreenPrivateKey, pGlxScreen);
diff --git a/glx/glxserver.h b/glx/glxserver.h
index 1d20929..a5ca0a2 100644
--- a/glx/glxserver.h
+++ b/glx/glxserver.h
@@ -135,8 +135,6 @@ enum {
     GLX_ALL_VISUALS
 };
 
-void GlxSetVisualConfig(int config);
-
 void __glXsetEnterLeaveServerFuncs(void (*enter)(GLboolean),
 				   void (*leave)(GLboolean));
 void __glXenterServer(GLboolean rendering);
diff --git a/hw/xfree86/dixmods/glxmodule.c b/hw/xfree86/dixmods/glxmodule.c
index f6fda4b..62a047e 100644
--- a/hw/xfree86/dixmods/glxmodule.c
+++ b/hw/xfree86/dixmods/glxmodule.c
@@ -101,21 +101,6 @@ glxSetup(pointer module, pointer opts, int *errmaj, int *errmin)
 	GlxPushProvider(provider);
     }
 
-    switch (xf86Info.glxVisuals) {
-    case XF86_GlxVisualsMinimal:
-	GlxSetVisualConfig(GLX_MINIMAL_VISUALS);
-	xf86Msg(xf86Info.aiglxFrom, "Exporting only minimal set of GLX visuals\n");
-	break;
-    case XF86_GlxVisualsTypical:
-	GlxSetVisualConfig(GLX_TYPICAL_VISUALS);
-	xf86Msg(xf86Info.aiglxFrom, "Exporting typical set of GLX visuals\n");
-	break;
-    case XF86_GlxVisualsAll:
-	GlxSetVisualConfig(GLX_ALL_VISUALS);
-	xf86Msg(xf86Info.aiglxFrom, "Exporting all GLX visuals\n");
-	break;
-    }
-
     LoadExtension(&GLXExt, FALSE);
 
     return module;
-- 
1.5.6.5




More information about the xorg mailing list