<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 29, 2017 at 4:42 PM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue 28 Nov 2017, Jason Ekstrand wrote:<br>
<div><div class="h5">><br>
><br>
> On Tue, Nov 21, 2017 at 3:05 PM, Chad Versace <[1]<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>><br>
> wrote:<br>
<br>
>     > @@ -442,7 +443,6 @@ intelSetTexBuffer2(__<wbr>DRIcontext *pDRICtx, GLint<br>
>     target,<br>
>     >     struct gl_texture_object *texObj;<br>
>     >     struct gl_texture_image *texImage;<br>
>     >     mesa_format texFormat = MESA_FORMAT_NONE;<br>
>     > -   struct intel_mipmap_tree *mt;<br>
>     >     GLenum internal_format = 0;<br>
>     ><br>
>     >     texObj = _mesa_get_current_tex_object(<wbr>ctx, target);<br>
>     > @@ -464,31 +464,24 @@ intelSetTexBuffer2(__<wbr>DRIcontext *pDRICtx, GLint<br>
>     target,<br>
>     >     if (rb->mt->cpp == 4) {<br>
>     >        if (texture_format == __DRI_TEXTURE_FORMAT_RGB) {<br>
>     >           internal_format = GL_RGB;<br>
>     > -         texFormat = MESA_FORMAT_B8G8R8X8_UNORM;<br>
>     > +         texFormat = MESA_FORMAT_B8G8R8A8_UNORM;<br>
><br>
>     Why replace rgbx with rgba? I suspect the replace is due to the same<br>
>     reasons explained in intel_miptree_create_for_dri_<wbr>image(). Whatever the<br>
>     reasons are, they're subtle and deserve a comment.<br>
><br>
><br>
> I believe your fears go away if you re-order things and put 3 before 2.  Why<br>
> RGBA instead of RGBX?  Because the underlying miptree of the renderbuffer is<br>
> likely to have that format.  That said, it's not actually guaranteed so making<br>
> that change in this patch is a bit bogus.  If we just make the change in 2<br>
> instead, I believe all bogosity is gone.<br>
<br>
</div></div>If I conceptually place patch 3 before patch 2, I see the correctness of<br>
everything. That makes this patch (patch 2)<br>
<br>
Reviewed-by: Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>><br>
<br>
If choose to fidget the code a little in this patch due to my complaint,<br>
my rb still stands.<br>
</blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">I sent 6 patches yesterday with some fidgeting.  No, I did not use a spinner.</div><div class="gmail_extra"><br></div><div class="gmail_extra">--Jason<br></div></div>