[Mesa-dev] [RFC PATCH 21/26] i965: add support for multisample textures

Paul Berry stereotype441 at gmail.com
Fri Jan 4 13:13:40 PST 2013


On 29 December 2012 04:35, Chris Forbes <chrisf at ijw.co.nz> wrote:

> This is a bit nasty -- I've just (ab)used the existing multisample
> renderbuffer path.
>
> Also put the actual sample count from the mt back into the
> renderbuffer so core mesa can see what the driver actually gave it.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  2 ++
>  src/mesa/drivers/dri/intel/intel_fbo.c           |  3 ++-
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.c   | 30
> ++++++++++++++++++++----
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.h   |  2 ++
>  src/mesa/drivers/dri/intel/intel_tex.c           | 18 +++++++++++---
>  5 files changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 94c01cb..1c54d59 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -59,6 +59,8 @@ translate_tex_target(GLenum target)
>     case GL_TEXTURE_2D:
>     case GL_TEXTURE_2D_ARRAY_EXT:
>     case GL_TEXTURE_EXTERNAL_OES:
> +   case GL_TEXTURE_2D_MULTISAMPLE:
> +   case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>        return BRW_SURFACE_2D;
>
>     case GL_TEXTURE_3D:
> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
> b/src/mesa/drivers/dri/intel/intel_fbo.c
> index 6a66521..4cc2c00 100644
> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> @@ -270,7 +270,7 @@ intel_alloc_renderbuffer_storage(struct gl_context *
> ctx, struct gl_renderbuffer
>        return true;
>
>     irb->mt = intel_miptree_create_for_renderbuffer(intel, rb->Format,
> -                                                  width, height,
> +                                                  width, height, 1,
>                                                     rb->NumSamples);
>     if (!irb->mt)
>        return false;
> @@ -509,6 +509,7 @@ intel_renderbuffer_update_wrapper(struct intel_context
> *intel,
>     rb->_BaseFormat = image->_BaseFormat;
>     rb->Width = mt->level[level].width;
>     rb->Height = mt->level[level].height;
> +   rb->NumSamples = mt->num_samples;
>
>     rb->Delete = intel_delete_renderbuffer;
>     rb->AllocStorage = intel_nop_alloc_storage;
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index 9ae7bcc..989c1f6 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -404,6 +404,7 @@ intel_miptree_create_for_dri2_buffer(struct
> intel_context *intel,
>                                                            format,
>                                                            region->width,
>                                                            region->height,
> +                                                          1,
>                                                            num_samples);
>     if (!multisample_mt) {
>        intel_miptree_release(&singlesample_mt);
> @@ -427,10 +428,10 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
>                                        gl_format format,
>                                        uint32_t width,
>                                        uint32_t height,
> +                                      uint32_t depth,
>                                        uint32_t num_samples)
>  {
>     struct intel_mipmap_tree *mt;
> -   uint32_t depth = 1;
>     enum intel_msaa_layout msaa_layout = INTEL_MSAA_LAYOUT_NONE;
>     const uint32_t singlesample_width = width;
>     const uint32_t singlesample_height = height;
> @@ -491,6 +492,7 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
>           }
>        } else {
>           /* Non-interleaved */
> +         assert(depth == 1);  /* XXX: do we ever hit this path with
> multisample arrays? */
>           depth = num_samples;
>        }
>     }
> @@ -508,6 +510,7 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
>     }
>
>     if (mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) {
> +      assert(depth == 1);  /* XXX: multisample arrays interaction with
> MCS? */
>        ok = intel_miptree_alloc_mcs(intel, mt, num_samples);
>        if (!ok)
>           goto fail;
> @@ -624,9 +627,27 @@ intel_miptree_match_image(struct intel_mipmap_tree
> *mt,
>      * minification.  This will also catch images not present in the
>      * tree, changed targets, etc.
>      */
> -   if (width != mt->level[level].width ||
> -       height != mt->level[level].height ||
> -       depth != mt->level[level].depth)
> +   if (mt->target == GL_TEXTURE_2D_MULTISAMPLE ||
> +         mt->target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) {
> +      /* nonzero level here is always bogus */
> +      assert(level == 0);
> +
> +      if (width != mt->singlesample_width0 ||
> +            height != mt->singlesample_height0 ||
> +            depth != mt->level[level].depth) {
> +         return false;
> +      }
> +   }
> +   else {
> +      /* all normal textures, renderbuffers, etc */
> +      if (width != mt->level[level].width ||
> +          height != mt->level[level].height ||
> +          depth != mt->level[level].depth) {
> +         return false;
> +      }
> +   }
> +
> +   if (image->TexObject->NumSamples != mt->num_samples)
>        return false;
>
>     return true;
> @@ -1644,6 +1665,7 @@ intel_miptree_map_multisample(struct intel_context
> *intel,
>                                                 mt->format,
>                                                 mt->singlesample_width0,
>                                                 mt->singlesample_height0,
> +                                               1,
>                                                 0 /*num_samples*/);
>        if (!mt->singlesample_mt)
>           goto fail;
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> index eb4ad7f..3de0019 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> @@ -405,12 +405,14 @@ intel_miptree_create_for_dri2_buffer(struct
> intel_context *intel,
>   *     - The target is GL_TEXTURE_2D.
>   *     - There are no levels other than the base level 0.
>   *     - Depth is 1.
> + *     XXX: this description is quite bogus now.
>   */
>  struct intel_mipmap_tree*
>  intel_miptree_create_for_renderbuffer(struct intel_context *intel,
>                                        gl_format format,
>                                        uint32_t width,
>                                        uint32_t height,
> +                                      uint32_t depth,
>                                        uint32_t num_samples);
>
>  /** \brief Assert that the level and layer are valid for the miptree. */
> diff --git a/src/mesa/drivers/dri/intel/intel_tex.c
> b/src/mesa/drivers/dri/intel/intel_tex.c
> index fdf5812..5f0301c 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex.c
> @@ -71,6 +71,7 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
>     switch (texobj->Target) {
>     case GL_TEXTURE_3D:
>     case GL_TEXTURE_2D_ARRAY:
> +   case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>        slices = image->Depth;
>        break;
>     case GL_TEXTURE_1D_ARRAY:
> @@ -91,9 +92,20 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
>            __FUNCTION__, texobj, image->Level,
>            image->Width, image->Height, image->Depth, intel_texobj->mt);
>     } else {
> -      intel_image->mt = intel_miptree_create_for_teximage(intel,
> intel_texobj,
> -                                                          intel_image,
> -                                                          false);
> +
> +      if (texobj->NumSamples) {
> +         /* Treat multisample textures exactly like renderbuffers */
> +         intel_image->mt = intel_miptree_create_for_renderbuffer(intel,
> +               image->TexFormat, image->Width, image->Height,
> image->Depth,
> +               texobj->NumSamples);
> +         /* Renderbuffer path doesn't bother to set the mt's target, so
> do it here */
> +         intel_image->mt->target = texobj->Target;
> +      }
> +      else {
> +         intel_image->mt = intel_miptree_create_for_teximage(intel,
> intel_texobj,
> +                                                             intel_image,
> +                                                             false);
> +      }
>

I assume you are doing this because you want to take advantage of the
multisampling-specific code in intel_miptree_create_for_renderbuffer (e.g.
the code that chooses the MSAA layout and adjusts height, width, and depth
for MSAA).  That seems like a kludge that's going to lead to maintenance
headaches down the road, because when we're working on
intel_miptree_create_for_renderbuffer, we'll forget that it's sometimes
used for textures too.  Is that the nastiness you were referring to in the
commit message?  I would rather see the multisample-specific code moved out
of intel_miptree_create_for_renderbuffer and into some common helper
function that both of them call (intel_miptree_create if possible,
otherwise some new function).  That way intel_alloc_texture_image_buffer
can continue unconiditionally calling intel_miptree_create_for_teximage.


>
>        /* Even if the object currently has a mipmap tree associated
>         * with it, this one is a more likely candidate to represent the
> --
> 1.8.0.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130104/d9b6668c/attachment-0001.html>


More information about the mesa-dev mailing list