Technically, a correctly written application should not rely on 
ChoosePixelFormat but should enumerate by itself through all the pixel formats 
and select the best one with its own algorithm.

However, for the (numerous) applications that use ChoosePixelFormat, color 
depth is more a question of quality of rendering, whereas depth and stencil 
buffers are a question a correct/incorrect rendering.

The current method gives a higher priority to color depth than to depth and 
stencil buffers depth to the point that a different color depth disables the 
stencil buffer.

To make things even worse, many applications (including GLUT) incorrectly set 
ppfd->cColorBits to 32 instead of 24, although the documentation clearly states 
that "For RGBA pixel types, it is the size of the color buffer, EXCLUDING THE 
ALPHA BITPLANES" (emphasis is mine).

As a result, those applications never get a stencil buffer, which leads to 
incorrect rendering. I stumbled on this problem while giving a try to OpenCSG 
and it took me a while to discover that the wrong results were caused by the 
absence of a stencil buffer requested by GLUT.

Although there is no universal selection algorithm, the one I suggest tries to 
enforce the following policy:

- Most important is to enable all the buffers (depth, stencil, accumulation...) 
that are requested (correction).
- Then, try to allocate at least as many bits as requested (quality + 
performance).
- Least important, try not to allocate more bits than requested (economy).

This algorithm seems to be in line with the behavior of most Windows drivers 
(Microsoft, NVIDIA, AMD) and, more important, I can't imagine a sensible 
scenario where this change would break an existing application.

-Olivier

-----Message d'origine-----
De : Andres Gomez [mailto:ago...@igalia.com] 
Envoyé : samedi 8 juillet 2017 22:08
À : Olivier Lauffenburger <o.lauffenbur...@topsolid.com>; 
mesa-dev@lists.freedesktop.org
Cc : mesa-sta...@lists.freedesktop.org; Brian Paul <bri...@vmware.com>
Objet : Re: [Mesa-dev] [PATCH] gallium: improve selection of pixel format

Olivier, Brian, do we want this into -stable?

On Thu, 2017-07-06 at 17:08 +0200, Olivier Lauffenburger wrote:
> Current selection of pixel format does not enforce the request of 
> stencil or depth buffer if the color depth is not the same as 
> requested.
> For instance, GLUT requests a 32-bit color buffer with an 8-bit 
> stencil buffer, but because color buffers are only 24-bit, no priority 
> is given to creating a stencil buffer.
> 
> This patch gives more priority to the creation of requested buffers 
> and less priority to the difference in bit depth.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101703
> 
> Signed-off-by: Olivier Lauffenburger <o.lauffenbur...@topsolid.com>
> ---
>  src/gallium/state_trackers/wgl/stw_pixelformat.c | 36 
> +++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/wgl/stw_pixelformat.c 
> b/src/gallium/state_trackers/wgl/stw_pixelformat.c
> index 7763f71cbc..833308d964 100644
> --- a/src/gallium/state_trackers/wgl/stw_pixelformat.c
> +++ b/src/gallium/state_trackers/wgl/stw_pixelformat.c
> @@ -432,17 +432,39 @@ stw_pixelformat_choose(HDC hdc, CONST 
> PIXELFORMATDESCRIPTOR *ppfd)
>            !!(pfi->pfd.dwFlags & PFD_DOUBLEBUFFER))
>           continue;
>  
> -      /* FIXME: Take in account individual channel bits */
> -      if (ppfd->cColorBits != pfi->pfd.cColorBits)
> -         delta += 8;
> +      /* Selection logic:
> +      * - Enabling a feature (depth, stencil...) is given highest priority.
> +      * - Giving as many bits as requested is given medium priority.
> +      * - Giving no more bits than requested is given lowest priority.
> +      */
>  
> -      if (ppfd->cDepthBits != pfi->pfd.cDepthBits)
> -         delta += 4;
> +      /* FIXME: Take in account individual channel bits */
> +      if (ppfd->cColorBits && !pfi->pfd.cColorBits)
> +         delta += 10000;
> +      else if (ppfd->cColorBits > pfi->pfd.cColorBits)
> +         delta += 100;
> +      else if (ppfd->cColorBits < pfi->pfd.cColorBits)
> +         delta++;
>  
> -      if (ppfd->cStencilBits != pfi->pfd.cStencilBits)
> +      if (ppfd->cDepthBits && !pfi->pfd.cDepthBits)
> +         delta += 10000;
> +      else if (ppfd->cDepthBits > pfi->pfd.cDepthBits)
> +         delta += 200;
> +      else if (ppfd->cDepthBits < pfi->pfd.cDepthBits)
>           delta += 2;
>  
> -      if (ppfd->cAlphaBits != pfi->pfd.cAlphaBits)
> +      if (ppfd->cStencilBits && !pfi->pfd.cStencilBits)
> +         delta += 10000;
> +      else if (ppfd->cStencilBits > pfi->pfd.cStencilBits)
> +         delta += 400;
> +      else if (ppfd->cStencilBits < pfi->pfd.cStencilBits)
> +         delta++;
> +
> +      if (ppfd->cAlphaBits && !pfi->pfd.cAlphaBits)
> +         delta += 10000;
> +      else if (ppfd->cAlphaBits > pfi->pfd.cAlphaBits)
> +         delta += 100;
> +      else if (ppfd->cAlphaBits < pfi->pfd.cAlphaBits)
>           delta++;
>  
>        if (delta < bestdelta) {
--
Br,

Andres


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to