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

[Brian Paul]

On 08/27/2015 03:15 PM, Ilia Mirkin wrote:
> 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?

I'm not sure what'll happen if one creates a cubemap with mismatched 
faces and creates another view into it.  I doubt anyone's tested that.

-Brian