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

[Chad Versace]

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.