On Thu, 17 Nov 2011 19:58:42 -0800, Chad Versace <chad.vers...@linux.intel.com>
wrote:
> This is in preparation for properly implementing glFramebufferTexture*()
> for mipmapped depthstencil textures. The FIXME comments deleted by this
> patch give a rough explanation of what was broken.
>
> This refactor does the following:
> - In intel_update_wrapper() and intel_wrap_texture(), prepare to
> replace the 'att' parameter with a miptree.
> - Move the call to intel_renderbuffer_set_draw_offsets() from
> intel_render_texture() into intel_udpate_wrapper().
>
> Each time I encounter those functions, I dislike their vague names.
> (Update which wrapper? What is wrapped? What is the wrapper?). So, while
> I was mucking around, I also renamed the functions.
>
> Signed-off-by: Chad Versace <chad.vers...@linux.intel.com>
> ---
> src/mesa/drivers/dri/intel/intel_fbo.c | 113
> +++++++++++++++++++++++---------
> 1 files changed, 81 insertions(+), 32 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
> b/src/mesa/drivers/dri/intel/intel_fbo.c
> index 8e4f7a9..a61c74c 100644
> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> @@ -945,41 +945,54 @@ intel_framebuffer_renderbuffer(struct gl_context * ctx,
> intel_draw_buffer(ctx);
> }
>
> +/**
> + * NOTE: The 'att' parameter is a kludge that will soon be removed. Its
> + * presence allows us to refactor the wrapping of depthstencil textures that
> + * use separate stencil in two easily manageable steps, rather than in one
> + * large, hairy step. First, refactor the common wrapping code used by all
> + * texture formats. Second, refactor the separate stencil code paths.
> + */
> static bool
> -intel_update_wrapper(struct gl_context *ctx, struct intel_renderbuffer *irb,
> - struct gl_renderbuffer_attachment *att)
> +intel_renderbuffer_update_wrapper(struct intel_context *intel,
> + struct intel_renderbuffer *irb,
> + struct intel_mipmap_tree *mt,
> + uint32_t level,
> + uint32_t layer,
> + GLenum internal_format,
> + struct gl_renderbuffer_attachment *att)
> {
> + struct gl_context *ctx = &intel->ctx;
> + struct gl_renderbuffer *rb = &irb->Base;
> +
> + /* The image variables are a kludge. See the note above for the att
> + * parameter.
> + */
> struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
> struct intel_texture_image *intel_image = intel_texture_image(texImage);
> - int width, height, depth;
>
> - if (!intel_span_supports_format(texImage->TexFormat)) {
> + irb->Base.Format = ctx->Driver.ChooseTextureFormat(ctx, internal_format,
> + GL_NONE, GL_NONE);
> +
> + if (!intel_span_supports_format(rb->Format)) {
> DBG("Render to texture BAD FORMAT %s\n",
> - _mesa_get_format_name(texImage->TexFormat));
> + _mesa_get_format_name(rb->Format));
> return false;
> } else {
> - DBG("Render to texture %s\n",
> _mesa_get_format_name(texImage->TexFormat));
> + DBG("Render to texture %s\n", _mesa_get_format_name(rb->Format));
> }
>
> - intel_miptree_get_dimensions_for_image(texImage, &width, &height, &depth);
> -
> - irb->Base.Format = texImage->TexFormat;
> - irb->Base.DataType =
> intel_mesa_format_to_rb_datatype(texImage->TexFormat);
> - irb->Base.InternalFormat = texImage->InternalFormat;
> - irb->Base._BaseFormat = _mesa_base_tex_format(ctx,
> irb->Base.InternalFormat);
> - irb->Base.Width = width;
> - irb->Base.Height = height;
> + rb->InternalFormat = internal_format;
> + rb->DataType = intel_mesa_format_to_rb_datatype(rb->Format);
> + rb->_BaseFormat = _mesa_get_format_base_format(rb->Format);
> + rb->Width = mt->level[level].width;
> + rb->Height = mt->level[level].height;
>
> irb->Base.Delete = intel_delete_renderbuffer;
> irb->Base.AllocStorage = intel_nop_alloc_storage;
>
> - irb->mt_level = att->TextureLevel;
> - if (att->CubeMapFace > 0) {
> - assert(att->Zoffset == 0);
> - irb->mt_layer = att->CubeMapFace;
> - } else {
> - irb->mt_layer= att->Zoffset;
> - }
> + intel_miptree_check_level_layer(mt, level, layer);
> + irb->mt_level = level;
> + irb->mt_layer = layer;
>
> if (intel_image->stencil_rb) {
> /* The tex image has packed depth/stencil format, but is using
> separate
> @@ -1002,29 +1015,46 @@ intel_update_wrapper(struct gl_context *ctx, struct
> intel_renderbuffer *irb,
> depth_irb = intel_renderbuffer(intel_image->depth_rb);
> depth_irb->mt_level = irb->mt_level;
> depth_irb->mt_layer = irb->mt_layer;
> + intel_renderbuffer_set_draw_offset(depth_irb);
>
> stencil_irb = intel_renderbuffer(intel_image->stencil_rb);
> stencil_irb->mt_level = irb->mt_level;
> stencil_irb->mt_layer = irb->mt_layer;
> + intel_renderbuffer_set_draw_offset(stencil_irb);
> } else {
> intel_miptree_reference(&irb->mt, intel_image->mt);
> + intel_renderbuffer_set_draw_offset(irb);
> }
> +
> return true;
> }
>
> /**
> - * When glFramebufferTexture[123]D is called this function sets up the
> - * gl_renderbuffer wrapper around the texture image.
> - * This will have the region info needed for hardware rendering.
> + * \brief Wrap a renderbuffer around a single slice of a miptree.
> + *
> + * Called by glFramebufferTexture*().
> + *
> + * NOTE: The 'att' parameter is a kludge that will soon be removed. Its
> + * presence allows us to refactor the wrapping of depthstencil textures that
> + * use separate stencil in two easily manageable steps, rather than in one
> + * large, hairy step. First, refactor the common wrapping code used by all
> + * texture formats. Second, refactor the separate stencil code paths.
> */
> -static struct intel_renderbuffer *
> -intel_wrap_texture(struct gl_context * ctx,
> - struct gl_renderbuffer_attachment *att)
> +static struct intel_renderbuffer*
> +intel_renderbuffer_wrap_miptree(struct intel_context *intel,
> + struct intel_mipmap_tree *mt,
> + uint32_t level,
> + uint32_t layer,
> + GLenum internal_format,
> + struct gl_renderbuffer_attachment *att)
> +I don't think this should be a separate function. I think the split between it and intel_update_wrapper() is just leftover mess from long ago before framebuffer completeness was worked out, and we would really use the _swrast_render_texture ptahs. The swrast path should be not happening today, since the renderbuffer won't be accessed because the FBO will be incomplete. I think folding those two together and not calling intel_update_wrapper twice instead of this patch would clean this code up more.
pgpUVeQn9a8Es.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
