Skip to content

Commit

Permalink
SDLSurfaceSprite2D: consolidate SetSurfaceRLE calls into a private fu…
Browse files Browse the repository at this point in the history
…nction

this is hopefully compatible with SDL 1.2
  • Loading branch information
bradallred committed Jun 20, 2013
1 parent 43aa7fd commit 1ede211
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 9 deletions.
2 changes: 2 additions & 0 deletions gemrb/core/Sprite2D.cpp
Expand Up @@ -32,6 +32,7 @@ Sprite2D::Sprite2D(int Width, int Height, int Bpp, const void* pixels)
: Width(Width), Height(Height), Bpp(Bpp), pixels(pixels) : Width(Width), Height(Height), Bpp(Bpp), pixels(pixels)
{ {
BAM = false; BAM = false;
RLE = false;
XPos = 0; XPos = 0;
YPos = 0; YPos = 0;
RefCount = 1; RefCount = 1;
Expand All @@ -41,6 +42,7 @@ Sprite2D::Sprite2D(int Width, int Height, int Bpp, const void* pixels)
Sprite2D::Sprite2D(const Sprite2D &obj) Sprite2D::Sprite2D(const Sprite2D &obj)
{ {
BAM = false; BAM = false;
RLE = false;
RefCount = 1; RefCount = 1;


XPos = obj.XPos; XPos = obj.XPos;
Expand Down
31 changes: 22 additions & 9 deletions gemrb/plugins/SDLVideo/SDLSurfaceSprite2D.cpp
Expand Up @@ -32,9 +32,7 @@ SDLSurfaceSprite2D::SDLSurfaceSprite2D(int Width, int Height, int Bpp, void* pix
surface = SDL_CreateRGBSurfaceFrom( pixels, Width, Height, Bpp < 8 ? 8 : Bpp, Width * ( Bpp / 8 ), surface = SDL_CreateRGBSurfaceFrom( pixels, Width, Height, Bpp < 8 ? 8 : Bpp, Width * ( Bpp / 8 ),
0, 0, 0, 0 ); 0, 0, 0, 0 );
freePixels = true; freePixels = true;
#if SDL_VERSION_ATLEAST(1,3,0) SetSurfaceRLE(true);

This comment has been minimized.

Copy link
@wjp

wjp Jul 6, 2013

Member

It's not a good idea to always force SDL_RLEACCEL. First of all it is meaningless when there is no colourkey or per-pixel alpha, and second some sprites regularly need direct pixel data access (such as TIS tiles via BlitTile), and having to do an SDL_LockSurface each call will be too expensive.

This comment has been minimized.

Copy link
@wjp

wjp Jul 6, 2013

Member

Also note that a few of our internal blitters (BlitTile, BlitGameSprite) currently use surface->pixels without calling SDL_LockSurface.

SDL_SetSurfaceRLE(surface, SDL_TRUE);
#endif
} }


SDLSurfaceSprite2D::SDLSurfaceSprite2D (int Width, int Height, int Bpp, void* pixels, SDLSurfaceSprite2D::SDLSurfaceSprite2D (int Width, int Height, int Bpp, void* pixels,
Expand All @@ -44,9 +42,7 @@ SDLSurfaceSprite2D::SDLSurfaceSprite2D (int Width, int Height, int Bpp, void* pi
surface = SDL_CreateRGBSurfaceFrom( pixels, Width, Height, Bpp < 8 ? 8 : Bpp, Width * ( Bpp / 8 ), surface = SDL_CreateRGBSurfaceFrom( pixels, Width, Height, Bpp < 8 ? 8 : Bpp, Width * ( Bpp / 8 ),
rmask, gmask, bmask, amask ); rmask, gmask, bmask, amask );
freePixels = true; freePixels = true;
#if SDL_VERSION_ATLEAST(1,3,0) SetSurfaceRLE(true);
SDL_SetSurfaceRLE(surface, SDL_TRUE);
#endif
} }


SDLSurfaceSprite2D::SDLSurfaceSprite2D(const SDLSurfaceSprite2D &obj) SDLSurfaceSprite2D::SDLSurfaceSprite2D(const SDLSurfaceSprite2D &obj)
Expand All @@ -55,9 +51,7 @@ SDLSurfaceSprite2D::SDLSurfaceSprite2D(const SDLSurfaceSprite2D &obj)
surface = SDL_ConvertSurface(obj.surface, obj.surface->format, obj.surface->flags); surface = SDL_ConvertSurface(obj.surface, obj.surface->format, obj.surface->flags);
pixels = surface->pixels; pixels = surface->pixels;
freePixels = false; freePixels = false;
#if SDL_VERSION_ATLEAST(1,3,0) SetSurfaceRLE(obj.RLE);
SDL_SetSurfaceRLE(surface, SDL_TRUE);
#endif
} }


SDLSurfaceSprite2D* SDLSurfaceSprite2D::copy() const SDLSurfaceSprite2D* SDLSurfaceSprite2D::copy() const
Expand Down Expand Up @@ -114,6 +108,7 @@ ieDword SDLSurfaceSprite2D::GetColorKey() const
void SDLSurfaceSprite2D::SetColorKey(ieDword ck) void SDLSurfaceSprite2D::SetColorKey(ieDword ck)
{ {
#if SDL_VERSION_ATLEAST(1,3,0) #if SDL_VERSION_ATLEAST(1,3,0)
// SDL 2 will enforce SDL_RLEACCEL
SDL_SetColorKey(surface, SDL_TRUE, ck); SDL_SetColorKey(surface, SDL_TRUE, ck);
#else #else
SDL_SetColorKey(surface, SDL_SRCCOLORKEY | SDL_RLEACCEL, ck); SDL_SetColorKey(surface, SDL_SRCCOLORKEY | SDL_RLEACCEL, ck);
Expand Down Expand Up @@ -190,6 +185,24 @@ bool SDLSurfaceSprite2D::ConvertFormatTo(int bpp, ieDword rmask, ieDword gmask,
return false; return false;
} }


void SDLSurfaceSprite2D::SetSurfaceRLE(bool rle)
{
#if SDL_VERSION_ATLEAST(1,3,0)
SDL_SetSurfaceRLE(surface, rle);
#else
if (rle) {
surface->flags |= SDL_RLEACCEL;

This comment has been minimized.

Copy link
@wjp

wjp Jul 6, 2013

Member

I strongly doubt you're allowed to set this flag on an SDL_Surface yourself. Use SDL_SetAlpha or SDL_SetColorKey for that.

This comment has been minimized.

Copy link
@bradallred

bradallred Jul 6, 2013

Author Member

you are probably correct. IIRC i looked at the SDL source, but could have been SDL2 source.

} else {
surface->flags &= ~SDL_RLEACCEL;
}
#endif
// regardless of rle or the success of SDL_SetSurfaceRLE
// we must keep RLE false because SDL hides the actual RLE data from us (see SDL_BlitMap)
// and we are left to access the pixels in decoded form (updated by SDL_UnlockSurface).
// SDL Blits will make use of RLE acceleration, but our internal blitters cannot.
assert(RLE == false);
}

void SDLSurfaceSprite2D::SetSurfacePalette(SDL_Surface* surf, SDL_Color* pal, int numcolors) void SDLSurfaceSprite2D::SetSurfacePalette(SDL_Surface* surf, SDL_Color* pal, int numcolors)
{ {
#if SDL_VERSION_ATLEAST(1,3,0) #if SDL_VERSION_ATLEAST(1,3,0)
Expand Down
2 changes: 2 additions & 0 deletions gemrb/plugins/SDLVideo/SDLSurfaceSprite2D.h
Expand Up @@ -32,6 +32,8 @@ class SDLSurfaceSprite2D : public Sprite2D {
private: private:
bool freePixels; bool freePixels;
SDL_Surface* surface; SDL_Surface* surface;
private:
void SetSurfaceRLE(bool);
public: public:
SDLSurfaceSprite2D(int Width, int Height, int Bpp, void* pixels); SDLSurfaceSprite2D(int Width, int Height, int Bpp, void* pixels);
SDLSurfaceSprite2D(int Width, int Height, int Bpp, void* pixels, SDLSurfaceSprite2D(int Width, int Height, int Bpp, void* pixels,
Expand Down

0 comments on commit 1ede211

Please sign in to comment.