Re: [Mesa-dev] [PATCH 1/2] st: Allow accelerated CopyTexImage from RGBA to RGB.

2018-03-22 Thread Brian Paul

On 03/22/2018 04:18 PM, Eric Anholt wrote:

There's nothing to worry about here -- the A channel just gets dropped by
the blit.  This avoids a segfault in the fallback path when copying from a
RGBA16_SINT renderbuffer to a RGB16_SINT destination represented by an
RGBA16_SINT texture (the fallback path tries to get/fetch to float
buffers, but the float pack/unpack functions are NULL for SINT/UINT).

Fixes KHR-GLES3.packed_pixels.pbo_rectangle.rgba16i on VC5.

v2: Extract the logic to a helper function and explain what's going on
 better.
---
  src/mesa/state_tracker/st_cb_texture.c | 32 ++--
  1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_texture.c 
b/src/mesa/state_tracker/st_cb_texture.c
index 6345ead6396b..5a23c7e8b6cd 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -2281,6 +2281,31 @@ fallback_copy_texsubimage(struct gl_context *ctx,
 pipe->transfer_unmap(pipe, src_trans);
  }
  
+static bool

+st_can_copyteximage_using_blit(struct gl_texture_image *texImage,
+   struct gl_renderbuffer *rb)


I think those params could be const-qualified.

Looks great otherwise.  Thanks.

Reviewed-by: Brian Paul 


+{
+   GLenum tex_baseformat = _mesa_get_format_base_format(texImage->TexFormat);
+
+   /* We don't blit to a teximage where the GL base format doesn't match the
+* texture's chosen format, except in the case of a GL_RGB texture
+* represented with GL_RGBA (where the alpha channel is just being
+* dropped).
+*/
+   if (texImage->_BaseFormat != tex_baseformat &&
+   ((texImage->_BaseFormat != GL_RGB || tex_baseformat != GL_RGBA))) {
+  return false;
+   }
+
+   /* We can't blit from a RB where the GL base format doesn't match the RB's
+* chosen format (for example, GL RGB or ALPHA with rb->Format of an RGBA
+* type, because the other channels will be undefined).
+*/
+   if (rb->_BaseFormat != _mesa_get_format_base_format(rb->Format))
+  return false;
+
+   return true;
+}
  
  /**

   * Do a CopyTex[Sub]Image1/2/3D() using a hardware (blit) path if possible.
@@ -2324,12 +2349,7 @@ st_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
goto fallback;
 }
  
-   /* The base internal format must match the mesa format, so make sure

-* e.g. an RGB internal format is really allocated as RGB and not as RGBA.
-*/
-   if (texImage->_BaseFormat !=
-   _mesa_get_format_base_format(texImage->TexFormat) ||
-   rb->_BaseFormat != _mesa_get_format_base_format(rb->Format)) {
+   if (!st_can_copyteximage_using_blit(texImage, rb)) {
goto fallback;
 }
  



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


[Mesa-dev] [PATCH 1/2] st: Allow accelerated CopyTexImage from RGBA to RGB.

2018-03-22 Thread Eric Anholt
There's nothing to worry about here -- the A channel just gets dropped by
the blit.  This avoids a segfault in the fallback path when copying from a
RGBA16_SINT renderbuffer to a RGB16_SINT destination represented by an
RGBA16_SINT texture (the fallback path tries to get/fetch to float
buffers, but the float pack/unpack functions are NULL for SINT/UINT).

Fixes KHR-GLES3.packed_pixels.pbo_rectangle.rgba16i on VC5.

v2: Extract the logic to a helper function and explain what's going on
better.
---
 src/mesa/state_tracker/st_cb_texture.c | 32 ++--
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_texture.c 
b/src/mesa/state_tracker/st_cb_texture.c
index 6345ead6396b..5a23c7e8b6cd 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -2281,6 +2281,31 @@ fallback_copy_texsubimage(struct gl_context *ctx,
pipe->transfer_unmap(pipe, src_trans);
 }
 
+static bool
+st_can_copyteximage_using_blit(struct gl_texture_image *texImage,
+   struct gl_renderbuffer *rb)
+{
+   GLenum tex_baseformat = _mesa_get_format_base_format(texImage->TexFormat);
+
+   /* We don't blit to a teximage where the GL base format doesn't match the
+* texture's chosen format, except in the case of a GL_RGB texture
+* represented with GL_RGBA (where the alpha channel is just being
+* dropped).
+*/
+   if (texImage->_BaseFormat != tex_baseformat &&
+   ((texImage->_BaseFormat != GL_RGB || tex_baseformat != GL_RGBA))) {
+  return false;
+   }
+
+   /* We can't blit from a RB where the GL base format doesn't match the RB's
+* chosen format (for example, GL RGB or ALPHA with rb->Format of an RGBA
+* type, because the other channels will be undefined).
+*/
+   if (rb->_BaseFormat != _mesa_get_format_base_format(rb->Format))
+  return false;
+
+   return true;
+}
 
 /**
  * Do a CopyTex[Sub]Image1/2/3D() using a hardware (blit) path if possible.
@@ -2324,12 +2349,7 @@ st_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
   goto fallback;
}
 
-   /* The base internal format must match the mesa format, so make sure
-* e.g. an RGB internal format is really allocated as RGB and not as RGBA.
-*/
-   if (texImage->_BaseFormat !=
-   _mesa_get_format_base_format(texImage->TexFormat) ||
-   rb->_BaseFormat != _mesa_get_format_base_format(rb->Format)) {
+   if (!st_can_copyteximage_using_blit(texImage, rb)) {
   goto fallback;
}
 
-- 
2.16.2

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