Inexplainable race condition in SDL Renderer

andreas_g · October 28, 2022, 11:48am

You will need the latest development version of SDL2. Or you can just remove the call to SDL_GetWindowSizeInPixels(). It is only required for proper window resize and not relevant for the issue.

Sanette · October 28, 2022, 3:59pm

ok I was able to compile!

But. I don’t see your bug

andreas_g · October 28, 2022, 4:45pm

That is what makes this issue so annoying. It is highly dependent on timings. If I run the application in debug mode (slower) I see the problem way more often than when running it in normal mode. I don‘t think the bug is in my code, but maybe someone can see a mistake when reading through it? Maybe some SDL developer can check if there is something in SDL that might cause this?

sjr · October 29, 2022, 2:40am

Is it possible you aren’t setting blitUI everywhere you need to? Or that something is clearing it?

andreas_g · October 29, 2022, 4:33am

No, all relevant parts of the code are reached.

rtrussell · October 29, 2022, 9:20am

I know this isn’t really helpful, but I’ve not seen any evidence of a similar issue in my app (which is also a kind of emulator) on multiple platforms - Windows, MacOS, Linux, Android, iOS and in-browser (Emscripten) - nor have any of my users reported it.

However I’m using the OpenGL backend everywhere (so if it’s related to Direct3D or Metal, I won’t have encountered it) and I’m not running anything later than SDL 2.0.22 on any of those platforms.

ROSY · October 29, 2022, 10:52am

Have you checked whether this game of multithreading gives real acceleration?

andreas_g · October 29, 2022, 3:52pm

By distributing tasks among several CPU cores I get about twice the performance for the critical part of the application (at least on older systems).

andreas_g · November 8, 2022, 6:53am

Still no idea what might be wrong? I might have to live with this bug, but after restructuring my app to do all rendering on the main thread another problem came up. I now have a major efficiency issue. While the emulator-thread get’s some extra performance because event handling is now on the other thread, my main thread uses way too much CPU and GPU.

I now do the rendering at 68 Hz (refresh rate of the emulated system). Before it was 60 Hz (with VSYNC from my host system). So I would expect just some minor changes. But the CPU and GPU usage almost doubled. Maybe someone can see the problem? Or someone knows a way to handle this (render screens from buffers to up to 4 windows at 68 Hz) more efficiently?

The code is here: previous / Code / [r1221] /branches/branch_softfloat

Relevant files are src/main.c, src/fast_screen.c, src/statusbar.c and src/dimension/nd_sdl.cpp.

Sanette · November 14, 2022, 9:08am

first of all, like several people said, you really need to use SDL_LockTexture() + unlock
instead of SDL_UpdateTexture. In my experience it is vastly faster.

Daniel_Gibson · November 14, 2022, 9:33am

you really need to use SDL_LockTexture() + unlock
instead of SDL_UpdateTexture. In my experience it is vastly faster.

Really? On which platform/render backend? I glanced at the code of several backends and SDL_UpdateTexture() and SDL_LockTexture()+SDL_UnlockTexture() looked pretty similar (and sometimes one is implemented with the other, like the OpenGL SDL_UnlockTexture() calls the SDL_UpdateTexture() implementation)

Sanette · November 14, 2022, 9:48am

I did some tests a while ago on linux/opengl

but now you give me some doubts… let me check my old code

Daniel_Gibson · November 14, 2022, 10:24am

if it was a while ago, maybe the code in SDL has changed since, but currently it looks like this:

github.com

libsdl-org/SDL/blob/eef4d3c86a653f91b7221c80809ba8ab56f94cf1/src/render/opengl/SDL_render_gl.c#L847-L859

      
        
            static void
            GL_UnlockTexture(SDL_Renderer * renderer, SDL_Texture * texture)
            {
                GL_TextureData *data = (GL_TextureData *) texture->driverdata;
                const SDL_Rect *rect;
                void *pixels;
            
            
    rect = &data->locked_rect;
                pixels =
                    (void *) ((Uint8 *) data->pixels + rect->y * data->pitch +
                              rect->x * SDL_BYTESPERPIXEL(texture->format));
                GL_UpdateTexture(renderer, texture, rect, pixels, data->pitch);
            }

and GL_UpdateTexture() is what’s called by SDL_UpdateTexture() as well, in the common case, see

github.com

libsdl-org/SDL/blob/eef4d3c86a653f91b7221c80809ba8ab56f94cf1/src/render/SDL_render.c#L1820-L1826

      
        
            } else {
                SDL_Renderer *renderer = texture->renderer;
                if (FlushRenderCommandsIfTextureNeeded(texture) < 0) {
                    return -1;
                }
                return renderer->UpdateTexture(renderer, texture, &real_rect, pixels, pitch);
            }

(and another case goes through SDL_UpdateTextureNative(), which uses SDL_LockTexture(), SDL_ConvertPixels() and SDL_UnlockTexture())

Sanette · November 14, 2022, 10:28am

interesting. I’ll try to make some new tests. So why does the doc say “This is a fairly slow function, intended for use with static textures that do not change often.” for SDL_UpdateTexture ?

Daniel_Gibson · November 14, 2022, 10:34am

I have no idea why the doc says that, maybe it used to be true, or someone thought it was true and wrote it into the wiki (and then it was later synced into the doc comments in the header)?

I only glanced at the Direct3D implementations a while ago (I’m not really familiar with D3D, but know a little OpenGL), but without having compared the different functions side-by-side, IIRC that the code in the UpdateTexture() implementation roughly did the same thing as the one in LockTexture() + UnlockTexture() (with memcpy() or something in between)