On Fri, Dec 11, 2015 at 1:02 PM, Kristian Høgsberg <[email protected]> wrote: > Neil Roberts <[email protected]> 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" <[email protected]> >> Cc: Ilia Mirkin <[email protected]> >> Suggested-by: Ilia Mirkin <[email protected]> >> --- >> 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 _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
