[Mesa-dev] [PATCH] i965: handle stencil_bits parameter for MESA_FORMAT_B8G8R8X8_UNORM format.

[Ilia Mirkin]

On Wed, Dec 9, 2015 at 11:18 AM, Deve <deveee at gmail.com> wrote:
> This patch indeed seems to not have a sense. I just added it to the bug
> report as a suggestion that it works for me after this modification. Emil
> Velikov said that I should send it to the mailing list.
>
> Here is how it works in Supertuxkart:
> We create rtt with following parameters:
>
> DepthStencilTexture = generateRTT(res, GL_DEPTH24_STENCIL8,
> GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8);
>
> Then, during rendering scene, we do:
>
>     glEnable(GL_FRAMEBUFFER_SRGB);
>     glBindFramebuffer(GL_FRAMEBUFFER, 0);

OK, so this is the "winsys" framebuffer (GL has some term for it,
sorry, I don't remember what it is... perhaps it's even winsys). This
is created based on parameters of your selected GLX visual. For
example, when I run glxinfo, I see (on nouveau; the list on intel will
be different but comparable):

480 GLX Visuals
    visual  x   bf lv rg d st  colorbuffer  sr ax dp st accumbuffer  ms  cav
  id dep cl sp  sz l  ci b ro  r  g  b  a F gb bf th cl  r  g  b  a ns b eat
----------------------------------------------------------------------------
0x021 24 tc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  0 0 None
0x022 24 dc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  0 0 None
...
0x343 24 tc  0  32  0 r  . .   8  8  8  8 .  s  0  0  0  0  0  0  0  0 0 None
0x344 24 tc  0  32  0 r  . .   8  8  8  8 .  s  0  0  0 16 16 16 16  0 0 Slow
0x345 24 tc  0  32  0 r  y .   8  8  8  8 .  s  0  0  0  0  0  0  0  0 0 None
0x346 24 tc  0  32  0 r  y .   8  8  8  8 .  s  0  0  0 16 16 16 16  0 0 Slow

Note how some of them have srgb, others don't (and have various
differences in their various other properties). EGL has something
similar I believe, but tbh I don't remember the specifics. That's the
mesaVis->sRGBCapable bit below. If you need an sRGB-capable visual,
are you sure you're picking one? This would be somewhere well before
any actual rendering takes place.

If you're not sure whether you need an sRGB-capable visual or not, try
making sure you pick a *non-srgb* visual in a working configuration
and see if it breaks.

Cheers,

  -ilia

>     (...)
>     render();
>     (...)
>     glDisable(GL_FRAMEBUFFER_SRGB);
>
> It looks that glEnable(GL_FRAMEBUFFER_SRGB) doesn't work anymore. It's
> because of following lines in intel_screen.c in intelCreateBuffer()
> function:
>
>    if (mesaVis->redBits == 5)
>       rgbFormat = MESA_FORMAT_B5G6R5_UNORM;
>    else if (mesaVis->sRGBCapable)
>       rgbFormat = MESA_FORMAT_B8G8R8A8_SRGB;
>    else if (mesaVis->alphaBits == 0)
>       rgbFormat = MESA_FORMAT_B8G8R8X8_UNORM;
>    else {
>       rgbFormat = MESA_FORMAT_B8G8R8A8_SRGB;
>       fb->Visual.sRGBCapable = true;
>    }
>
> Previously MESA_FORMAT_B8G8R8X8_UNORM was not available, and thus
> MESA_FORMAT_B8G8R8A8_UNORM was handled as last case (using
> MESA_FORMAT_B8G8R8A8_SRGB format). Now it uses MESA_FORMAT_B8G8R8X8_UNORM
> format.
>
> Any ideas how it should be handled?
>
> Regards,
> Deve
>
> W dniu 09.12.2015 o 03:00, Ilia Mirkin pisze:
>
>> On Mon, Dec 7, 2015 at 5:32 PM, Dawid Gan <deveee at gmail.com> wrote:
>>>
>>> This format has been added in commit:
>>> 28090b30dd6b5977de085f48c620574214b6b4ba
>>> But it was handled in the same way as MESA_FORMAT_B8G8R8A8_UNORM format.
>>> It was causing the screen in Supertuxkart to be darker than expected,
>>> see:
>>> https://bugs.freedesktop.org/show_bug.cgi?id=92759
>>>
>>> Cc: Boyan Ding <boyan.j.ding at gmail.com>
>>> Cc: "11.0 11.1" <mesa-stable at lists.freedesktop.org>
>>> Fixes: 28090b30dd6 "i965: Add XRGB8888 format to
>>> intel_screen_make_configs"
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92759
>>> ---
>>>   src/mesa/drivers/dri/i965/intel_screen.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
>>> b/src/mesa/drivers/dri/i965/intel_screen.c
>>> index cc90efe..75d5a65 100644
>>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>>> @@ -1237,6 +1237,9 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>>>                stencil_bits[2] = 8;
>>>                num_depth_stencil_bits = 3;
>>>            }
>>> +      } else if (formats[i] == MESA_FORMAT_B8G8R8X8_UNORM) {
>>> +         depth_bits[1] = 24;
>>> +         stencil_bits[1] = 0;
>>
>>
>> Why would you want depth without stencil when using BGRX? I don't see
>> how the two are connected... Are you sure you're picking the right
>> visual?
>>
>>    -ilia
>>
>