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

[Brian Paul]

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().

-Brian