<div dir="ltr">On 29 December 2012 04:35, Chris Forbes <<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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 <<a href="mailto:chrisf@ijw.co.nz">chrisf@ijw.co.nz</a>> --- 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? */ </blockquote><div> </div><div>Yeah, this assertion isn't going to work for Gen7's non-interleaved multisample arrays. Note that the Intel hardware docs use the term "physical" to describe the dimensions of the data structure that stores the multisampled data (i.e. after width, height, and/or depth have been scaled up to account for the number of samples) and "logical" to describe the dimensions of the image from the client's point of view. I think I'm going to start adopting that terminology for clarity. Bearing that in mind, the way the format works for Gen7 is that for each logical plane requested by the client using the depth parameter, N contiguous physical planes are allocated by hardware, one for each sample. So, for example, if the user allocates a buffer of depth 2 using 4x MSAA, the surface we allocate will need to have a physical depth of 8, with the first 4 physical planes corresponding to samples 0-3 of the logical plane having z coordinate 0, and the next 4 physical planes corresponding to samples 0-3 of the logical plane having z coordinate 1. So I think what we need to do here is remove the assertion and update depth like so: </div><div>depth *= num_samples </div><div>(of course everything is going to look a little different assuming we move this logic into common code that's shared by intel_miptree_create_for{texture,renderbuffer}). </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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); </blockquote><div> </div><div>I don't think you need the assertion here--intel_miptree_alloc_mcs() reads the depth out of the miptree, so it should allocate the proper size buffer without us having to do any extra work here. Having said that, I think I just discovered a bug: intel_miptree_alloc_mcs() allocates a surface whose depth is equal to mt->depth0, which is the physical depth. But it should be using the logical depth, since the MCS buffer stores data per pixel, not per sample. It's a fairly benign bug--it just means we're allocating more memory than we need--but embarrassing nonetheless. </div><div>I'm beginning to think that what we really ought to do is: </div><div>- Rename intel_mipmap_tree::{width,height,depth}0 to intel_mipmap_tree::physical_{width,height,depth}0 to clarify that they represent the width, height, and depth *after* appropriate multiplications have been made to account for the number of samples. </div><div>- Rename intel_mipmap_tree::singlesample_{width,height}0 to intel_mipmap_tree::logical_{width,height}0 to clarify that they represent the width and height of the surface from the client's point of view. </div><div>- Add intel_mipmap_tree::logical_depth0 to represent the depth of the surface from the client's point of view. </div><div>- Modify the code that initializes intel_mipmap_tree so that logical_{width,heigt,depth}0 are always initialized (at the moment they're only initialized for multisampled surfaces, and I think that leads to confusion). </div><div>Unless someone has objections, I'll try to submit a patch to do it ASAP (and fix the overallocation of MCS memory), and then hopefully we can rebase Chris's patches on top of it. </div><div> </div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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); + } /* 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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>