[Mesa-dev] [PATCH 1/2] dri/i965: extend GLES3 sRGB workaround to cover all byte orders

[Jason Ekstrand]

On Wed, Apr 6, 2016 at 10:35 PM, Haixia Shi <hshi at chromium.org> wrote:

> I am afraid that I am not the original author of the
> gles3_srgb_workaround, and from the comments it appears to only apply for
> argb formats, so I am not sure if extending it to all other formats would
> cause behavioral regression.
> On Apr 6, 2016 10:13 PM, "Jason Ekstrand" <jason at jlekstrand.net> wrote:
>
>>
>> On Apr 6, 2016 7:23 PM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:
>> >
>> > On Wednesday, April 6, 2016 4:43:39 PM PDT Haixia Shi wrote:
>> > > It is incorrect to assume BGRA byte order for the GLES3 sRGB
>> workaround.
>> > >
>> > > Signed-off-by: Haixia Shi <hshi at chromium.org>
>> > > Reviewed-by: Stéphane Marchesin <marcheu at chromium.org>
>> > > Cc: kenneth.w.graunke at intel.com
>> > >
>> > > Change-Id: I5a081d7eaa7544afff0e7874cffef80d3f69a401
>> > > ---
>> > >  src/mesa/drivers/dri/i965/brw_context.c | 18 ++++++++++++++++--
>> > >  1 file changed, 16 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
>> b/src/mesa/drivers/dri/
>> > i965/brw_context.c
>> > > index 2d480d0..cebfbda 100644
>> > > --- a/src/mesa/drivers/dri/i965/brw_context.c
>> > > +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> > > @@ -1151,9 +1151,23 @@ intel_gles3_srgb_workaround(struct brw_context
>> *brw,
>> > >      */
>> > >     fb->Visual.sRGBCapable = false;
>> > >     for (int i = 0; i < BUFFER_COUNT; i++) {
>> > > -      if (fb->Attachment[i].Renderbuffer &&
>> > > -          fb->Attachment[i].Renderbuffer->Format ==
>> > MESA_FORMAT_B8G8R8A8_SRGB) {
>> > > +      if (!fb->Attachment[i].Renderbuffer)
>> > > +         continue;
>> > > +      switch (fb->Attachment[i].Renderbuffer->Format) {
>> > > +      case MESA_FORMAT_A8B8G8R8_SRGB:
>> > > +         fb->Attachment[i].Renderbuffer->Format =
>> > MESA_FORMAT_A8B8G8R8_UNORM;
>> > > +         break;
>> > > +      case MESA_FORMAT_B8G8R8A8_SRGB:
>> > >           fb->Attachment[i].Renderbuffer->Format =
>> > MESA_FORMAT_B8G8R8A8_UNORM;
>> > > +         break;
>> > > +      case MESA_FORMAT_A8R8G8B8_SRGB:
>> > > +         fb->Attachment[i].Renderbuffer->Format =
>> > MESA_FORMAT_A8R8G8B8_UNORM;
>> > > +         break;
>> > > +      case MESA_FORMAT_R8G8B8A8_SRGB:
>> > > +         fb->Attachment[i].Renderbuffer->Format =
>> > MESA_FORMAT_R8G8B8A8_UNORM;
>> > > +         break;
>> > > +      default:
>> > > +         break;
>> > >        }
>> > >     }
>> > >  }
>> > >
>> >
>> > Why don't we simply do:
>> >
>> >    struct gl_renderbuffer *rb = fb->Attachment[i].Renderbuffer;
>> >    rb->Format = _mesa_get_srgb_format_linear(rb->Format);
>> >
>> > This would handle far more formats than we need to, but that shouldn't
>> > be a problem.
>>
>> I'll second that.
>>
>
Look at the comment right above the function in question, I think Ken's
suggest is exactly the right thing to do.  There's nothing involved here
that should be 8888-specific.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160408/5fc915a6/attachment-0001.html>