[Mesa-dev] [PATCH 2/4] mesa: implement EXT_packed_float

[Marek Olšák]

On Thu, Apr 28, 2011 at 6:36 PM, Eric Anholt <eric at anholt.net> wrote:
> On Thu, 28 Apr 2011 14:04:23 +0200, Marek Olšák <maraeo at gmail.com> wrote:
>> +static GLboolean
>> +_mesa_texstore_r11_g11_b10f(TEXSTORE_PARAMS)
>> +{
>> +   const GLenum baseFormat = _mesa_get_format_base_format(dstFormat);
>> +
>> +   ASSERT(dstFormat == MESA_FORMAT_R11_G11_B10_FLOAT);
>> +   ASSERT(baseInternalFormat == GL_RGB);
>> +
>> +   if (!ctx->_ImageTransferState &&
>> +       !srcPacking->SwapBytes &&
>> +       srcFormat == GL_RGB &&
>> +       srcType == GL_UNSIGNED_INT_10F_11F_11F_REV) {
>> +      /* simple memcpy path */
>> +      memcpy_texture(ctx, dims,
>> +                     dstFormat, dstAddr, dstXoffset, dstYoffset, dstZoffset,
>> +                     dstRowStride,
>> +                     dstImageOffsets,
>> +                     srcWidth, srcHeight, srcDepth, srcFormat, srcType,
>> +                     srcAddr, srcPacking);
>> +   }
>> +   else {
>> +      /* general path */
>> +      const GLfloat *tempImage = _mesa_make_temp_float_image(ctx, dims,
>> +                                                 baseInternalFormat,
>> +                                                 baseFormat,
>> +                                                 srcWidth, srcHeight, srcDepth,
>> +                                                 srcFormat, srcType, srcAddr,
>> +                                                 srcPacking,
>> +                                                 ctx->_ImageTransferState);
>> +      const GLfloat *srcRow = tempImage;
>> +      GLint bytesPerRow;
>> +      GLint img, row, col;
>> +      if (!tempImage)
>> +         return GL_FALSE;
>> +      bytesPerRow = srcWidth * 3 * sizeof(GLfloat);
>
> Unused variable bytesPerRow?

Good catch. I'll remove that variable in a follow-up commit when I
push all of this.

>
> Other than that,
>
> Reviewed-by: Eric Anholt <eric at anholt.net>
>
> Seeing this same shape of function over and over, I wish we had
> something that would try to come up with a MESA_FORMAT for a src
> type/format, do the memcpy if it matches dst, and falls back to a
> default make_temp_float_image otherwise.
>

I absolutely agree. I am just following the way it's done there.

And we can go even further. Now that we have a wide range of texture
formats supported by hardware drivers, we can just query whether a
driver can blit natively from (format,type) to internalformat and let
it do it without having to resort to _mesa_make_temp_float_image. But
doing cleanups and improvements like that is far less important to me
than, say, bumping the GL version to 3.0.

Marek