[Mesa-dev] [PATCH 2/4] i965/tex_image: Reference the renderbuffer miptree in setTexBuffer2

[Jason Ekstrand]

On Wed, Nov 29, 2017 at 4:42 PM, Chad Versace <chadversary at chromium.org>
wrote:

> On Tue 28 Nov 2017, Jason Ekstrand wrote:
> >
> >
> > On Tue, Nov 21, 2017 at 3:05 PM, Chad Versace <[1]
> chadversary at chromium.org>
> > wrote:
>
> >     > @@ -442,7 +443,6 @@ intelSetTexBuffer2(__DRIcontext *pDRICtx,
> GLint
> >     target,
> >     >     struct gl_texture_object *texObj;
> >     >     struct gl_texture_image *texImage;
> >     >     mesa_format texFormat = MESA_FORMAT_NONE;
> >     > -   struct intel_mipmap_tree *mt;
> >     >     GLenum internal_format = 0;
> >     >
> >     >     texObj = _mesa_get_current_tex_object(ctx, target);
> >     > @@ -464,31 +464,24 @@ intelSetTexBuffer2(__DRIcontext *pDRICtx,
> GLint
> >     target,
> >     >     if (rb->mt->cpp == 4) {
> >     >        if (texture_format == __DRI_TEXTURE_FORMAT_RGB) {
> >     >           internal_format = GL_RGB;
> >     > -         texFormat = MESA_FORMAT_B8G8R8X8_UNORM;
> >     > +         texFormat = MESA_FORMAT_B8G8R8A8_UNORM;
> >
> >     Why replace rgbx with rgba? I suspect the replace is due to the same
> >     reasons explained in intel_miptree_create_for_dri_image(). Whatever
> the
> >     reasons are, they're subtle and deserve a comment.
> >
> >
> > I believe your fears go away if you re-order things and put 3 before 2.
> Why
> > RGBA instead of RGBX?  Because the underlying miptree of the
> renderbuffer is
> > likely to have that format.  That said, it's not actually guaranteed so
> making
> > that change in this patch is a bit bogus.  If we just make the change in
> 2
> > instead, I believe all bogosity is gone.
>
> If I conceptually place patch 3 before patch 2, I see the correctness of
> everything. That makes this patch (patch 2)
>
> Reviewed-by: Chad Versace <chadversary at chromium.org>
>
> If choose to fidget the code a little in this patch due to my complaint,
> my rb still stands.
>

I sent 6 patches yesterday with some fidgeting.  No, I did not use a
spinner.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171129/b43c097d/attachment.html>