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

2011-06-30 Thread Kenneth Graunke 
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
ARGB 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 kenn...@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);
-- 
1.7.6

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

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

2011-06-30 Thread Brian Paul 
On Thu, Jun 30, 2011 at 12:04 AM, Kenneth Graunke kenn...@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
 ARGB 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 kenn...@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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

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


pgp75jfHBBBEL.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev