Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPU transfer buffer contains data from another buffer after defrag #12242

Open
meyraud705 opened this issue Feb 10, 2025 · 1 comment · May be fixed by #12256
Open

GPU transfer buffer contains data from another buffer after defrag #12242

meyraud705 opened this issue Feb 10, 2025 · 1 comment · May be fixed by #12256
Assignees
Milestone

Comments

@meyraud705
Copy link
Contributor

On the Vulkan back end, upload transfer buffer are corrupted after a defrag.

I made this example based on copyAndReadBack.c example:

  • create an upload transfer buffer and initialize it once with value from 0 to 127
  • create and release temporary upload buffer each frame to trigger a defrag, initialize them with value from 128 to 255
  • copy the upload buffer to a GPU buffer and read it back in a download buffer each frame

After a defrag, which usually happens on the 3rd frame, data read back matches value from temporary buffer instead of original value. Looking at this with RenderDoc I see that the original upload buffer does not contain the correct data.

clear_transfer_buffer.c
#include <SDL3/SDL.h>

// gcc -g -Og -fPIE -fno-strict-aliasing -Wall -Wextra -lSDL3 ./clear_transfer_buffer.c -o test_clear_transfer_buffer

int main(void)
{
    int frames = 0;
    if (!SDL_Init(SDL_INIT_VIDEO | SDL_INIT_EVENTS)) {
        SDL_Log("SDL initialisation failed: %s", SDL_GetError());
        return -1;
    }
    
    SDL_Window* window = SDL_CreateWindow("test", 128, 128, 0);
    if (!window)
    {
        SDL_Log("could not create window");
        return -1;
    }
    
    SDL_GPUDevice* device = SDL_CreateGPUDevice(SDL_GPU_SHADERFORMAT_SPIRV, true, NULL);
    if (!device)
    {
        SDL_Log("GPUCreateDevice failed");
        return -1;
    }
    
    if (!SDL_ClaimWindowForGPUDevice(device, window))
    {
        SDL_Log("could not claim window");
        return -1;
    }
    
    SDL_PropertiesID pid = SDL_CreateProperties();
    
    Uint32 bufferData[128]; // data for the original upload buffer
    for (int i = 0; i < 128; ++i) {
        bufferData[i] = i;
    }

    SDL_SetStringProperty(pid, SDL_PROP_GPU_BUFFER_CREATE_NAME_STRING, "gpu_buffer");
    SDL_GPUBuffer* OriginalBuffer = SDL_CreateGPUBuffer(
        device,
        &(SDL_GPUBufferCreateInfo) {
            .usage = SDL_GPU_BUFFERUSAGE_COMPUTE_STORAGE_READ | SDL_GPU_BUFFERUSAGE_COMPUTE_STORAGE_WRITE,
            .size = sizeof(bufferData),
            .props = pid
        }
    );

    SDL_SetStringProperty(pid, SDL_PROP_GPU_TRANSFERBUFFER_CREATE_NAME_STRING, "download_buffer");
    SDL_GPUTransferBuffer* downloadTransferBuffer = SDL_CreateGPUTransferBuffer(
        device,
        &(SDL_GPUTransferBufferCreateInfo) {
            .usage = SDL_GPU_TRANSFERBUFFERUSAGE_DOWNLOAD,
            .size = sizeof(bufferData),
            .props = pid
        }
    );
    
    // Create upload buffer and initialize it
    SDL_SetStringProperty(pid, SDL_PROP_GPU_TRANSFERBUFFER_CREATE_NAME_STRING, "***upload_buffer***");
    SDL_GPUTransferBuffer* uploadTransferBuffer = SDL_CreateGPUTransferBuffer(
        device,
        &(SDL_GPUTransferBufferCreateInfo) {
            .usage = SDL_GPU_TRANSFERBUFFERUSAGE_UPLOAD,
            .size = sizeof(bufferData),
            .props = pid
        }
    );
    SDL_DestroyProperties(pid);

    Uint8* uploadTransferPtr = SDL_MapGPUTransferBuffer(
        device,
        uploadTransferBuffer,
        false
    );
    SDL_memcpy(uploadTransferPtr, bufferData, sizeof(bufferData));
    SDL_UnmapGPUTransferBuffer(device, uploadTransferBuffer);

    // Create and release buffer randomly to trigger a defrag
    SDL_GPUTransferBuffer* tmpBuffers[32] = {NULL};
    Uint32 tmpBufferData[128]; // data for the temporary upload buffers
    for (int i = 0; i < 128; ++i) {
        tmpBufferData[i] = i + 128;
    }
    for (int i = 0; i < 32; ++i) {
        tmpBuffers[i] = SDL_CreateGPUTransferBuffer(
            device,
            &(SDL_GPUTransferBufferCreateInfo) {
                .usage = SDL_GPU_TRANSFERBUFFERUSAGE_UPLOAD,
                .size = 128
            }
        );
        uploadTransferPtr = SDL_MapGPUTransferBuffer(
            device,
            tmpBuffers[i],
            false
        );
        SDL_memcpy(uploadTransferPtr, tmpBufferData, sizeof(tmpBufferData));
        SDL_UnmapGPUTransferBuffer(device, tmpBuffers[i]);
    }
    
    bool quit = false;
    Uint32 i_frame = 0;
    while (!quit) {
        SDL_Event evt;
        while (SDL_PollEvent(&evt)) {
            if (evt.type == SDL_EVENT_QUIT) {
                quit = true;
            } else if (evt.type == SDL_EVENT_KEY_DOWN && evt.key.scancode == SDL_SCANCODE_ESCAPE) {
                quit = true;
            }
        }
        ++i_frame;
        
        SDL_GPUCommandBuffer* cmdbuf = SDL_AcquireGPUCommandBuffer(device);
        
        SDL_GPUCopyPass* copyPass = SDL_BeginGPUCopyPass(cmdbuf);
        
        // clear GPU buffer
        SDL_UploadToGPUBuffer(
            copyPass,
            &(SDL_GPUTransferBufferLocation) {
                .transfer_buffer = uploadTransferBuffer,
                .offset = 0,
            },
            &(SDL_GPUBufferRegion) {
                .buffer = OriginalBuffer,
                .offset = 0,
                .size = sizeof(bufferData)
            },
            false
        );
        
        // Download the original bytes from the copy
        SDL_DownloadFromGPUBuffer(
            copyPass,
            &(SDL_GPUBufferRegion) {
                .buffer = OriginalBuffer,
                .offset = 0,
                .size = sizeof(bufferData)
            },
            &(SDL_GPUTransferBufferLocation) {
                .transfer_buffer = downloadTransferBuffer,
                .offset = 0
            }
        );

        SDL_EndGPUCopyPass(copyPass);
        
        // Create and release buffer randomly to trigger a defrag
        int r = 32; SDL_rand(32*2);
        // if (r <= 32) {
            // SDL_Log("%d: creating %d buffers", i_frame, r);
            for (int i = 0; i < r; ++i) {
                SDL_ReleaseGPUTransferBuffer(device, tmpBuffers[i]);
                tmpBuffers[i] = SDL_CreateGPUTransferBuffer(
                    device,
                    &(SDL_GPUTransferBufferCreateInfo) {
                        .usage = SDL_GPU_TRANSFERBUFFERUSAGE_UPLOAD,
                        .size = 1024*1024,//(1024*i_frame*r)%(33554432)
                    }
                );
                uploadTransferPtr = SDL_MapGPUTransferBuffer(
                    device,
                    tmpBuffers[i],
                    false
                );
                SDL_memcpy(uploadTransferPtr, tmpBufferData, sizeof(tmpBufferData));
                SDL_UnmapGPUTransferBuffer(device, tmpBuffers[i]);
            }
        // }
        
        SDL_GPUFence* fence = SDL_SubmitGPUCommandBufferAndAcquireFence(cmdbuf);
        
        SDL_WaitForGPUFences(device, true, &fence, 1);
        SDL_ReleaseGPUFence(device, fence);

        // Compare the original bytes to the copied bytes
        Uint8 *downloadedData = SDL_MapGPUTransferBuffer(
            device,
            downloadTransferBuffer,
            false
        );

        if (SDL_memcmp(downloadedData, bufferData, sizeof(bufferData)) == 0)
        {
            SDL_Log("SUCCESS! Original buffer bytes and the downloaded bytes match!");
        }
        else
        {
            SDL_Log("%d: FAILURE! Original buffer bytes do not match downloaded bytes!", i_frame);
            if (SDL_memcmp(downloadedData, tmpBufferData, sizeof(tmpBufferData)) == 0) {
                SDL_Log("contain data of another buffer!");
            }
        }

        SDL_UnmapGPUTransferBuffer(device, downloadTransferBuffer);
        
        // present to trigger a defrag
        cmdbuf = SDL_AcquireGPUCommandBuffer(device);
        if (cmdbuf == NULL) {
            SDL_Log("SDL_AcquireGPUCommandBuffer failed: %s", SDL_GetError());
            return -1;
        }

        SDL_GPUTexture *swapchainTexture;
        if (!SDL_WaitAndAcquireGPUSwapchainTexture(cmdbuf, window, &swapchainTexture, NULL, NULL)) {
            SDL_Log("SDL_AcquireGPUSwapchainTexture failed: %s", SDL_GetError());
            return -1;
        }

        if (swapchainTexture != NULL) {
            const double currentTime = (double)SDL_GetPerformanceCounter() / SDL_GetPerformanceFrequency();
            SDL_GPURenderPass *renderPass;
            SDL_GPUColorTargetInfo color_target_info;
            SDL_zero(color_target_info);
            color_target_info.texture = swapchainTexture;
            color_target_info.clear_color.r = (float)(0.5 + 0.5 * SDL_sin(currentTime));
            color_target_info.clear_color.g = (float)(0.5 + 0.5 * SDL_sin(currentTime + SDL_PI_D * 2 / 3));
            color_target_info.clear_color.b = (float)(0.5 + 0.5 * SDL_sin(currentTime + SDL_PI_D * 4 / 3));
            color_target_info.clear_color.a = 1.0f;
            color_target_info.load_op = SDL_GPU_LOADOP_CLEAR;
            color_target_info.store_op = SDL_GPU_STOREOP_STORE;

            renderPass = SDL_BeginGPURenderPass(cmdbuf, &color_target_info, 1, NULL);
            SDL_EndGPURenderPass(renderPass);

            SDL_SubmitGPUCommandBuffer(cmdbuf);
        } else {
            /* Swapchain is unavailable, cancel work */
            SDL_CancelGPUCommandBuffer(cmdbuf);
        }

        frames++;
    }

    // Cleanup
    SDL_ReleaseGPUTransferBuffer(device, downloadTransferBuffer);
    SDL_ReleaseGPUTransferBuffer(device, uploadTransferBuffer);
    SDL_ReleaseGPUBuffer(device, OriginalBuffer);
    for (int i = 0; i < 32; ++i) {
        SDL_ReleaseGPUTransferBuffer(device, tmpBuffers[i]);
    }
    SDL_DestroyGPUDevice(device);
    SDL_DestroyWindow(window);
    SDL_Quit();
    return 0;
}
@thatcosmonaut
Copy link
Collaborator

thatcosmonaut commented Feb 10, 2025

Dug into this and the issue is that we don't preserve upload transfer contents on defrag.

The two possible solutions are preserving upload contents on copy, or turning all transfer buffers into dedicated allocs.

@thatcosmonaut thatcosmonaut self-assigned this Feb 10, 2025
@thatcosmonaut thatcosmonaut added this to the 3.2.6 milestone Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants