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_SoftStretch compatibility #251

Open
Starbuck5 opened this issue Jan 22, 2025 · 13 comments · Fixed by #296
Open

SDL_SoftStretch compatibility #251

Starbuck5 opened this issue Jan 22, 2025 · 13 comments · Fixed by #296
Milestone

Comments

@Starbuck5
Copy link

Hello, I worked a bit on testing this against pygame-ce (pygame-community/pygame-ce#3287). I feel that pygame-ce is a pretty bad scenario for this library because we do plenty of weird stuff and also try to expose almost all SDL APIs.

I ran into some crashes that I have yet to debug, but one thing I do see straight away is differences in SDL_SoftStretch.

In our implementation of pygame.transform.scale we lock the surface and then call SDL_SoftStretch. Now that softstretch is actually a blit, this yields a "surface can't be blit while locked" error.

On another note, I don't understand how softstretch can be replaced with a scaled blit. If you're trying to scale a surface with alpha wouldn't the blit scaled blend with whatever is in the destination surface?

@slouken
Copy link
Collaborator

slouken commented Feb 1, 2025

Can you provide the parameters for the call, including formats of the src and dst surfaces? SDL_SoftStretch() isn't a scaled blit, but some format conversion paths through it might involve some pixel conversion or blitting, so it would be helpful to know exactly what you're running into.

Also, I've been looking forward to the pygame test results. You guys put the API through a torture test, and if sdl2-compat can pass that, I'm pretty confident it'll handle almost anything a real game can toss at us.

@Starbuck5
Copy link
Author

If I'm reading the source of SDL2 compat properly the functions are implemented as a scaled blit: https://github.com/libsdl-org/sdl2-compat/blob/main/src/sdl2_compat.c#L3370-L3380

@slouken
Copy link
Collaborator

slouken commented Feb 1, 2025

Ah, you are correct. I was looking at the SDL3 implementation. I'm not sure why they differ. I'll have to take a look.

@slouken slouken added this to the 2.30.52 milestone Feb 1, 2025
@slouken
Copy link
Collaborator

slouken commented Feb 1, 2025

So SDL_SoftStretch() is intended to be equivalent to blitting with blending turned off, and sdl2-compat assumes that blending is turned off for surfaces that it's stretching, which is obviously not correct. Do you have a minimal repro of incorrect behavior we can use to validate any fixes?

@Starbuck5
Copy link
Author

I haven’t made a reproducer in C yet and the reproducer in Python fails because of the locking issue.

Interestingly, looking at the SDL source the softstretch function still exists, it’s just not publicly exposed.

@Starbuck5
Copy link
Author

Our goal in pygame-ce is to get all our modules compiling with both SDL2 and SDL3, so I think the way we’ll do it is by vendoring the function into our own source.

@slouken
Copy link
Collaborator

slouken commented Feb 1, 2025

I'm considering re-adding it to the API. It was removed as a simplification, but if there's a legitimate use case for it still, we can put it back.

@slouken slouken modified the milestones: 2.30.52, 2.30.54 Feb 2, 2025
@Starbuck5
Copy link
Author

I've generated some examples to help test.

#define SDL_MAIN_HANDLED
#include <SDL.h>

int main() {
    SDL_Surface* surf = SDL_LoadBMP("alien.bmp");

    SDL_Surface* larger_surf = 
        SDL_CreateRGBSurfaceWithFormat(0, 100, 100, surf->format->BytesPerPixel, surf->format->format);

    SDL_FillRect(larger_surf, NULL, SDL_MapRGB(surf->format, 0, 0, 0));

    SDL_SoftStretch(surf, NULL, larger_surf, NULL);

    SDL_SaveBMP(larger_surf, "output.bmp");

    return 0;
}

Source file, zipped because github doesn't like bmp uploads. alien.zip

SDL2 output (reencoded as png)
Image

SDL2 compat output (reencoded as png)
Image

#define SDL_MAIN_HANDLED
#include <SDL.h>
#include <stdio.h>

int main() {
    SDL_Surface* surf = SDL_LoadBMP("alien.bmp");

    SDL_Surface* larger_surf = 
        SDL_CreateRGBSurfaceWithFormat(0, 100, 100, surf->format->BytesPerPixel, surf->format->format);

    SDL_FillRect(larger_surf, NULL, SDL_MapRGB(surf->format, 0, 0, 0));

    SDL_LockSurface(surf);
    if (SDL_SoftStretch(surf, NULL, larger_surf, NULL) < 0) {
        printf("Error: %s", SDL_GetError());
        return -1;
    }
    SDL_UnlockSurface(surf);

    SDL_SaveBMP(larger_surf, "output.bmp");

    return 0;
}

SDL2 produces image as expected still, SDL3 produces error: "Surfaces must not be locked during blit"

@slouken
Copy link
Collaborator

slouken commented Feb 2, 2025

So I'm less concerned about the locked error, we can fix that. I'm more concerned about incorrect output. Is the output correct if you comment out that check in SDL?

@Starbuck5
Copy link
Author

In my second code, if SDL_SoftStretch is not error checked it outputs a fully black surface.

slouken added a commit to slouken/sdl2-compat that referenced this issue Feb 2, 2025
@slouken slouken closed this as completed in 3ffd925 Feb 2, 2025
@slouken
Copy link
Collaborator

slouken commented Feb 2, 2025

Okay, thank you for the excellent test case. I re-added SDL_SoftStretch() to the API, and confirmed that fixes this issue.

@Starbuck5
Copy link
Author

Thank you!

@slouken
Copy link
Collaborator

slouken commented Feb 3, 2025

I'm going to reopen this, as it relies on a version of SDL that isn't released yet. Once SDL is released with the new function in a couple of weeks, I'll re-add this.

@slouken slouken reopened this Feb 3, 2025
slouken added a commit to slouken/sdl2-compat that referenced this issue Feb 3, 2025
We may be releasing further updates to sdl2-compat before then.

Reopens libsdl-org#251
slouken added a commit to slouken/sdl2-compat that referenced this issue Feb 3, 2025
We may be releasing further updates to sdl2-compat before then.

Reopens libsdl-org#251
slouken added a commit that referenced this issue Feb 3, 2025
We may be releasing further updates to sdl2-compat before then.

Reopens #251
@slouken slouken added this to the SDL 3.2.4 milestone Feb 4, 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

Successfully merging a pull request may close this issue.

2 participants