On Wed, Nov 29, 2017 at 4:42 PM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Tue 28 Nov 2017, Jason Ekstrand wrote:
> >
> >
> > On Tue, Nov 21, 2017 at 3:05 PM, Chad Versace <[1]
> chadvers...@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 <chadvers...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
