Parameter consistency for textures/buffers

[flibitijibibo]

From the API PR:

Functions that operate on buffer take a buffer and a struct for parameters. Functions that operate on texture only take a struct that includes both texture and parameters. I think it would be better to have functions with similar signature.

[thatcosmonaut]

Breaking this down a bit...

TextureSlice is a struct which contains a texture pointer, a layer, and mip level. This is used by functions which need to bind a specific resource view, like BeginRenderPass and BindFragmentStorageTextures.

TextureRegion is a struct which encompasses a TextureSlice and 3D offset/dimensions. This is used by functions which write data to a specific region of a texture, like UploadToTexture and CopyTextureToTexture. You cannot have a TextureRegion without also needing a TextureSlice, so it makes sense for TextureRegion to contain TextureSlice.

Let's compare the signatures of UploadToTexture and UploadToBuffer.

extern SDL_DECLSPEC void SDLCALL SDL_GpuUploadToTexture(
    SDL_GpuCopyPass *copyPass,
    SDL_GpuTransferBuffer *transferBuffer,
    SDL_GpuTextureRegion *textureRegion,
    SDL_GpuBufferImageCopy *copyParams,
    SDL_bool cycle);

extern SDL_DECLSPEC void SDLCALL SDL_GpuUploadToBuffer(
    SDL_GpuCopyPass *copyPass,
    SDL_GpuTransferBuffer *transferBuffer,
    SDL_GpuBuffer *buffer,
    SDL_GpuBufferCopy *copyParams,
    SDL_bool cycle);

These have a similar shape. You have a copy pass, the transfer buffer, the resource being uploaded to, some parameters particular to the copy operation, and a cycle bool. The main difference is that TextureRegion contains data about where the data is being copied to, whereas offset data for buffers is contained in the BufferCopy struct.

If we were to change the definition of TextureRegion, we would have to add an extra TextureSlice parameter to every struct and function that uses it. I don't think that is really an improvement.

We could create structs like TransferBufferRegion and BufferRegion that contain a pointer, an offset, and a size. In that case the BufferCopy param would be unnecessary, but then the client would have to duplicate the size fields, and UploadToTexture and DownloadToTexture would still require inputs about stride and pitch. In a way the size duplication would be similar to how CopyTextureToTexture requires that the TextureRegion dimensions match, but I am again not sure if this is a clear improvement.

I don't really see a strong reason for changing the parameter structure. It's a reasonable design as-is and I think this is mostly a matter of personal preference.

[flibitijibibo]

It does seem like it would just add layers of indirection without actually changing how it's called or implemented - @meyraud705, thoughts?

[meyraud705]

UploadToTexture and UploadToBuffer have similar signature but different call: order of copy parameters for UploadToTexture look reversed if you use compound literal.

SDL_GpuUploadToTexture(copypass,
    transfer_buffer,  // source
    &(SDL_GpuTextureRegion){{texture, mipmap, 0}, // destination
                            0, 0, 0, w, h, 1}, // destination parameters
    &(SDL_GpuBufferImageCopy){0, pitch, s}, // source parameters
    SDL_FALSE);
SDL_GpuUploadToBuffer(copypass,
    transfer_buffer, // source
    buffer, // destination
    &(SDL_GpuBufferCopy){src_offset, // source parameter
                         dst_offset, // destination parameter
                         size}, // global parameter
    SDL_FALSE);

And SDL_GpuDownloadFromTexture() is also different: src, src params, dst, dst params.

By moving TextureSlice out of TextureRegion all functions have same parameter order when calling them: src, dst, src_params, dst_params. I think it doesn't really add a parameter, it flatten them.

SDL_GpuUploadToTexture(copypass,
    transfer_buffer,  // source
    &(SDL_GpuTextureSlice){texture, mipmap, 0}, // destination
    &(SDL_GpuBufferImageCopy){0, pitch, s}, // source parameters
    &(SDL_GpuTextureRegion){0, 0, 0, w, h, 1}, // destination parameters
    SDL_FALSE);

[flibitijibibo]

After trying a few different things there is now #51

[flibitijibibo]

This is now in the latest gpu revision, the result is soooo much nicer to read/write so extra thanks to @meyraud705 for the feedback!