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_CreateRenderer errors out if Window already has a surface => Should use existing surface instead #11870

Open
Robert-M-Muench opened this issue Jan 6, 2025 · 4 comments
Milestone

Comments

@Robert-M-Muench
Copy link

This code first checks if the Window has a surface and gives an error, but further down in line 1000 there is a check for an existing surface, in which case SDL's software renderer is used, which IMO makes a lot of sense.

However, this code will never be reached because of the former check.

SDL/src/render/SDL_render.c

Lines 985 to 1002 in c7584df

if (window && SDL_WindowHasSurface(window)) {
SDL_SetError("Surface already associated with window");
goto error;
}
if (window && SDL_GetRenderer(window)) {
SDL_SetError("Renderer already associated with window");
goto error;
}
hint = SDL_GetHint(SDL_HINT_RENDER_VSYNC);
if (hint && *hint) {
SDL_SetNumberProperty(props, SDL_PROP_RENDERER_CREATE_PRESENT_VSYNC_NUMBER, SDL_GetHintBoolean(SDL_HINT_RENDER_VSYNC, true));
}
if (surface) {
#ifdef SDL_VIDEO_RENDER_SW
const bool rc = SW_CreateRendererForSurface(renderer, surface, props);

Unfortunately, I don't have enough knowledge about the code(base) to propose a good fix.

@slouken
Copy link
Collaborator

slouken commented Jan 6, 2025

What's the use case here?

It would be unexpected to get the software renderer if a software renderer has already been created and the code is unaware of that, and if the code is aware of that, why not use the existing software renderer?

@Robert-M-Muench
Copy link
Author

Robert-M-Muench commented Jan 7, 2025

When using SW renderer(s) there is no need to limit it to one. All work against window->surface (the pixel-buffer), so it shouldn't harm to have several working on the same pixel-buffer.

Why did I hit this limitation: I have a 3rd party rendering lib (Blend2D in may case), which has some high-performance functions, and more graphic primitives. I created a simple benchmark which you could switch between using a key, by implementing the same code using SDL's SW renderer, and Blend2D's. Since both should work in the same window, I already allocated a window->surface before creating SDL's software renderer, which then failed.

I'm always trying to avoid any non-necessary limitations in libraries. Only having one renderer per window->surface is such a one. If I want to use 20 different software renderes, all can work with window->surface.

@slouken
Copy link
Collaborator

slouken commented Jan 7, 2025

The check on line 1000 is creating a renderer for a surface, not a window that has a surface. Can you put together a simple repro case for this?

@slouken slouken added this to the 3.x milestone Jan 7, 2025
@Robert-M-Muench
Copy link
Author

The check on 985 is the problematic one. If a window has a surface, line 1000 is never reached.

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

2 participants