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

Simple bug fix in SDL_render #12027

Closed
ThrudTheBarbarian opened this issue Jan 20, 2025 · 1 comment
Closed

Simple bug fix in SDL_render #12027

ThrudTheBarbarian opened this issue Jan 20, 2025 · 1 comment
Milestone

Comments

@ThrudTheBarbarian
Copy link

Looks like the colourspace logic is initialising the wrong colourspace before testing things...

Simple patch (really doesn't seem necessary to fork/create branch/create pull-request, it's only 1 line :)

diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c
index 06c0381ca..eed841561 100644
--- a/src/render/SDL_render.c
+++ b/src/render/SDL_render.c
@@ -1634,7 +1634,7 @@ SDL_Texture *SDL_CreateTextureFromSurface(SDL_Renderer *renderer, SDL_Surface *s
         }
     }
 
-    texture_colorspace = SDL_GetSurfaceColorspace(surface);
+    surface_colorspace = SDL_GetSurfaceColorspace(surface);
 
     // Try to have the best pixel format for the texture
     // No alpha, but a colorkey => promote to alpha

The first reference to the colourspace is later on down the file, line 1700 at this moment in time, where tests on surface_colorspace are used to determine texture_colorspace

    if (surface_colorspace == SDL_COLORSPACE_SRGB_LINEAR ||
        SDL_COLORSPACETRANSFER(surface_colorspace) == SDL_TRANSFER_CHARACTERISTICS_PQ) {
        if (SDL_ISPIXELFORMAT_FLOAT(format)) {
            texture_colorspace = SDL_COLORSPACE_SRGB_LINEAR;
        } else if (SDL_ISPIXELFORMAT_10BIT(format)) {
            texture_colorspace = SDL_COLORSPACE_HDR10;
        } else {
            texture_colorspace = SDL_COLORSPACE_SRGB;
        }
    }

So it's pretty clear the intent was to set surface_colorspace not texture_colorspace

Seems that re-implementing the render api to work on top of the GPU API can have some benefits after all :)

@slouken
Copy link
Collaborator

slouken commented Jan 20, 2025

Fixed, thanks!

@slouken slouken added this to the 3.2.0 milestone Jan 20, 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

2 participants