On 11/18/2011 03:42 PM, Kenneth Graunke wrote: > On 11/17/2011 07:58 PM, Chad Versace wrote: >> Extract the body of the inner loop into a new function, >> intel_miptree_copy_slice(). >> >> This is in preparation for adding support for separate stencil and HiZ to >> intel_miptree_copy_teximage(). When copying a slice of a depthstencil >> miptree that uses separate stencil, we will also need to copy the >> corresponding slice of the stencil miptree. The easiest way to do this >> will be to call intel_miptree_copy_slice() recursively. Analogous >> reasoning applies to copying a slice of a depth miptree with HiZ. >> >> Signed-off-by: Chad Versace <chad.vers...@linux.intel.com> >> --- >> src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 122 >> +++++++++++++----------- >> 1 files changed, 66 insertions(+), 56 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c >> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c >> index 7f9e606..8f10101 100644 >> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c >> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c >> @@ -354,6 +354,69 @@ intel_miptree_get_image_offset(struct intel_mipmap_tree >> *mt, >> } >> } >> >> +static void >> +intel_miptree_copy_slice(struct intel_context *intel, >> + struct intel_mipmap_tree *dst_mt, >> + struct intel_mipmap_tree *src_mt, >> + int level, >> + int face, >> + int depth) >> + >> +{ >> + gl_format format = src_mt->format; >> + uint32_t width = src_mt->level[level].width; >> + uint32_t height = src_mt->level[level].height; >> + >> + assert(depth < src_mt->level[level].depth); >> + >> + if (dst_mt->compressed) { >> + uint32_t align_w, align_h; >> + intel_get_texture_alignment_unit(format, >> + &align_w, &align_h); >> + height = ALIGN(height, align_h) / align_h; >> + width = ALIGN(width, align_w); >> + } > > This wasn't originally inside the loop; you've effectively moved it > there. Since intel_get_texture_alignment_unit actually does some work > these days, I'm wondering if this could be a performance hit.
Patch 36/41 "Store miptree alignment units in the miptree" remove this call intel_get_texture_alignment_unit(). Does that solve your performance fear? > At any rate, it doesn't seem necessary...I'd probably just add > height/width function parameters and move this hunk back. It feels really strange to me to pass width and height parameters into intel_miptree_copy_slice(). I imagine someone in the future encountering the function and being puzzled: "The slice specifier (level, face, depth) is already passed into intel_miptree_copy_slice(). The width and height are determined by the slice, so why are width and height also passed? Are we perhaps not copying the entire slice?" I'd rather not make a potentially confusing design choice for the sake of a potential performance penalty that will be rendered moot in a future commit. ---- Chad Versace chad.vers...@linux.intel.com _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev