This bug report was migrated from our old Bugzilla tracker.

Reported in version: HG 2.1
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2020-10-04 16:10:26 +0000, Rene Dudfield wrote:

There’s a RLE bug in SDL with SDL_SetSurfaceRLE.

Functions like SDL_LockSurface, and SDL_MUSTLOCK use surface->flags. However SDL_SetSurfaceRLE doesn’t update surface->flags , only the internal surface->map->info.flags.

SDL_SetSurfaceRLE(surf, SDL_TRUE);
assert (SDL_MUSTLOCK(surf)); // this fails, because surf->flags 
                             // doesn't have the SDL_RLEACCEL flag.

SDL_SetSurfaceRLE should update surface->flags with the SDL_RLEACCEL flag set when it is RLE encoded.

On 2020-10-14 19:28:14 +0000, Dan Lawrence wrote:

I spent a while looking into this for pygame.

I think we could actually solve a lot of the confusion we are having in matching pygame using SDL2 to pygame using SDL1, by adding a function to SDL that lets people inspect the SDL_COPY_RLE_DESIRED status directly. Something like:

SDL_bool
SDL_HasSurfaceRLE(SDL_Surface * surface)
{
    if (!surface) {
        return SDL_FALSE;
    }

    if (!(surface->map->info.flags & SDL_COPY_RLE_DESIRED)) {
        return SDL_FALSE;
    }

    return SDL_TRUE;
}

This mirrors the existing function SDL_HasColorKey() which lets you do the same for the SDL_COPY_COLORKEY blit info flag.

I believe previously SDL1 let you set the SDL_RLEACCEL immediately on a surface and as a result pygame has a corresponding RLEACCEL flag that is intended to let you inspect whether a surface is supposed to be using RLE or not.

Now, in SDL2, when you set a surface to be RLE, SDL_COPY_RLE_DESIRED gets set initially and remains the only thing on a surface until you actually blit the surface onto something at which point SDL_RLEACCEL is finally set.

Anyway this SDl1->SDL2 change was causing a bunch of pygame tests to fail, convincing us that RLE was broken somehow, when I think it is probably fine. BAsically, it'd be nice to have this convenience function so we can make pygame smoothly backwards compatible - even if hardly anyone actually uses RLE these days.

I'm not sure how to make patches to SDL, but I added this function locally and built SDL on windows and it all seems to function how I would expect and fixes the pygame tests in a sensible seeming way.

On 2020-10-18 06:38:29 +0000, Sylvain wrote:

Do you have a test-case for this ?

On 2020-10-18 07:12:35 +0000, Rene Dudfield wrote:

Hello,

our wish is for a SDL_HasSurfaceRLE function:

SDL_bool
SDL_HasSurfaceRLE(SDL_Surface * surface)
{
    if (!surface) {
        return SDL_FALSE;
    }

    if (!(surface->map->info.flags & SDL_COPY_RLE_DESIRED)) {
        return SDL_FALSE;
    }

    return SDL_TRUE;
}

However, to support older versions of SDL we have had to hack in some access to the private "surface->map->info.flags".

We had a misunderstanding of the RLE changes. I think the original reported issue is not an actual bug, but a change in how RLE works in SDL2 compared to SDL1. This could perhaps be listed in the migration guide.

However the ability to know if a surface is using RLE has disappeared from SDL2 (before surface->flags could be queried). This is why we ask for a SDL_HasSurfaceRLE to match the recently added SDL_HasColorKey.

On 2020-10-18 07:54:33 +0000, Sylvain wrote:

Thanks, added in https://hg.libsdl.org/SDL/rev/159ec99cd0e6

On 2020-10-18 10:28:12 +0000, Rene Dudfield wrote:

Thanks :)

On 2020-10-19 17:30:56 +0000, Rene Dudfield wrote:

Marking as resolved.