[Mesa-dev] [PATCH 09/29] mesa: Add _mesa_swap2_copy and _mesa_swap4_copy

[Jason Ekstrand]

On Thu, Nov 20, 2014 at 12:48 AM, Iago Toral <itoral at igalia.com> wrote:

> On Wed, 2014-11-19 at 14:15 -0600, Patrick Baggett wrote:
> >
> >
> > On Tue, Nov 18, 2014 at 3:23 AM, Iago Toral Quiroga
> > <itoral at igalia.com> wrote:
> >         We have _mesa_swap{2,4} but these do in-place byte-swapping
> >         only. The new
> >         functions receive an extra parameter so we can swap bytes on a
> >         source
> >         input array and store the results in a (possibly different)
> >         destination
> >         array.
> >
> >
> >
> > If this is being split into an "in-place" and "different pointers"
> > version, I think using the "restrict" keyword would be useful here to
> > improve the performance. Then, the in-place one cannot be implemented
> > as copy(p,p,n), but the code isn't overly complicated.
>
> I did not know about 'restrict', thanks for the tip!.
>
> It kind of defeats the original idea of not duplicating the code but it
> is true that it is not particularly complex anyway, so maybe it is worth
> it if Jason agrees with having two versions of the functions instead of
> just one in the end. Jason, what do you think?
>

The restrict keyword is a C99 thing and I don't think it's supported in
MSVC so that would be a problem.  If it won't build with MSVC then it's a
non-starter.  If MSVC can handle "restrict", then I don't know that I care
much either way about 2 functions or 4


>
> Iago
>
> >
> >
> >         This is useful to implement byte-swapping in pixel uploads,
> >         since in this
> >         case we need to swap bytes on the src data which is owned by
> >         the
> >         application so we can't do an in-place byte swap.
> >         ---
> >          src/mesa/main/image.c | 25 +++++++++++++++++--------
> >          src/mesa/main/image.h | 10 ++++++++--
> >          2 files changed, 25 insertions(+), 10 deletions(-)
> >
> >         diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
> >         index 4ea5f04..9ad97c5 100644
> >         --- a/src/mesa/main/image.c
> >         +++ b/src/mesa/main/image.c
> >         @@ -41,36 +41,45 @@
> >
> >
> >          /**
> >         - * Flip the order of the 2 bytes in each word in the given
> >         array.
> >         + * Flip the order of the 2 bytes in each word in the given
> >         array (src) and
> >         + * store the result in another array (dst). For in-place
> >         byte-swapping this
> >         + * function can be called with the same array for src and
> >         dst.
> >           *
> >         - * \param p array.
> >         + * \param dst the array where byte-swapped data will be
> >         stored.
> >         + * \param src the array with the source data we want to
> >         byte-swap.
> >           * \param n number of words.
> >           */
> >          void
> >         -_mesa_swap2( GLushort *p, GLuint n )
> >         +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
> >          {
> >             GLuint i;
> >             for (i = 0; i < n; i++) {
> >         -      p[i] = (p[i] >> 8) | ((p[i] << 8) & 0xff00);
> >         +      dst[i] = (src[i] >> 8) | ((src[i] << 8) & 0xff00);
> >             }
> >          }
> >
> >
> >
> >          /*
> >         - * Flip the order of the 4 bytes in each word in the given
> >         array.
> >         + * Flip the order of the 4 bytes in each word in the given
> >         array (src) and
> >         + * store the result in another array (dst). For in-place
> >         byte-swapping this
> >         + * function can be called with the same array for src and
> >         dst.
> >         + *
> >         + * \param dst the array where byte-swapped data will be
> >         stored.
> >         + * \param src the array with the source data we want to
> >         byte-swap.
> >         + * \param n number of words.
> >           */
> >          void
> >         -_mesa_swap4( GLuint *p, GLuint n )
> >         +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n )
> >          {
> >             GLuint i, a, b;
> >             for (i = 0; i < n; i++) {
> >         -      b = p[i];
> >         +      b = src[i];
> >                a =  (b >> 24)
> >                 | ((b >> 8) & 0xff00)
> >                 | ((b << 8) & 0xff0000)
> >                 | ((b << 24) & 0xff000000);
> >         -      p[i] = a;
> >         +      dst[i] = a;
> >             }
> >          }
> >
> >         diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h
> >         index abd84bf..79c6e68 100644
> >         --- a/src/mesa/main/image.h
> >         +++ b/src/mesa/main/image.h
> >         @@ -33,10 +33,16 @@ struct gl_context;
> >          struct gl_pixelstore_attrib;
> >
> >          extern void
> >         -_mesa_swap2( GLushort *p, GLuint n );
> >         +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n );
> >
> >          extern void
> >         -_mesa_swap4( GLuint *p, GLuint n );
> >         +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n );
> >         +
> >         +static inline void
> >         +_mesa_swap2( GLushort *p, GLuint n ) { _mesa_swap2_copy(p, p,
> >         n); }
> >         +
> >         +static inline void
> >         +_mesa_swap4( GLuint *p, GLuint n ) { _mesa_swap4_copy(p, p,
> >         n); }
> >
> >          extern GLintptr
> >          _mesa_image_offset( GLuint dimensions,
> >         --
> >         1.9.1
> >
> >         _______________________________________________
> >         mesa-dev mailing list
> >         mesa-dev at lists.freedesktop.org
> >         http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141120/a90882be/attachment.html>