[Mesa-dev] [PATCH 09/10] mesa: Add support to unpack depth-stencil texture in to FLOAT_32_UNSIGNED_INT_24_8_REV

Roland Scheidegger sroland at vmware.com
Mon Mar 24 10:38:12 PDT 2014


Am 22.03.2014 18:15, schrieb Brian Paul:
> On 03/21/2014 04:01 PM, Anuj Phogat wrote:
>> Cc: <mesa-stable at lists.freedesktop.org>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>>   src/mesa/main/format_unpack.c | 79
>> ++++++++++++++++++++++++++++++++++++++++++-
>>   src/mesa/main/format_unpack.h |  5 +++
>>   2 files changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/format_unpack.c
>> b/src/mesa/main/format_unpack.c
>> index 7abbe46..1725f10 100644
>> --- a/src/mesa/main/format_unpack.c
>> +++ b/src/mesa/main/format_unpack.c
>> @@ -4264,6 +4264,79 @@
>> _mesa_unpack_uint_24_8_depth_stencil_row(mesa_format format, GLuint n,
>>      }
>>   }
>>
>> +static void
>> +unpack_float_32_uint_24_8_depth_stencil_S8_Z24(const GLuint *src,
>> +                                               GLuint *dst, GLuint n)
>> +{
>> +   GLuint i;
>> +
>> +   for (i = 0; i < n; i++) {
>> +      GLuint val = src[i];
>> +      GLuint z24 = val & 0xffffff;
>> +      GLfloat zf = z24 * (1.0F / 0xffffff);
> 
> In other function where we convert 24-bit Z to a float we need to use
> double precision.  See fc52534f012837a39c03a764eb611d460210514a
As a side note, the commit there is telling lies ("GLfloat doesn't have
enough precision to exactly represent 0xffffff and 0xffffffff. (and a
reciprocal of those, if I am not mistaken)"
0xffffff is perfectly representable using floats (as is any other 24bit
unorm number). Therefore, I can't see why the _pack_ functions need
doubles (I have some faint memory though there was some discussion about
it...). The reciprocal, however, indeed is not (I doubt it's exactly
representable with doubles neither, but with floats indeed that last bit
is nearly exactly 50% off). I guess it would be nice if those functions
would use some inline helper for unorm24->float and vice versa
conversions so at least the same logic is used everywhere. But in any
case since it's all scalar double vs. float probably doesn't really matter.

Roland


> 
> 
>> +      GLuint s = val & 0xff000000;
>> +      ((GLfloat *) dst)[i * 2 + 0] = zf;
>> +      dst[i * 2 + 1] = s;
>> +   }
>> +}
>> +
>> +static void
>> +unpack_float_32_uint_24_8_depth_stencil_Z32_S8X24(const GLuint *src,
>> +                                                  GLuint *dst, GLuint n)
>> +{
>> +   GLuint i;
>> +
>> +   for (i = 0; i < n; i++) {
>> +      /* 8 bytes per pixel (float + uint32) */
>> +      GLfloat zf = ((GLfloat *) src)[i * 2 + 0];
>> +      GLuint s = src[i * 2 + 1] & 0xff;
>> +      ((GLfloat *) dst)[i * 2 + 0] = zf;
>> +      dst[i * 2 + 1] = s;
>> +   }
>> +}
>> +
>> +static void
>> +unpack_float_32_uint_24_8_depth_stencil_Z24_S8(const GLuint *src,
>> +                                               GLuint *dst, GLuint n)
>> +{
>> +   GLuint i;
>> +
>> +   for (i = 0; i < n; i++) {
>> +      GLuint val = src[i];
>> +      GLuint z24 = (val >> 8);
>> +      GLfloat zf = z24 * (1.0F / 0xffffff);
>> +      GLuint s = val & 0xff;
>> +      ((GLfloat *) dst)[i * 2 + 0] = zf;
>> +      dst[i * 2 + 1] = s << 24;
>> +   }
>> +}
>> +
>> +/**
>> + * Unpack depth/stencil returning as GL_FLOAT_32_UNSIGNED_INT_24_8_REV.
>> + * \param format  the source data format
>> + */
>> +void
>> +_mesa_unpack_float_32_uint_24_8_depth_stencil_row(mesa_format format,
>> GLuint n,
>> +                                      const void *src, GLuint *dst)
>> +{
>> +   switch (format) {
>> +   case MESA_FORMAT_S8_UINT_Z24_UNORM:
>> +      unpack_float_32_uint_24_8_depth_stencil_Z24_S8(src, dst, n);
> 
> Let's try to follow the new format naming conventions.  So can we rename
> the function to include the actual src format name?  Something like:
> 
> unpack_float_32_uint_24_8_depth_stencil_S8_UINT_Z24_UNORM().
> 
> or maybe unpack_float_32_uint_24_8_from_S8_UINT_Z24_UNORM().
> 
> I know the name gets a little crazy, but I think including the actual
> format name in the function name is desirable.  I'm about to post a
> series which does this for most other functions in the
> format_pack/unpack.c files.
> 
> 
> 
>> +      break;
>> +   case MESA_FORMAT_Z24_UNORM_S8_UINT:
>> +      unpack_float_32_uint_24_8_depth_stencil_S8_Z24(src, dst, n);
>> +      break;
>> +   case MESA_FORMAT_Z32_FLOAT_S8X24_UINT:
>> +      unpack_float_32_uint_24_8_depth_stencil_Z32_S8X24(src, dst, n);
>> +      break;
>> +   default:
>> +      _mesa_problem(NULL,
>> +                    "bad format %s in
>> _mesa_unpack_uint_24_8_depth_stencil_row",
>> +                    _mesa_get_format_name(format));
>> +      return;
>> +   }
>> +}
>> +
>>   /**
>>    * Unpack depth/stencil
>>    * \param format  the source data format
>> @@ -4274,12 +4347,16 @@ _mesa_unpack_depth_stencil_row(mesa_format
>> format, GLuint n,
>>                              const void *src, GLenum type,
>>                                  GLuint *dst)
>>   {
>> -   assert(type == GL_UNSIGNED_INT_24_8);
>> +   assert(type == GL_UNSIGNED_INT_24_8 ||
>> +          type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV);
>>
>>      switch (type) {
>>      case GL_UNSIGNED_INT_24_8:
>>         _mesa_unpack_uint_24_8_depth_stencil_row(format, n, src, dst);
>>         break;
>> +   case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
>> +      _mesa_unpack_float_32_uint_24_8_depth_stencil_row(format, n,
>> src, dst);
>> +      break;
>>      default:
>>         _mesa_problem(NULL,
>>                       "bad type 0x%x in _mesa_unpack_depth_stencil_row",
>> diff --git a/src/mesa/main/format_unpack.h
>> b/src/mesa/main/format_unpack.h
>> index 5904a28..51f97df 100644
>> --- a/src/mesa/main/format_unpack.h
>> +++ b/src/mesa/main/format_unpack.h
>> @@ -64,6 +64,11 @@
>> _mesa_unpack_uint_24_8_depth_stencil_row(mesa_format format, GLuint n,
>>                        const void *src, GLuint *dst);
>>
>>   void
>> +_mesa_unpack_float_32_uint_24_8_depth_stencil_row(mesa_format format,
>> +                                                  GLuint n,
>> +                                                  const void *src,
>> +                                                  GLuint *dst);
>> +void
>>   _mesa_unpack_depth_stencil_row(mesa_format format, GLuint n,
>>                                 const void *src, GLenum type,
>>                                 GLuint *dst);
>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=XkdvM1Ozy%2FuaX47BDM4wYrNlvm3GVLf%2FWMaakWqmJjo%3D%0A&s=dc5ecb42e197d4a274517f26ebd961f64e2fd92063a23dc1c9ff9ff621e3792a
> 


More information about the mesa-dev mailing list