[Mesa-dev] [PATCH] mesa: only copy the requested teximage faces

[Ilia Mirkin]

On Thu, Aug 27, 2015 at 5:15 PM, Brian Paul <brianp at vmware.com> wrote:
> On 08/27/2015 03:05 PM, Ilia Mirkin wrote:
>>
>> On Thu, Aug 27, 2015 at 4:43 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>
>>> On Thu, Aug 27, 2015 at 4:32 PM, Brian Paul <brianp at vmware.com> wrote:
>>>>
>>>> On 08/27/2015 01:43 PM, Ilia Mirkin wrote:
>>>>>
>>>>>
>>>>> Cube maps are special in that they have separate teximages for each
>>>>> face. We handled that by copying the data to them separately, but in
>>>>> case zoffset != 0 or depth != 6 we would read off the end of the client
>>>>> array or modify the wrong images.
>>>>>
>>>>> zoffset/depth have already been verified by the time the code gets to
>>>>> this stage, so no need to double-check.
>>>>>
>>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>>> ---
>>>>>    src/mesa/main/teximage.c | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>>>> index 253e881..d7b4304 100644
>>>>> --- a/src/mesa/main/teximage.c
>>>>> +++ b/src/mesa/main/teximage.c
>>>>> @@ -3711,12 +3711,12 @@ texturesubimage(struct gl_context *ctx, GLuint
>>>>> dims,
>>>>>          rowStride = _mesa_image_image_stride(&ctx->Unpack, width,
>>>>> height,
>>>>>                                               format, type);
>>>>>          /* Copy in each face. */
>>>>> -      for (i = 0; i < 6; ++i) {
>>>>> +      for (i = zoffset; i < zoffset + depth; ++i) {
>>>>>             texImage = texObj->Image[i][level];
>>>>>             assert(texImage);
>>>>>
>>>>>             _mesa_texture_sub_image(ctx, 3, texObj, texImage,
>>>>> texObj->Target,
>>>>> -                                 level, xoffset, yoffset, zoffset,
>>>>> +                                 level, xoffset, yoffset, 0,
>>
>>
>> Actually I wonder if this needs to be 'i' instead of 0 or zoffset?
>> There are no operational tests unfortunately, and I don't know enough
>> about teximages.
>
>
> No, 0 is correct because each cube face is in a different gl_texture_image.
> For conventional (not array) cube maps, the six faces are stored in separate
> texObj->Image[face][level] entries.  The texImage = texObj->Image[i][level]
> assignment above chooses the dest texture image.
>
> I wish we could get rid of that and use one gl_texture_image to represent a
> cube map as six slices, but there's nothing preventing the user from
> creating a cube map with six faces of different sizes with glTexImage2D().
> Such a cube map is invalid for rendering, but you can built it that way.
>
> That can't happen with cube map arrays created with glTexImage3D().

What if you have a cubemap texture view of a 2d array or cubemap array
texture? Does this logic still hold?