[Mesa-dev] [PATCH 08/41] intel: Replace intel_renderbuffer::region with a miptree

[Eric Anholt]

On Thu, 17 Nov 2011 19:58:35 -0800, Chad Versace <chad.versace at linux.intel.com> wrote:
> Essentially, this patch just globally substitutes `irb->region` with
> `irb->mt->region` and then does some minor cleanups to avoid segfaults
> and other problems.
> 
> This is in preparation for
>   1. Fixing scatter/gather for mipmapped separate stencil textures.
>   2. Supporting HiZ for mipmapped depth textures.
> 
> As a nice benefit, this lays down some preliminary groundwork for easily
> texturing from any renderbuffer, even those of the window system.
> 
> A future commit will replace intel_mipmap_tree::hiz_region with a miptree.
> 
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>

In case I get distracted before finishing, patches 1-7 are

Reviewed-by: Eric Anholt <eric at anholt.net>

> ---
> @@ -1381,9 +1396,12 @@ intel_process_dri2_buffer_with_separate_stencil(struct intel_context *intel,
>  				    buffer_name);
>  
>     if (buffer->attachment == __DRI_BUFFER_HIZ) {
> -      intel_region_reference(&rb->hiz_region, region);
> +      intel_region_reference(&rb->mt->hiz_region, region);
>     } else {
> -      intel_region_reference(&rb->region, region);
> +      rb->mt = intel_miptree_create_for_region(intel,
> +                                               GL_TEXTURE_2D,
> +                                               rb->Base.Format,
> +                                               region);;
>     }

Leaked old rb->mt here?  I don't see this function kicking off with an
intel_miptree_release(&rb->mt).  Next quoted hunk has the same issue.

>     if (buffer->attachment == __DRI_BUFFER_HIZ) {
> -      intel_region_reference(&rb->hiz_region, region);
> +      intel_region_reference(&rb->mt->hiz_region, region);
>     } else {
> -      intel_region_reference(&rb->region, region);
> +      rb->mt = intel_miptree_create_for_region(intel,
> +                                               GL_TEXTURE_2D,
> +                                               rb->Base.Format,
> +                                               region);;

extra ';'

> -	 intel_region_reference(&intel_get_renderbuffer(fb, BUFFER_DEPTH)->region,
> -				region);
> -	 intel_region_reference(&intel_get_renderbuffer(fb, BUFFER_STENCIL)->region,
> -				region);
> +	 struct intel_mipmap_tree *mt =
> +	       intel_miptree_create_for_region(intel,
> +	                                       GL_TEXTURE_2D,
> +	                                       depth_stencil_rb->Base.Format,
> +	                                       region);
>  	 intel_region_release(&region);
> +	 intel_miptree_reference(&intel_get_renderbuffer(fb, BUFFER_DEPTH)->mt, mt);
> +	 intel_miptree_reference(&intel_get_renderbuffer(fb, BUFFER_STENCIL)->mt, mt);

Haven't you leaked a mt reference here?
>     }
>  
>     irb = intel_renderbuffer(rb);
> -   intel_region_reference(&irb->region, image->region);
> +   irb->mt = intel_miptree_create_for_region(intel,
> +                                             GL_TEXTURE_2D,
> +                                             image->format,
> +                                             image->region);
> +   if (!irb->mt)
> +      return;

Leak of existing irb->mt?

-------------- 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/20111118/48c1a49d/attachment.pgp>