Re: [PATCH 2/3] wined3d: Handle partial updates to 3D textures in wined3d_texture_gl_upload_bo().

[Henri Verbeet]

On Wed, 10 Mar 2021 at 15:11, Jan Sikorski  wrote:
> static void wined3d_texture_gl_upload_bo(const struct wined3d_format *src_format, GLenum target,
> -
>    unsigned int level, unsigned int src_row_pitch, unsigned int dst_x, unsigned int dst_y,
> -
>    unsigned int level, unsigned int src_row_pitch, unsigned int src_slice_pitch, unsigned int dst_x, unsigned int dst_y,
>    unsigned int dst_z, unsigned int update_w, unsigned int update_h, unsigned int update_d,
>    const BYTE *addr, BOOL srgb, struct wined3d_texture *dst_texture,
>    const struct wined3d_gl_info *gl_info)

That line is a bit on the long side. (For reference, there's a soft
limit at 100 columns, and a hard limit at 120 columns.)
> @@ -2213,7 +2213,9 @@ static void wined3d_texture_gl_upload_bo(const struct wined3d_format *src_format
>      }
>      else
>      {
> -
>    unsigned int y, y_count;
> -
>    unsigned int slice_to_row_pitch = src_slice_pitch / src_row_pitch;

"src_row_pitch" may be 0.
> -
>    if (src_slice_pitch && slice_to_row_pitch * src_row_pitch == src_slice_pitch)
>    {
> -
>        if (target == GL_TEXTURE_2D_ARRAY || target == GL_TEXTURE_3D)
> -
>        {
> -
>            GL_EXTCALL(glTexSubImage3D(target, level, dst_x, dst_y + y, dst_z,
> -
>                    update_w, update_h, update_d, format_gl->format, format_gl->type, addr));
> -
>        }
> -
>        else if (target == GL_TEXTURE_1D)
> -
>        {
> -
>            gl_info->gl_ops.gl.p_glTexSubImage1D(target, level, dst_x,
> -
>                    update_w, format_gl->format, format_gl->type, addr);
> -
>        }
> -
>        else
> -
>        gl_info->gl_ops.gl.p_glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, slice_to_row_pitch);
> -
>        z_count = 1;
> -
>    }

So essentially,

if (src_slice_pitch && src_row_pitch && !(src_slice_pitch % src_row_pitch))
    {
        gl_info->gl_ops.gl.p_glPixelStorei(GL_UNPACK_IMAGE_HEIGHT,
src_slice_pitch / src_row_pitch);
        ...
    }

right? The row pitch probably needs a similar alignment check,
although unaligned pitches are perhaps rare enough that we've never
run into them.
> -
>        for (y = 0; y < y_count; ++y)
>        {
> -
>            gl_info->gl_ops.gl.p_glTexSubImage2D(target, level, dst_x, dst_y + y,
> -
>                    update_w, update_h, format_gl->format, format_gl->type, addr);
> -
>            upload_addr = addr;
> -
>            if (target == GL_TEXTURE_2D_ARRAY || target == GL_TEXTURE_3D)
> -
>            {
> -
>                GL_EXTCALL(glTexSubImage3D(target, level, dst_x, dst_y + y, dst_z + z,
> -
>                        update_w, update_h, update_d, format_gl->format, format_gl->type, upload_addr));
> -
>            }
> -
>            else if (target == GL_TEXTURE_1D)
> -
>            {
> -
>                gl_info->gl_ops.gl.p_glTexSubImage1D(target, level, dst_x,
> -
>                        update_w, format_gl->format, format_gl->type, upload_addr);
> -
>            }
> -
>            else
> -
>            {
> -
>                gl_info->gl_ops.gl.p_glTexSubImage2D(target, level, dst_x, dst_y + y,
> -
>                        update_w, update_h, format_gl->format, format_gl->type, upload_addr);
> -
>            }
> -
>            upload_addr += src_row_pitch;
>        }

Incrementing "upload_addr" does nothing here, it's reset again at the
start of the loop. It would probably be best to simply use "&addr[z *
slice_pitch + y * row_pitch]".

You'd also need to handle slice pitches for compressed formats.