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

SDL_RenderReadPixels() doesn't seem to work with offscreen render-targets. #11776

Closed
ThrudTheBarbarian opened this issue Dec 29, 2024 · 10 comments
Assignees
Milestone

Comments

@ThrudTheBarbarian
Copy link

I've got an app that is creating a bunch of background targets and switching the renderer between them frequently. Basically every view or image is a texture. To make this easier, there is a texture-manager, and I refer to all the textures by an integer id.

All the imagery is backed by a texture, but sometimes I might want to write an image to disk, so I've been trying to use SDL_RenderReadPixels() to pull the data back from the GPU into CPU land. The problem is that even though I'm setting the render-target, what I get back is the main window's pixels, not the pixels of the offscreen render-target I just chose.

The below is the test-app for the framework. As a debugging tool, when I change the radio buttons, it tries to load an image, then immediately save it to /tmp/test.png.

test

I put in some logging for:

  • when the radio button is pressed
  • whenever the render-target is selected
  • when the save operation is complete

And I see: (trimming a whole bunch of render-target selection before I press the button as the UI is rendered out)

radio button pressed:<AZButton: 0x12deb9b10>
Loaded image, texture is 29
Start to save texture 29
lock focus on 29
Done saving
lock focus on 15
lock focus on 16

(The last two, 15 and 16, are the radio buttons redrawing themselves, so we can ignore). The point is that the renderer is switched to the texture representing the image (texture id 29) all the time the save operation is in progress, but the output I get is this exact screen in the image above, not the image I told it to load.

The load-image process is wrapped up in framework code, but basically boils down to:

SDL_Surface *img = IMG_Load(path);
SDL_Texture *texture = SDL_CreateTextureFromSurface(_renderer, img);
[store and return the integer-id that we stored it under]

I did look at the example code for SDL_RenderReadPixels() but it doesn't seem to deal with offscreen textures, just with copying from the main window. Which definitely works! :)

This is with the 2D render API btw. Created via SDL_CreateWindowAndRenderer()

@slouken
Copy link
Collaborator

slouken commented Dec 29, 2024

What platform are you testing on and which renderer are you using?

@slouken slouken added this to the 3.2.0 milestone Dec 29, 2024
@slouken slouken added the waiting Waiting on user response label Dec 29, 2024
@ThrudTheBarbarian
Copy link
Author

This is on a Mac, and the renderer is the 2D renderer ( Created via SDL_CreateWindowAndRenderer())

@slouken
Copy link
Collaborator

slouken commented Dec 29, 2024

What is the output of SDL_GetRendererName()?

@ThrudTheBarbarian
Copy link
Author

Ah, sorry, not really understanding your question there. It outputs 'metal'.

@slouken slouken removed the waiting Waiting on user response label Dec 30, 2024
@slouken
Copy link
Collaborator

slouken commented Dec 30, 2024

Okay, thanks for the info!

@icculus
Copy link
Collaborator

icculus commented Dec 30, 2024

I'll try to reproduce, this sounds like a legit bug.

@icculus icculus added the icculus-priority Things @icculus is looking at within a larger milestone. label Jan 8, 2025
@icculus
Copy link
Collaborator

icculus commented Jan 9, 2025

This is working here with this test program:

#include <SDL3/SDL.h>
#include <SDL3/SDL_main.h>

static void take_snapshot(SDL_Renderer *renderer, const char *fname)
{
    SDL_Surface *surface = SDL_RenderReadPixels(renderer, NULL);
    if (surface) {
        SDL_SaveBMP(surface, fname);
        SDL_DestroySurface(surface);
    }
}

int main(int argc, char **argv)
{
    SDL_Window *window;
    SDL_Renderer *renderer;
    SDL_Init(SDL_INIT_VIDEO);
    SDL_CreateWindowAndRenderer("Hello SDL", 640, 480, 0, &window, &renderer);
    SDL_Texture *rt = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_RGBA32, SDL_TEXTUREACCESS_TARGET, 640, 480);

    SDL_Log("Current SDL 2D render backend: '%s'", SDL_GetRendererName(renderer));

    bool done = false;
    while (!done) {
        bool snap = false;
        SDL_Event e;
        while (SDL_PollEvent(&e)) {
            if (e.type == SDL_EVENT_QUIT) {
                done = true;
            } else if ((e.type == SDL_EVENT_KEY_DOWN) && (e.key.key == SDLK_SPACE)) {
                snap = true;
            }
        }

        /* window framebuffer is blue, render target is red. */

        SDL_SetRenderTarget(renderer, rt);
        SDL_SetRenderDrawColor(renderer, 255, 0, 0, 255);
        SDL_RenderClear(renderer);

        if (snap) {
            take_snapshot(renderer, "render-target.bmp");
        }

        SDL_SetRenderTarget(renderer, NULL);
        SDL_SetRenderDrawColor(renderer, 0, 0, 255, 255);
        SDL_RenderClear(renderer);
        if (snap) {
            take_snapshot(renderer, "framebuffer.bmp");
        }

        SDL_RenderPresent(renderer);
    }

    SDL_Quit();
    return 0;
}

Basically it clears the window to blue, clears a render target to red, and when you press the space bar it calls SDL_RenderReadPixels on each and saves the data to .bmp files ... both files are correct when I do it here on a M1 Mac Mini, using either the Metal or OpenGL backends.

Running macOS Sonoma 14.4.1, using the latest in SDL3's revision control.

If this test program works for you, too, take another look at your code to make sure it's not something silly like SDL_SetRenderTarget() accidentally got called with a NULL pointer before the ReadPixels operation. If it's still a mystery after that, let's try to figure out next steps to solve your problem.

@ThrudTheBarbarian
Copy link
Author

Ok, so this works (sort of, see [*] below) for me too - so I must have a set-to-NULL somewhere in the chain. It's can be a long, convoluted chain, which I thought I'd worked through, but presumably I missed something.

Thanks for doing the legwork, sorry for the noise :)

[*] First off, I couldn't find where the files were being written to. Apple's sandbox is just annoying. Forcing a write to /tmp/... made the files appear, but they couldn't be loaded by Preview. I'd get:

Screenshot 2025-01-09 at 12 56 51 pm

However, if I ran 'file' on the outputted file, it reported it correctly:

@Mac ~ % file /tmp/render-target.bmp 
/tmp/render-target.bmp: PC bitmap, Windows 95/NT4 and newer format, 640 x 480 x 32, cbSize 1228922, bits offset 122

I got the same 'unrecognised file' when I tried to load it using "xv" (yes, I am that old). If I ran the file through bmptoppm though, it converted the files correctly, giving me a red and blue ppm as expected.

So I'm wondering if there's an endian issue in the SDL_SaveBMP() method, or maybe Preview doesn't support the "Windows 95/NT4" format. Or perhaps it's just an OS bug... I don't know... :)

In any event, it looks as though I need to track down where the renderer target is being set to NULL...

@ThrudTheBarbarian
Copy link
Author

Just to follow up on this - I found the bug and it was in my code [blush]

The problem wasn't the render-target being set to NULL explicitly . It was that I was creating some textures with SDL_CreateTextureFromSurface() which creates them with SDL_TEXTUREACCESS_STATIC as the access hint. So then when I tried to set the render-target to that texture, it was failing, and falling back to the NULL texture (the window).

Anyway, now I create my texture from the surface and immediately create another texture and copy into it, throwing away the old one that was generated from the surface, and setting SDL_TEXTUREACCESS_TARGET on my new one.

It was all in the documentation. I should maybe try reading that sometime... :)

@icculus
Copy link
Collaborator

icculus commented Jan 10, 2025

I'm glad it's working now!

While working on this, I also noticed that Preview doesn't like our (32-bit) bitmaps, so we're tracking that over in #11903, if you're curious.

@icculus icculus removed the icculus-priority Things @icculus is looking at within a larger milestone. label Jan 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

No branches or pull requests

3 participants