[Mesa-dev] [PATCH 3/3] i965: Use MESA_FORMAT_B8G8R8X8_SRGB for RGB visuals

[Ilia Mirkin]

On Fri, Dec 11, 2015 at 1:02 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> Neil Roberts <neil at linux.intel.com> writes:
>
>> Previously if the visual didn't have an alpha channel then it would
>> pick a format that is not sRGB-capable. I don't think there's any
>> reason not to always have an sRGB-capable visual. Since 28090b30 there
>> are now visuals advertised without an alpha channel which means that
>> games that don't request alpha bits in the config would end up without
>> an sRGB-capable visual. This was breaking supertuxkart which assumes
>> the winsys buffer is always sRGB-capable.
>>
>> The previous code always used an RGBA format if the visual config
>> itself was marked as sRGB-capable regardless of whether the visual has
>> alpha bits. I think we don't actually advertise any sRGB-capable
>> visuals (but we just use sRGB formats anyway) so it shouldn't make any
>> difference. However this patch also changes it to use RGBX if an
>> sRGB-capable visual is requested without alpha bits for consistency.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92759
>> Cc: "11.0 11.1" <mesa-stable at lists.freedesktop.org>
>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>> Suggested-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/mesa/drivers/dri/i965/intel_screen.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
>> index cc90efe..825a7c1 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -999,14 +999,13 @@ intelCreateBuffer(__DRIscreen * driScrnPriv,
>>        fb->Visual.samples = num_samples;
>>     }
>>
>> -   if (mesaVis->redBits == 5)
>> +   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;
>> +   } else {
>> +      if (mesaVis->alphaBits == 0)
>> +         rgbFormat = MESA_FORMAT_B8G8R8X8_SRGB;
>> +      else
>> +         rgbFormat = MESA_FORMAT_B8G8R8A8_SRGB;
>>        fb->Visual.sRGBCapable = true;
>
> I agree with Ilia that this code is notoriously subtle and prone to
> breaking applications. While I don't fully understand why we use
> MESA_FORMAT_B8G8R8A8_SRGB for !mesaVis->sRGBCapable, the change you're

Well, for some reason, i965 doesn't expose *any* srgb-capable visuals.
But previoiusly all the 8-bit color ones used to be srgb-capable
anyways, whereas after BGRX was added, suddenly requesting an
alpha-less visual would silently lose you srgb, with no way to get it
back other than to guess that alpha = srgb.

The application shouldn't require a visual without srgb advertised to
produce srgb-capable framebuffers, so that's an application bug.
However the situation with srgb vs non-srgb was also a little
confusing, so I think it's worth straightening it out.

  -ilia