Closed smcv closed 1 year ago
@slouken, @icculus: I think this is ready for at least a preliminary "is this a good direction?" review, but it isn't sufficiently tested to land it yet.
This seems like a good direction to me.
Rebased on main and fixed a missing NULL check. Briefly tested with:
and seems to work as intended.
I also contributed a fix for the incorrect free (PerlGameDev/SDL#306) which was applied to Debian's libsdl-perl package, but I haven't seen any sign of it being reviewed upstream. I also accidentally found PerlGameDev/SDL#307 while testing that change, and it hasn't had any response either, so PerlGameDev/SDL might be relatively dead.
Okay, well let's merge this then.
Thanks!
In #305, we have trouble with libsdl-perl making assumptions about the video surface never being freed or reallocated, and it being a no-op to "free" the video surface.
There is nothing that says a SDL 1.2 surface must always wrap the same SDL 2 surface, so we can maybe accommodate those assumptions by having the video surface be a special-cased global pointer that is never freed, and only reallocate its contents?
This seems to work with libsdl-perl's
core_video.t
test, but I haven't tested it extensively; consider it a proof-of-concept for now.Factor out Surface20to12InPlace
This is a step towards making it possible to use the same SDL12_Surface for the video surface at all times, and only reallocating its underlying SDL 2 SDL_Surface.
Factor out FreeSurfaceContents, and handle surface20 more defensively
This is a step towards making it possible to use the same SDL12_Surface for the video surface at all times, and only reallocating its underlying SDL 2 SDL_Surface.
Factor out enough of SDL_CreateRGBSurface to create surfaces in-place
The part before we create the SDL 1.2 surface (creating the SDL 2.0 surface) becomes CreateRGBSurface(), and the part after becomes Surface12SetMasks().
Allocate video surface object statically as a global
The SDL Perl bindings incorrectly call SDL_FreeSurface() on the result of functions that return a "borrowed" pointer to the video surface, namely SDL_SetVideoMode() and SDL_GetVideoSurface(). (See https://github.com/PerlGameDev/SDL/issues/305)
When we would previously have allocated or freed the video surface wrapper object, instead allocate or free its contents in-place.
When checking whether the video surface exists, because we never destroy it, we must now also check whether its underlying SDL2 video surface exists.
Resolves: https://github.com/libsdl-org/sdl12-compat/issues/305