[PATCH] glx: Duplicate relevant fbconfigs for compositing visuals

[Fredrik Höglund]

Tested-by: Fredrik Höglund <fredrik at kde.org>

On Wednesday 27 September 2017, Thomas Hellstrom wrote:
> Previously, before GLX_OML_swap_method was fixed, both the X server and
> client ignored the swapMethod fbconfig value, which meant that, if the dri
> driver thought it exposed more than one swapMethod, it actually just
> exported a duplicated set of fbconfigs. When fixing GLX_OML_swap_method
> and restricting the choice for built-in visuals to a single swap method
> that meant we didn't have that many fbconfigs to choose from when pairing
> the compositing visual with an fbconfig, resulting in the fbconfig paired
> with the compositing visual becoming too restrictive for some applications,
> (at least for kwin). This problem would also happen if the dri driver
> only exposed a single swap method to begin with.
> 
> So, to make sure the compositing visual gets a good enough fbconfig,
> duplicate fbconfigs that are suitable for compositing visuals and make
> sure these duplicated fbconfigs can be used only by compositing visuals.
> For duplicated fbconfigs not paired with a compositing visual, construct
> new compositing visuals, making compositing clients able to choose visuals
> / fbconfig more adapted to their needs.
> 
> This is in some sense equivalent to adding a new "TRUECOLOR_COMPOSITING"
> GLX visualtype.
> 
> Fixes: 4486d199bd3b ("glx: Fix visual fbconfig matching with respect to
>                       swap method")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102806
> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
> Tested-By: Nick Sarnie <commendsarnex at gmail.com>
> ---
>  glx/glxdricommon.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>  glx/glxscreens.c   | 18 ++++++++++++++++--
>  glx/glxscreens.h   |  4 ++++
>  3 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c
> index 96f28d0..1879bfc 100644
> --- a/glx/glxdricommon.c
> +++ b/glx/glxdricommon.c
> @@ -116,13 +116,15 @@ render_type_is_pbuffer_only(unsigned renderType)
>  static __GLXconfig *
>  createModeFromConfig(const __DRIcoreExtension * core,
>                       const __DRIconfig * driConfig,
> -                     unsigned int visualType)
> +                     unsigned int visualType,
> +                     GLboolean duplicateForComp)
>  {
>      __GLXDRIconfig *config;
>      GLint renderType = 0;
>      unsigned int attrib, value, drawableType = GLX_PBUFFER_BIT;
>      int i;
>  
> +
>      config = calloc(1, sizeof *config);
>  
>      config->driConfig = driConfig;
> @@ -180,6 +182,28 @@ createModeFromConfig(const __DRIcoreExtension * core,
>      config->config.drawableType = drawableType;
>      config->config.yInverted = GL_TRUE;
>  
> +#ifdef COMPOSITE
> +    /*
> +     * Here we decide what fbconfigs will be duplicated for compositing.
> +     * fgbconfigs marked with duplicatedForConf will be reserved for
> +     * compositing visuals.
> +     * It might look strange to do this decision this late when translation
> +     * from a __DRIConfig is already done, but using the __DRIConfig
> +     * accessor function becomes worse both with respect to code complexity
> +     * and CPU usage.
> +     */
> +    if (duplicateForComp &&
> +        (render_type_is_pbuffer_only(renderType) ||
> +         config->config.rgbBits != 32 ||
> +         config->config.visualRating != GLX_NONE ||
> +         config->config.sampleBuffers != 0)) {
> +        free(config);
> +        return NULL;
> +    }
> +
> +    config->config.duplicatedForComp = duplicateForComp;
> +#endif
> +
>      return &config->config;
>  }
>  
> @@ -194,21 +218,34 @@ glxConvertConfigs(const __DRIcoreExtension * core,
>      head.next = NULL;
>  
>      for (i = 0; configs[i]; i++) {
> -        tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR);
> +        tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR,
> +                                          GL_FALSE);
>          if (tail->next == NULL)
>              break;
> -
>          tail = tail->next;
>      }
>  
>      for (i = 0; configs[i]; i++) {
> -        tail->next = createModeFromConfig(core, configs[i], GLX_DIRECT_COLOR);
> +        tail->next = createModeFromConfig(core, configs[i], GLX_DIRECT_COLOR,
> +                                          GL_FALSE);
>          if (tail->next == NULL)
>              break;
>  
>          tail = tail->next;
>      }
>  
> +#ifdef COMPOSITE
> +    /* Duplicate fbconfigs for use with compositing visuals */
> +    for (i = 0; configs[i]; i++) {
> +        tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR,
> +                                          GL_TRUE);
> +        if (tail->next == NULL)
> +            continue;
> +
> +	tail = tail->next;
> +    }
> +#endif
> +
>      return head.next;
>  }
>  
> diff --git a/glx/glxscreens.c b/glx/glxscreens.c
> index f000e56..99bf6dd 100644
> --- a/glx/glxscreens.c
> +++ b/glx/glxscreens.c
> @@ -274,7 +274,12 @@ pickFBConfig(__GLXscreen * pGlxScreen, VisualPtr visual)
>          /* Can't use the same FBconfig for multiple X visuals.  I think. */
>          if (config->visualID != 0)
>              continue;
> -
> +#ifdef COMPOSITE
> +	/* Use only duplicated configs for compIsAlternateVisuals */
> +        if (!!compIsAlternateVisual(pGlxScreen->pScreen, visual->vid) !=
> +	    !!config->duplicatedForComp)
> +            continue;
> +#endif
>          /*
>           * If possible, use the same swapmethod for all built-in visual
>           * fbconfigs, to avoid getting the 32-bit composite visual when
> @@ -365,7 +370,12 @@ __glXScreenInit(__GLXscreen * pGlxScreen, ScreenPtr pScreen)
>           * set up above is for.
>           */
>          depth = config->redBits + config->greenBits + config->blueBits;
> -
> +#ifdef COMPOSITE
> +	if (config->duplicatedForComp) {
> +		depth += config->alphaBits;
> +		config->visualSelectGroup++;
> +	}
> +#endif
>          /* Make sure that our FBconfig's depth can actually be displayed
>           * (corresponds to an existing visual).
>           */
> @@ -388,6 +398,10 @@ __glXScreenInit(__GLXscreen * pGlxScreen, ScreenPtr pScreen)
>          if (visual == NULL)
>              continue;
>  
> +#ifdef COMPOSITE
> +        if (config->duplicatedForComp)
> +	    (void) CompositeRegisterAlternateVisuals(pScreen, &visual->vid, 1);
> +#endif
>          pGlxScreen->visuals[pGlxScreen->numVisuals++] = config;
>          initGlxVisual(visual, config);
>      }
> diff --git a/glx/glxscreens.h b/glx/glxscreens.h
> index 0f9a2b9..c6a0c50 100644
> --- a/glx/glxscreens.h
> +++ b/glx/glxscreens.h
> @@ -39,7 +39,11 @@
>  
>  typedef struct __GLXconfig __GLXconfig;
>  struct __GLXconfig {
> +    /* Management */
>      __GLXconfig *next;
> +#ifdef COMPOSITE
> +    GLboolean duplicatedForComp;
> +#endif
>      GLuint doubleBufferMode;
>      GLuint stereoMode;
>  
>