[Mesa-dev] [PATCH 5/6] mesa: Make Mesa core set up wrapped texture renderbuffer state.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On 04/30/2013 12:56 PM, Eric Anholt wrote:
>> Everyone was doing effectively the same thing, except for some funky code
>> reuse in Intel, and swrast mistakenly recomputing _BaseFormat instead of
>> using the texture's _BaseFormat.  swrast's sRGB handling is left in place,
>> though it should be done by using _mesa_get_render_format() at render time
>> instead (as-is, it will miss updates to GL_FRAMEBUFFER_SRGB).
>> ---
>>   src/mesa/drivers/dri/intel/intel_fbo.c     |  6 ------
>>   src/mesa/drivers/dri/nouveau/nouveau_fbo.c | 18 ------------------
>>   src/mesa/main/fbobject.c                   |  7 +++++++
>>   src/mesa/state_tracker/st_cb_fbo.c         |  5 -----
>>   src/mesa/swrast/s_texrender.c              |  5 -----
>>   5 files changed, 7 insertions(+), 34 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c
>> index a3817eb..f037445 100644
>> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
>> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
>> @@ -489,12 +489,6 @@ intel_renderbuffer_update_wrapper(struct intel_context *intel,
>>      struct intel_mipmap_tree *mt = intel_image->mt;
>>      int level = image->Level;
>>
>> -   rb->Format = image->TexFormat;
>> -   rb->InternalFormat = image->InternalFormat;
>> -   rb->_BaseFormat = image->_BaseFormat;
>> -   rb->NumSamples = mt->num_samples;
>> -   rb->Width = image->Width2;
>> -   rb->Height = image->Height2;
>>      rb->Delete = intel_delete_renderbuffer;
>>      rb->AllocStorage = intel_nop_alloc_storage;
>>
>> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_fbo.c b/src/mesa/drivers/dri/nouveau/nouveau_fbo.c
>> index adead3d..a692051 100644
>> --- a/src/mesa/drivers/dri/nouveau/nouveau_fbo.c
>> +++ b/src/mesa/drivers/dri/nouveau/nouveau_fbo.c
>> @@ -247,21 +247,6 @@ nouveau_framebuffer_renderbuffer(struct gl_context *ctx, struct gl_framebuffer *
>>   	context_dirty(ctx, FRAMEBUFFER);
>>   }
>>
>> -static GLenum
>> -get_tex_format(struct gl_texture_image *ti)
>> -{
>> -	switch (ti->TexFormat) {
>> -	case MESA_FORMAT_ARGB8888:
>> -		return GL_RGBA8;
>> -	case MESA_FORMAT_XRGB8888:
>> -		return GL_RGB8;
>> -	case MESA_FORMAT_RGB565:
>> -		return GL_RGB5;
>> -	default:
>> -		return GL_NONE;
>> -	}
>> -}
>> -
>>   static void
>>   nouveau_render_texture(struct gl_context *ctx, struct gl_framebuffer *fb,
>>   		       struct gl_renderbuffer_attachment *att)
>> @@ -271,9 +256,6 @@ nouveau_render_texture(struct gl_context *ctx, struct gl_framebuffer *fb,
>>   		att->Texture->Image[att->CubeMapFace][att->TextureLevel];
>>
>>   	/* Update the renderbuffer fields from the texture. */
>> -	set_renderbuffer_format(rb, get_tex_format(ti));
>
> I think the call to set_renderbuffer_format is still necessary, since it 
> does:
>
> struct nouveau_surface *s = &to_nouveau_renderbuffer(rb)->surface;
> s->cpp = ...;
>
> and the core Mesa code doesn't do that.  Otherwise, this looks good.
>
> I looked over the series for about an hour and didn't notice any other 
> large issues.  I'm really glad to see this cleaned up - it was really 
> awkward having the core Mesa code rely on driver hooks setting up the 
> renderbuffer wrapper.
>
> For the series (assuming this nouveau thing gets fixed):
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

The s->cpp and s->format were immediately getting set to the texture's
cpp and format by:

        nouveau_surface_ref(&to_nouveau_teximage(ti)->surface,
                            &to_nouveau_renderbuffer(rb)->surface);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130506/3384150c/attachment.pgp>