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

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

Open
icculus opened this issue Oct 11, 2024 · 9 comments
Open

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

icculus opened this issue Oct 11, 2024 · 9 comments
Milestone

Comments

@icculus
Copy link
Collaborator

icculus commented Oct 11, 2024

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.

@icculus icculus added this to the 3.2.0 milestone Oct 11, 2024
@slouken
Copy link
Collaborator

slouken commented Oct 11, 2024

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
Copy link
Collaborator

slouken commented Oct 11, 2024

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

@thatcosmonaut
Copy link
Collaborator

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
Copy link
Collaborator

slouken commented Oct 11, 2024

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
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator Author

icculus commented Oct 11, 2024

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.

#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 *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 ran_this_step)
{
    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(run_this_step);

    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(run_this_step);

    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(run_this_step);

    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(run_this_step);

    return true;
}

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

    StepComplete(run_this_step);

    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) {
        semaphore = SDL_CreateSemaphore(0);
        if (!semaphore) {
            SDL_Log("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_WaitThread(thread, NULL);
    }
    SDL_DestroySemaphore(semaphore);
    SDL_DestroyTexture(texture);
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    TRACE("main thread terminating");
}

@slime73
Copy link
Contributor

slime73 commented Oct 11, 2024

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
Copy link
Collaborator Author

icculus commented Oct 11, 2024

@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.

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

No branches or pull requests

5 participants