[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mesa3d-dev
Subject:    Re: [Mesa-dev] [PATCH 1/3] intel: Recognize GL_DEPTH_COMPONENT32 in
From:       Eric Anholt <eric () anholt ! net>
Date:       2011-06-30 16:02:31
Message-ID: 874o37nw88.fsf () eliezer ! anholt ! net
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Thu, 30 Jun 2011 06:28:13 -0600, Brian Paul <brian.e.paul@gmail.com> wrote:
> On Thu, Jun 30, 2011 at 12:04 AM, Kenneth Graunke <kenneth@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@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.

internalFormat in this case is supposed to determine what is copied, so
we have to look at it, not the (current, if any) texture format.  So, I
think _mesa_is_*_format() are going to be the right way to go.

Also, we've got separate stencil issues in this path we need to look
into.


[Attachment #5 (application/pgp-signature)]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[prev in list] [next in list] [prev in thread] [next in thread]