| Summary: | SDL_SetSurfaceRLE SDL_RLEACCEL bug with locking, and flags. | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Rene Dudfield <renesd> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | critical | ||
| Priority: | P2 | CC: | danintheshed, sylvain.becker |
| Version: | HG 2.1 | ||
| Hardware: | All | ||
| OS: | All | ||
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.
Do you have a test-case for this ? 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.
Thanks, added in https://hg.libsdl.org/SDL/rev/159ec99cd0e6 Thanks :) Marking as resolved. |
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.