<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 6, 2016 at 10:35 PM, Haixia Shi <span dir="ltr"><<a href="mailto:hshi@chromium.org" target="_blank">hshi@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">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.</p><div class="HOEnZb"><div class="h5">
<div class="gmail_quote">On Apr 6, 2016 10:13 PM, "Jason Ekstrand" <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"><br>
On Apr 6, 2016 7:23 PM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote:<br>
><br>
> On Wednesday, April 6, 2016 4:43:39 PM PDT Haixia Shi wrote:<br>
> > It is incorrect to assume BGRA byte order for the GLES3 sRGB workaround.<br>
> ><br>
> > Signed-off-by: Haixia Shi <<a href="mailto:hshi@chromium.org" target="_blank">hshi@chromium.org</a>><br>
> > Reviewed-by: Stéphane Marchesin <<a href="mailto:marcheu@chromium.org" target="_blank">marcheu@chromium.org</a>><br>
> > Cc: <a href="mailto:kenneth.w.graunke@intel.com" target="_blank">kenneth.w.graunke@intel.com</a><br>
> ><br>
> > Change-Id: I5a081d7eaa7544afff0e7874cffef80d3f69a401<br>
> > ---<br>
> >  src/mesa/drivers/dri/i965/brw_context.c | 18 ++++++++++++++++--<br>
> >  1 file changed, 16 insertions(+), 2 deletions(-)<br>
> ><br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/<br>
> i965/brw_context.c<br>
> > index 2d480d0..cebfbda 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_context.c<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c<br>
> > @@ -1151,9 +1151,23 @@ intel_gles3_srgb_workaround(struct brw_context *brw,<br>
> >      */<br>
> >     fb->Visual.sRGBCapable = false;<br>
> >     for (int i = 0; i < BUFFER_COUNT; i++) {<br>
> > -      if (fb->Attachment[i].Renderbuffer &&<br>
> > -          fb->Attachment[i].Renderbuffer->Format ==<br>
> MESA_FORMAT_B8G8R8A8_SRGB) {<br>
> > +      if (!fb->Attachment[i].Renderbuffer)<br>
> > +         continue;<br>
> > +      switch (fb->Attachment[i].Renderbuffer->Format) {<br>
> > +      case MESA_FORMAT_A8B8G8R8_SRGB:<br>
> > +         fb->Attachment[i].Renderbuffer->Format =<br>
> MESA_FORMAT_A8B8G8R8_UNORM;<br>
> > +         break;<br>
> > +      case MESA_FORMAT_B8G8R8A8_SRGB:<br>
> >           fb->Attachment[i].Renderbuffer->Format =<br>
> MESA_FORMAT_B8G8R8A8_UNORM;<br>
> > +         break;<br>
> > +      case MESA_FORMAT_A8R8G8B8_SRGB:<br>
> > +         fb->Attachment[i].Renderbuffer->Format =<br>
> MESA_FORMAT_A8R8G8B8_UNORM;<br>
> > +         break;<br>
> > +      case MESA_FORMAT_R8G8B8A8_SRGB:<br>
> > +         fb->Attachment[i].Renderbuffer->Format =<br>
> MESA_FORMAT_R8G8B8A8_UNORM;<br>
> > +         break;<br>
> > +      default:<br>
> > +         break;<br>
> >        }<br>
> >     }<br>
> >  }<br>
> ><br>
><br>
> Why don't we simply do:<br>
><br>
>    struct gl_renderbuffer *rb = fb->Attachment[i].Renderbuffer;<br>
>    rb->Format = _mesa_get_srgb_format_linear(rb->Format);<br>
><br>
> This would handle far more formats than we need to, but that shouldn't<br>
> be a problem.</p>
<p dir="ltr">I'll second that.</p></blockquote></div></div></div></blockquote><div><br>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.<br></div><div>--Jason<br></div></div><br></div></div>