[Mesa-dev] [PATCH 1/3] intel: Recognize GL_DEPTH_COMPONENT32 in get_teximage_readbuffer.

[Brian Paul]

On Thu, Jun 30, 2011 at 12:04 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> gl_texture_image::InternalFormat is actually the user requested internal
> format, not what the texture actually is.  Thus, even though we don't
> support 32-bit depth buffers, we need to recognize the enumeration here.
> Otherwise, it wrongly returns the color read buffer instead of the depth
> read buffer.
>
> Fixes an issue in PlaneShift 0.5.7 when casting spells.  The game calls
> CopyTexSubImage2D on buffers with a GL_DEPTH_COMPONENT32 internal
> format, which (prior to this patch) resulted in an attempt to copy an
> ARGB8888 to S8_Z24.  This patch fixes the behavior, but does not yet
> eliminate the software fallback.
>
> NOTE: This is a candidate for the 7.10 and 7.11 branches.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/intel/intel_tex_copy.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> I kind of wonder if we should just be using TexFormat (the actual format)
> rather than InternalFormat (the user requested format).
>
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_copy.c b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> index eda07a4..8b5c3f0 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_copy.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> @@ -58,6 +58,7 @@ get_teximage_readbuffer(struct intel_context *intel, GLenum internalFormat)
>    switch (internalFormat) {
>    case GL_DEPTH_COMPONENT:
>    case GL_DEPTH_COMPONENT16:
> +   case GL_DEPTH_COMPONENT32:
>    case GL_DEPTH24_STENCIL8_EXT:
>    case GL_DEPTH_STENCIL_EXT:
>       return intel_get_renderbuffer(intel->ctx.ReadBuffer, BUFFER_DEPTH);

In the interest of covering all current and future depth formats, you
could replace the switch with a call to _mesa_is_depth_format() ||
_mesa_is_depthstencil_format().  Or don't use internalFormat at all-
query _mesa_get_format_bits(texImage->TexFormat, GL_DEPTH_BITS) > 0.

In fact, any place where we're doing a switch on a texture/image
format we should look if the job can be done better with a call to a
format predicate function.

-Brian