[Mesa-dev] [PATCH 1/7] mesa: Hide weirdness of 1D_ARRAY textures from Driver.CopyTexSubImage().

[Eric Anholt]

Brian Paul <brianp at vmware.com> writes:

> On 06/05/2013 10:14 AM, Eric Anholt wrote:
>> Intel had brokenness here, and I'd like to continue moving Mesa toward
>> hiding 1D_ARRAY's ridiculousness inside of the core, like we did with
>> MapTextureImage.  Fixes copyteximage 1D_ARRAY on intel.
>>
>> There's still an impedance mismatch in meta when falling back to read and
>> texsubimage, since texsubimage expects coordinates into 1D_ARRAY as
>> (width, slice, 0) instead of (width, 0, slice).

>>
>> +static void
>> +copytexsubimage_by_slice(struct gl_context *ctx,
>> +                         struct gl_texture_image *texImage,
>> +                         GLuint dims,
>> +                         GLint xoffset, GLint yoffset, GLint zoffset,
>> +                         struct gl_renderbuffer *rb,
>> +                         GLint x, GLint y,
>> +                         GLsizei width, GLsizei height)
>> +{
>> +   if (texImage->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
>> +      int slice;
>> +
>> +      /* For 1D arrays, we copy each scanline of the source rectangle into the
>> +       * next array slice.
>> +       */
>> +      assert(zoffset == 0);
>> +
>> +      for (slice = 0; slice < height; slice++) {
>> +         if (yoffset + slice >= texImage->Height)
>> +            break;
>> +
>> +         ctx->Driver.CopyTexSubImage(ctx, 2, texImage,
>> +                                     xoffset, 0, yoffset + slice,
>> +                                     rb, x, y, width, 1);
>
> Should that be 'y + slice'?  Otherwise I think we're always copying from 
> the same Y position.

Good catch!  Fixed.

>> -   /* 1D array textures need special treatment.
>> -    * Blit rows from the source to layers in the destination. */
>> -   if (texImage->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
>> -      int y, layer;
>> -
>> -      for (y = srcY0, layer = 0; layer < height; y += yStep, layer++) {
>> -         blit.src.box.y = y;
>> -         blit.src.box.height = 1;
>> -         blit.dst.box.y = 0;
>> -         blit.dst.box.height = 1;
>> -         blit.dst.box.z = destY + layer;
>> -
>> -         pipe->blit(pipe, &blit);
>> -      }
>> -   }
>> -   else {
>> -      /* All the other texture targets. */
>> -      pipe->blit(pipe, &blit);
>> -   }
>> +   pipe->blit(pipe, &blit);
>>      return;
>>
>>   fallback:
>>      /* software fallback */
>>      fallback_copy_texsubimage(ctx,
>>                                strb, stImage, texImage->_BaseFormat,
>> -                             destX, destY, destZ,
>> +                             destX, destY, slice,
>>                                srcX, srcY, width, height);
>>   }
>
> Thanks for updating the state tracker code.  You removed the code above 
> on the premise that height will always be 1 if we're copying to a 1D 
> array texture, right?  Maybe we should assert that just to be safe.

I think instead of putting an assert like that in the places we think of
it at the middle levels, we should do so in our drivers at the bottom
level of mapping or blitting a particular slice, so that we catch
mistakes with slices or clipping wherever they may happen.

Turns out I don't have some of the asserts I thought I did in the Intel
driver, though.  I should go fix that.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130607/3a80a8c7/attachment-0001.pgp>