Can the 2D renderer be made thread safe? #11159

icculus · 2024-10-11T01:09:20Z

So #11150 brings up something that keeps coming up, and that's the requirement that the 2D renderer run on the main thread.

Part of the problem is that this causes problems, but also part of the problem is sometimes it doesn't, depending on the platform, so people keep doing it.

I thought I'd start an issue to talk about this and see if we can find a reasonable way to remove this requirement. It's totally possible we won't be able to do that, to be clear.

But I think it might be worth exploring a few questions:

Is this actually still a requirement, or did we simply get burned in the past, and modern systems won't have that issue?
If some systems have this issue, but not others, which ones?
If some backends have this issue, which ones?
Are the issues isolated to a few key places, like swapping buffers or uploading data?

I'm going to tweak testsprite.c to do its rendering in a background thread and see what blows up and where. It's not the most dramatic use of the API; there are no texture uploads after startup, no ReadPixels, etc, but it's a good start.

I'll report back.

slouken · 2024-10-11T01:23:32Z

I think the most common use case is creating a renderer and doing all rendering/texture operations and presenting in a separate thread . The second case is creating a renderer on the main thread, doing rendering/texture operations off the main thread, and presenting on the main thread. Note that we have SDL_HINT_RENDER_DIRECT3D_THREADSAFE to accommodate the second case for D3D9 and D3D11.

slouken · 2024-10-11T01:24:42Z

Also, by definition, it might appear to work most of the time and blow up sometimes or with certain operations.

thatcosmonaut · 2024-10-11T01:41:43Z

I will say that this is a pain for Render GPU. The intention in a modern explicit API is that the command buffer is only accessed from one thread, and certain operations cannot be interleaved. We'll have to put locks everywhere and even single threaded applications will have to pay for the overhead.

slouken · 2024-10-11T01:46:14Z

I will say that this is a pain for Render GPU. The intention in a modern explicit API is that the command buffer is only accessed from one thread, and certain operations cannot be interleaved. We'll have to put locks everywhere and even single threaded applications will have to pay for the overhead.

Our intent is not to make the GPU renderer completely multi-threaded, it's to understand the natural limitations of threading and renderers on the various platforms. We won't be making the kinds of changes you're anticipating, we just want to see if there's an off-main thread case that makes sense and can be officially supported. And if not, well, we'll note that clearly in the renderer documentation, along with any caveats, and call it a day.

thatcosmonaut · 2024-10-11T02:05:32Z

In that case the two common cases you mentioned should be fine as far as GPU is concerned - the first one will definitely already work, and the second one will probably already work.

andreasgrabher · 2024-10-11T06:02:31Z

My application creates the renderer on the main thread and calls these functions from a secondary thread (SDL2):

SDL_RenderClear();
SDL_RenderCopy();
SDL_RenderPresent();

With SDL2 on macOS this did not cause any problems for years. The renderer used on macOS was Metal (default in SDL2). I also did not get any bug reports from Windows users but it seems that some Linux users do have problems.

With the recent revision of SDL3 on macOS it depends. The default renderer now is GPU. It seems to work when using SDL_LOGICAL_PRESENTATION_DISABLED but causes warnings when using SDL_LOGICAL_PRESENTATION_LETTERBOX. When forcing Metal renderer I see no issues or warnings.

icculus · 2024-10-11T17:34:23Z

The test application. I wrote a simple thing from scratch so it can thread any specific part of the work (SDL_Init, Create window and renderer, upload texture, draw a frame, present a frame).

This uses a semaphore to keep the main thread in sync with the background thread. A side effect of this is that drawing and presenting only happens at the right time during SDL_AppIterate even if in a background thread, but one problem at a time here.

This works on X11+OpenGL with any part threaded, which was the first one I expected to fail. It also works on the GPU backend as-is.

I have to run out for a bit, but more testing later today.

/*
  Copyright (C) 1997-2024 Sam Lantinga <slouken@libsdl.org>

  This software is provided 'as-is', without any express or implied
  warranty.  In no event will the authors be held liable for any damages
  arising from the use of this software.

  Permission is granted to anyone to use this software for any purpose,
  including commercial applications, and to alter it and redistribute it
  freely.
*/

#define SDL_MAIN_USE_CALLBACKS 1
#include <SDL3/SDL.h>
#include <SDL3/SDL_main.h>

#define WINDOW_WIDTH 640
#define WINDOW_HEIGHT 480

static SDL_Window *window = NULL;
static SDL_Renderer *renderer = NULL;
static SDL_Texture *texture = NULL;
static SDL_Semaphore *main_semaphore = NULL;
static SDL_Semaphore *background_semaphore = NULL;
static SDL_Thread *thread = NULL;
static SDL_AtomicInt quit;
static int texture_width, texture_height;
static SDL_ThreadID main_thread_id;

static Uint32 threaded = 0;
#define THREADED_INIT           (1<<0)
#define THREADED_CREATE         (1<<1)
#define THREADED_TEXTURE_UPLOAD (1<<2)
#define THREADED_DRAW           (1<<3)
#define THREADED_PRESENT        (1<<4)


#if 1
#define TRACE(what) SDL_Log("TRACE: [%s] %s", (SDL_GetCurrentThreadID() == main_thread_id) ? "main" : "background", what);
#else
#define TRACE(what)
#endif

static void StepComplete(bool background_thread, bool ran_this_step, Uint32 task)
{
    SDL_Semaphore *semaphore = (threaded & task) ? background_semaphore : main_semaphore;
    if (ran_this_step) {
        TRACE("signal other thread");
        SDL_SignalSemaphore(semaphore);
    } else {
        TRACE("Waiting on other thread");
        SDL_WaitSemaphore(semaphore);
    }
    TRACE("Step is complete");
}

static bool InitSDL(bool background_thread)
{
    const bool run_this_step = (background_thread == ((threaded & THREADED_INIT) != 0));
    if (run_this_step) {
        TRACE("InitSDL");
        if (!SDL_Init(SDL_INIT_VIDEO)) {
            SDL_Log("Couldn't initialize SDL: %s", SDL_GetError());
            return false;
        }
    }

    StepComplete(background_thread, run_this_step, THREADED_INIT);

    return true;
}

static bool CreateRenderer(bool background_thread)
{
    const bool run_this_step = (background_thread == ((threaded & THREADED_CREATE) != 0));
    if (run_this_step) {
        TRACE("CreateRenderer");
        if (!SDL_CreateWindowAndRenderer("testrenderthread", WINDOW_WIDTH, WINDOW_HEIGHT, 0, &window, &renderer)) {
            SDL_Log("Couldn't create window/renderer: %s", SDL_GetError());
            return false;
        }
    }

    StepComplete(background_thread, run_this_step, THREADED_CREATE);

    return true;
}

static bool UploadTexture(bool background_thread)
{
    const bool run_this_step = (background_thread == ((threaded & THREADED_TEXTURE_UPLOAD) != 0));
    if (run_this_step) {
        SDL_Surface *surface = NULL;
        char *bmp_path = NULL;
        TRACE("UploadTexture")
        SDL_asprintf(&bmp_path, "%ssample.bmp", SDL_GetBasePath());  /* allocate a string of the full file path */
        surface = SDL_LoadBMP(bmp_path);
        if (!surface) {
            SDL_Log("Couldn't load bitmap: %s", SDL_GetError());
            SDL_free(bmp_path);
            return false;
        }

        SDL_free(bmp_path);  /* done with this, the file is loaded. */

        texture_width = surface->w;
        texture_height = surface->h;

        texture = SDL_CreateTextureFromSurface(renderer, surface);
        SDL_DestroySurface(surface);
        if (!texture) {
            SDL_Log("Couldn't create static texture: %s", SDL_GetError());
            return false;
        }
    }

    StepComplete(background_thread, run_this_step, THREADED_TEXTURE_UPLOAD);

    return true;
}

static SDL_AppResult DoInit(bool background_thread)
{
    bool okay = true;

    TRACE("DoInit");

    okay = InitSDL(background_thread) && okay;
    okay = CreateRenderer(background_thread) && okay;
    okay = UploadTexture(background_thread) && okay;

    if (!okay) {
        SDL_Log("DoInit failed!");
        return SDL_APP_FAILURE;
    }

    return SDL_APP_CONTINUE;
}

static bool DrawFrame(bool background_thread)
{
    const bool run_this_step = (background_thread == ((threaded & THREADED_DRAW) != 0));
    if (run_this_step) {
        SDL_FRect dst_rect;
        SDL_FPoint center;
        const Uint64 now = SDL_GetTicks();

        /* we'll have a texture rotate around over 2 seconds (2000 milliseconds). 360 degrees in a circle! */
        const float rotation = (((float) ((int) (now % 2000))) / 2000.0f) * 360.0f;

        TRACE("DrawFrame");

        /* as you can see from this, rendering draws over whatever was drawn before it. */
        SDL_SetRenderDrawColor(renderer, 0, 0, 0, 255);  /* black, full alpha */
        SDL_RenderClear(renderer);  /* start with a blank canvas. */

        /* Center this one, and draw it with some rotation so it spins! */
        dst_rect.x = ((float) (WINDOW_WIDTH - texture_width)) / 2.0f;
        dst_rect.y = ((float) (WINDOW_HEIGHT - texture_height)) / 2.0f;
        dst_rect.w = (float) texture_width;
        dst_rect.h = (float) texture_height;
        /* rotate it around the center of the texture; you can rotate it from a different point, too! */
        center.x = texture_width / 2.0f;
        center.y = texture_height / 2.0f;
        SDL_RenderTextureRotated(renderer, texture, NULL, &dst_rect, rotation, &center, SDL_FLIP_NONE);
    }

    StepComplete(background_thread, run_this_step, THREADED_DRAW);

    return true;
}

static bool PresentFrame(bool background_thread)
{
    const bool run_this_step = (background_thread == ((threaded & THREADED_PRESENT) != 0));
    if (run_this_step) {
        TRACE("PresentFrame");
        if (!SDL_RenderPresent(renderer)) {
            SDL_Log("SDL_RenderPresent failed: %s", SDL_GetError());
            return false;
        }
    }

    StepComplete(background_thread, run_this_step, THREADED_PRESENT);

    return true;
}

static SDL_AppResult DoFrame(bool background_thread)
{
    bool okay = true;

    okay = DrawFrame(background_thread) && okay;
    okay = PresentFrame(background_thread) && okay;

    if (!okay) {
        SDL_Log("DoFrame failed!");
        return SDL_APP_FAILURE;
    }

    return SDL_APP_CONTINUE;
}

static int SDLCALL RenderWorker(void *unused)
{
    TRACE("background thread");

    if (DoInit(true) != SDL_APP_CONTINUE) {
        SDL_Event e;
        SDL_zero(e);
        e.type = SDL_EVENT_QUIT;
        SDL_PushEvent(&e);
    } else {
        while (!SDL_GetAtomicInt(&quit)) {
            if (DoFrame(true) != SDL_APP_CONTINUE) {
                SDL_Event e;
                SDL_zero(e);
                e.type = SDL_EVENT_QUIT;
                SDL_PushEvent(&e);
                break;
            }
        }
    }

    TRACE("background thread terminating");
    return 0;
}


SDL_AppResult SDL_AppInit(void **appstate, int argc, char *argv[])
{
    int i;

    main_thread_id = SDL_GetCurrentThreadID();

    TRACE("main thread");

    for (i = 1; i < argc;) {
        bool okay = true;
        if (SDL_strcasecmp(argv[i++], "--threaded") == 0) {  /* THREADED RENDERING IS NOT SUPPORTED, THIS IS JUST FOR TESTING PURPOSES! */
            if (argv[i]) {
                const char *arg = argv[i++];
                if (SDL_strcasecmp(arg, "all") == 0) {
                    /* do everything on a background thread (window  */
                    threaded = 0xFFFFFFFF;
                } else if (SDL_strcasecmp(arg, "init") == 0) {
                    /* SDL_Init on a background thread. */
                    threaded |= THREADED_INIT;
                } else if (SDL_strcasecmp(arg, "create") == 0) {
                    /* Create window and renderer on background thread. */
                    threaded |= THREADED_CREATE;
                } else if (SDL_strcasecmp(arg, "texture") == 0) {
                    /* Upload texture on background thread. */
                    threaded |= THREADED_TEXTURE_UPLOAD;
                } else if (SDL_strcasecmp(arg, "draw") == 0) {
                    /* Rendering happens on background thread. */
                    threaded |= THREADED_DRAW;
                } else if (SDL_strcasecmp(arg, "present") == 0) {
                    /* Present happens on background thread. */
                    threaded |= THREADED_PRESENT;
                } else {
                    return SDL_APP_FAILURE;
                }
            } else {
                okay = false;
            }
        } else {
            okay = false;
        }

        if (!okay) {
            SDL_Log("USAGE: %s [--threaded all|init|create|texture|draw|present] ...", argv[0]);
            return SDL_APP_FAILURE;
        }
    }

    if (threaded) {
        main_semaphore = SDL_CreateSemaphore(0);
        if (!main_semaphore) {
            SDL_Log("main SDL_CreateSemaphore failed: %s", SDL_GetError());
            return SDL_APP_FAILURE;
        }
        background_semaphore = SDL_CreateSemaphore(0);
        if (!background_semaphore) {
            SDL_Log("background SDL_CreateSemaphore failed: %s", SDL_GetError());
            return SDL_APP_FAILURE;
        }
        thread = SDL_CreateThread(RenderWorker, "renderer", NULL);
        if (!thread) {
            SDL_Log("SDL_CreateThread failed: %s", SDL_GetError());
            return SDL_APP_FAILURE;
        }
    }

    return DoInit(false);
}

SDL_AppResult SDL_AppEvent(void *appstate, SDL_Event *event)
{
    TRACE("SDL_AppEvent");
    if (event->type == SDL_EVENT_QUIT) {
        return SDL_APP_SUCCESS;
    }
    return SDL_APP_CONTINUE;
}

SDL_AppResult SDL_AppIterate(void *appstate)
{
    TRACE("SDL_AppIterate");
    return DoFrame(false);
}

void SDL_AppQuit(void *appstate, SDL_AppResult result)
{
    TRACE("SDL_AppQuit");
    SDL_Log("platform='%s', video='%s', renderer='%s'", SDL_GetPlatform(), SDL_GetCurrentVideoDriver(), SDL_GetRendererName(renderer));
    if (thread) {
        SDL_SetAtomicInt(&quit, 1);
        SDL_SignalSemaphore(main_semaphore);
        SDL_SignalSemaphore(main_semaphore);
        SDL_SignalSemaphore(main_semaphore);
        SDL_SignalSemaphore(main_semaphore);
        SDL_SignalSemaphore(main_semaphore);
        SDL_SignalSemaphore(main_semaphore);
        SDL_SignalSemaphore(background_semaphore);
        SDL_SignalSemaphore(background_semaphore);
        SDL_SignalSemaphore(background_semaphore);
        SDL_SignalSemaphore(background_semaphore);
        SDL_SignalSemaphore(background_semaphore);
        SDL_SignalSemaphore(background_semaphore);
        SDL_WaitThread(thread, NULL);
    }
    SDL_DestroySemaphore(main_semaphore);
    SDL_DestroySemaphore(background_semaphore);
    SDL_DestroyTexture(texture);
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    TRACE("main thread terminating");
}

slime73 · 2024-10-11T18:29:44Z

Some things that might be useful to test:

resizing the window
minimizing the window
all of the above with the main thread checker active on macOS

Also with OpenGL, it's not legal to have the same context active on multiple threads at once, and different platforms may be more lax or more strict with enforcing that. That rules out CreateWindowAndRenderer from being thread-capable on all platforms I think, unless the context is deactivated at the end of GL/GLES renderer creation.

I also have some memories of OpenGL ES on iOS needing the right objects bound on the main thread during the event loop, but I'm not positive about that...

icculus · 2024-10-11T20:06:42Z

@slime73 is correct, OpenGL works if everything (including SDL_Init) are on a background thread. In other cases it will fail.

GPU (vulkan+x11) works with --threaded draw --threaded present. It'll also work if everything (including SDL_Init) is on a background thread.

software (with or without framebuffer acceleration) is the same.

X11 seems to want SDL_Init and SDL_CreateWindow to be on the same thread, but it doesn't have to be the main thread.

icculus · 2024-10-12T00:48:28Z

Fixed some bugs in the test program (updated above) and OpenGL works on wayland with only --threaded draw ... present and texture upload, etc, have to happen on the main thread.

I think we could probably go so far as to say the GL renderer sets a NULL context current after creation, and then sets a flag to set it current on the first draw call, and demand that every rendering call happens only from that thread, which will probably get --threaded texture and present and such working.

I'm just talking out loud here.

kanjitalk755 · 2024-10-12T09:01:34Z

[macOS]
When draw and present were placed on a background thread, a problem occurred with window resizing.

Build with the SDL_CreateWindowAndRenderer flag set to SDL_WINDOW_RESIZABLE.
Set followling:

export MTL_DEBUG_LAYER=1
export MTL_DEBUG_LAYER_ERROR_MODE=assert

Run with arguments --threaded draw --threaded present.
If you keep resizing the window quickly, will get a Metal Validation Assert.

-[MTLDebugRenderCommandEncoder setScissorRect:]:4048: failed assertion `Set Scissor Rect Validation
(rect.x(0) + rect.width(909))(909) must be <= render pass width(901)
(rect.y(0) + rect.height(658))(658) must be <= render pass height(653)
'
Process 1733 stopped
* thread #2, name = 'renderer', stop reason = hit program assert
    frame #4: 0x000000018bf37194 Metal`MTLReportFailure.cold.1 + 48
Metal`bool MTLGetEnvCase<MTLErrorModeType>(char const*, MTLErrorModeType&, std::__1::vector<std::__1::pair<char const*, MTLErrorModeType>, std::__1::allocator<std::__1::pair<char const*, MTLErrorModeType>>> const&) (.cold.1):
->  0x18bf37194 <+0>:  pacibsp 
    0x18bf37198 <+4>:  sub    sp, sp, #0x40
    0x18bf3719c <+8>:  stp    x22, x21, [sp, #0x10]
    0x18bf371a0 <+12>: stp    x20, x19, [sp, #0x20]
Target 0: (sdl_test) stopped.
(lldb) bt
* thread #2, name = 'renderer', stop reason = hit program assert
    frame #0: 0x0000000181a395d0 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x0000000181a71c20 libsystem_pthread.dylib`pthread_kill + 288
    frame #2: 0x000000018197ea30 libsystem_c.dylib`abort + 180
    frame #3: 0x000000018197dd20 libsystem_c.dylib`__assert_rtn + 284
  * frame #4: 0x000000018bf37194 Metal`MTLReportFailure.cold.1 + 48
    frame #5: 0x000000018bf13db0 Metal`MTLReportFailure + 464
    frame #6: 0x000000018bf09d58 Metal`_MTLMessageContextEnd + 876
    frame #7: 0x000000018230f824 MetalTools`-[MTLDebugRenderCommandEncoder setScissorRect:] + 244
    frame #8: 0x0000000100aa841c SDL3`METAL_SetScissor(commandBuffer=0x000000014b889e00, scissor=0x000000016fe86978) at SDL_gpu_metal.m:2192:9
    frame #9: 0x0000000100aa7f84 SDL3`METAL_BeginRenderPass(commandBuffer=0x000000014b889e00, colorTargetInfos=0x000000016fe86b20, numColorTargets=1, depthStencilTargetInfo=0x0000000000000000) at SDL_gpu_metal.m:2340:9
    frame #10: 0x00000001008e1610 SDL3`SDL_BeginGPURenderPass_REAL(command_buffer=0x000000014b889e00, color_target_infos=0x000000016fe86b20, num_color_targets=1, depth_stencil_target_info=0x0000000000000000) at SDL_gpu.c:1462:5
    frame #11: 0x00000001008e0a98 SDL3`SDL_GPU_BlitCommon(command_buffer=0x000000014b889e00, info=0x000000016fe86d08, blit_linear_sampler=0x00006000028bdef0, blit_nearest_sampler=0x00006000028bdee0, blit_vertex_shader=0x0000600002a98320, blit_from_2d_shader=0x0000600002a983a0, blit_from_2d_array_shader=0x0000600002a98300, blit_from_3d_shader=0x0000600002a983e0, blit_from_cube_shader=0x0000600002a984e0, blit_from_cube_array_shader=0x0000600002a98520, blit_pipelines=0x000000014b6462e0, blit_pipeline_count=0x000000014b6462e8, blit_pipeline_capacity=0x000000014b6462ec) at SDL_gpu.c:314:19
    frame #12: 0x0000000100aaa6a8 SDL3`METAL_Blit(commandBuffer=0x000000014b889e00, info=0x000000016fe86d08) at SDL_gpu_metal.m:2992:5
    frame #13: 0x00000001008e8f2c SDL3`SDL_BlitGPUTexture_REAL(command_buffer=0x000000014b889e00, info=0x000000016fe86d08) at SDL_gpu.c:2538:5
    frame #14: 0x00000001008873dc SDL3`GPU_RenderPresent(renderer=0x000000014b646550) at SDL_render_gpu.c:983:5
    frame #15: 0x00000001009de100 SDL3`SDL_RenderPresent_REAL(renderer=0x000000014b646550) at SDL_render.c:4989:10
    frame #16: 0x000000010096cab8 SDL3`SDL_RenderPresent(a=0x000000014b646550) at SDL_dynapi_procs.h:780:1
    frame #17: 0x0000000100003b70 sdl_test`PresentFrame(background_thread=true) at thread.c:179:8
    frame #18: 0x0000000100003504 sdl_test`DoFrame(background_thread=true) at thread.c:195:9
    frame #19: 0x0000000100003324 sdl_test`RenderWorker(unused=0x0000000000000000) at thread.c:217:8
    frame #20: 0x00000001008bb530 SDL3`SDL_RunThread(thread=0x00006000001bc230) at SDL_thread.c:323:18
    frame #21: 0x00000001008dcf14 SDL3`RunThread(data=0x00006000001bc230) at SDL_systhread.c:69:5
    frame #22: 0x0000000181a71f94 libsystem_pthread.dylib`_pthread_start + 136
(lldb)

icculus added this to the 3.2.0 milestone Oct 11, 2024

Kontrabant mentioned this issue Nov 8, 2024

SDL2: SDL_EventFilter breaks window resizing for software renderer when event is filtered #11423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Can the 2D renderer be made thread safe? #11159

Can the 2D renderer be made thread safe? #11159

icculus commented Oct 11, 2024

slouken commented Oct 11, 2024

slouken commented Oct 11, 2024

thatcosmonaut commented Oct 11, 2024

slouken commented Oct 11, 2024

thatcosmonaut commented Oct 11, 2024

andreasgrabher commented Oct 11, 2024

icculus commented Oct 11, 2024 •

edited

Loading

slime73 commented Oct 11, 2024 •

edited

Loading

icculus commented Oct 11, 2024

icculus commented Oct 12, 2024

kanjitalk755 commented Oct 12, 2024

Can the 2D renderer be made thread safe? #11159

Can the 2D renderer be made thread safe? #11159

Comments

icculus commented Oct 11, 2024

slouken commented Oct 11, 2024

slouken commented Oct 11, 2024

thatcosmonaut commented Oct 11, 2024

slouken commented Oct 11, 2024

thatcosmonaut commented Oct 11, 2024

andreasgrabher commented Oct 11, 2024

icculus commented Oct 11, 2024 • edited Loading

slime73 commented Oct 11, 2024 • edited Loading

icculus commented Oct 11, 2024

icculus commented Oct 12, 2024

kanjitalk755 commented Oct 12, 2024

icculus commented Oct 11, 2024 •

edited

Loading

slime73 commented Oct 11, 2024 •

edited

Loading