[Mesa-dev] [PATCH] mesa: remove unnecessary checks in _mesa_readpixels_needs_slow_path

Iago Toral Quiroga itoral at igalia.com
Tue Jun 23 03:34:26 PDT 2015

readpixels_can_use_memcpy will later call _mesa_format_matches_format_and_type
which does much tighter checks than these to decide if we can use
memcpy for readpixels.

Also, the checks do not seem to be extensive enough anyway, since we are
checking for signed/unsigned conversion only when the framebuffer has integers,
but the same checks could be done for other types anyway, since as long as
there is a signed/unsigned conversion we can't memcpy.

No regressions observed on i965/llvmpipe.
The way gallium uses _mesa_format_matches_format_and_type and
_mesa_readpixels_needs_slow_path is a bit different, they call these
functions to decide if they want to fallback to Mesa's implementation
of ReadPixels, not exactly to check if we can memcpy... so it is unclear
to me if some combinations may still need these checks to make the right
decision. I did not see any regressions with llvmpipe at least, so I am
assuming that they are not needed, but maybe someone wants to give this
patch a test run on nouveau or radeon, just in case. If they are needed
I guess the right thing would be to move the checks to st_cb_readpixels.c.

 src/mesa/main/readpix.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index a3357cd..e256695 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -128,7 +128,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context *ctx, GLenum format,
    struct gl_renderbuffer *rb =
          _mesa_get_read_renderbuffer_for_format(ctx, format);
-   GLenum srcType;
@@ -153,21 +152,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context *ctx, GLenum format,
          return GL_TRUE;
-      /* Conversion between signed and unsigned integers needs masking
-       * (it isn't just memcpy). */
-      srcType = _mesa_get_format_datatype(rb->Format);
-      if ((srcType == GL_INT &&
-           (type == GL_UNSIGNED_INT ||
-            type == GL_UNSIGNED_SHORT ||
-            type == GL_UNSIGNED_BYTE)) ||
-          (srcType == GL_UNSIGNED_INT &&
-           (type == GL_INT ||
-            type == GL_SHORT ||
-            type == GL_BYTE))) {
-         return GL_TRUE;
-      }
       /* And finally, see if there are any transfer ops. */
       return get_readpixels_transfer_ops(ctx, rb->Format, format, type,
                                          uses_blit) != 0;

More information about the mesa-dev mailing list