Some more refactoring based on advice from clang-tidy.

- Eliminate virtual calls in c'tor/d'tor
- Eliminate some redundant calls in d'tor
- Don't reinitialize blitter unless absolutely necessary
This commit is contained in:
Stephen Anthony 2019-12-22 11:56:24 -03:30
parent 6ec46b9836
commit 099bff3d26
5 changed files with 60 additions and 37 deletions

View File

@ -17,6 +17,7 @@
#include "FBSurfaceSDL2.hxx" #include "FBSurfaceSDL2.hxx"
#include "Logger.hxx"
#include "ThreadDebugging.hxx" #include "ThreadDebugging.hxx"
#include "sdl_blitter/BlitterFactory.hxx" #include "sdl_blitter/BlitterFactory.hxx"
@ -43,7 +44,7 @@ namespace {
FBSurfaceSDL2::FBSurfaceSDL2(FrameBufferSDL2& buffer, FBSurfaceSDL2::FBSurfaceSDL2(FrameBufferSDL2& buffer,
uInt32 width, uInt32 height, uInt32 width, uInt32 height,
FrameBuffer::ScalingInterpolation interpolation, FrameBuffer::ScalingInterpolation interpolation,
const uInt32* data) const uInt32* staticData)
: myFB(buffer), : myFB(buffer),
myInterpolationMode(interpolation), myInterpolationMode(interpolation),
mySurface(nullptr), mySurface(nullptr),
@ -54,7 +55,7 @@ FBSurfaceSDL2::FBSurfaceSDL2(FrameBufferSDL2& buffer,
mySrcGUIR({0, 0, 0, 0}), mySrcGUIR({0, 0, 0, 0}),
myDstGUIR({0, 0, 0, 0}) myDstGUIR({0, 0, 0, 0})
{ {
createSurface(width, height, data); createSurface(width, height, staticData);
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -67,8 +68,6 @@ FBSurfaceSDL2::~FBSurfaceSDL2()
SDL_FreeSurface(mySurface); SDL_FreeSurface(mySurface);
mySurface = nullptr; mySurface = nullptr;
} }
free();
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -112,37 +111,41 @@ const Common::Rect& FBSurfaceSDL2::dstRect() const
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void FBSurfaceSDL2::setSrcPos(uInt32 x, uInt32 y) void FBSurfaceSDL2::setSrcPos(uInt32 x, uInt32 y)
{ {
mySrcR.x = x; mySrcR.y = y; if(x != static_cast<uInt32>(mySrcR.x) || y != static_cast<uInt32>(mySrcR.y))
mySrcGUIR.moveTo(x, y); {
setSrcPosInternal(x, y);
reinitializeBlitter(); reinitializeBlitter();
}
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void FBSurfaceSDL2::setSrcSize(uInt32 w, uInt32 h) void FBSurfaceSDL2::setSrcSize(uInt32 w, uInt32 h)
{ {
mySrcR.w = w; mySrcR.h = h; if(w != static_cast<uInt32>(mySrcR.w) || h != static_cast<uInt32>(mySrcR.h))
mySrcGUIR.setWidth(w); mySrcGUIR.setHeight(h); {
setSrcSizeInternal(w, h);
reinitializeBlitter(); reinitializeBlitter();
}
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void FBSurfaceSDL2::setDstPos(uInt32 x, uInt32 y) void FBSurfaceSDL2::setDstPos(uInt32 x, uInt32 y)
{ {
myDstR.x = x; myDstR.y = y; if(x != static_cast<uInt32>(myDstR.x) || y != static_cast<uInt32>(myDstR.y))
myDstGUIR.moveTo(x, y); {
setDstPosInternal(x, y);
reinitializeBlitter(); reinitializeBlitter();
}
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void FBSurfaceSDL2::setDstSize(uInt32 w, uInt32 h) void FBSurfaceSDL2::setDstSize(uInt32 w, uInt32 h)
{ {
myDstR.w = w; myDstR.h = h; if(w != static_cast<uInt32>(myDstR.w) || h != static_cast<uInt32>(myDstR.h))
myDstGUIR.setWidth(w); myDstGUIR.setHeight(h); {
setDstSizeInternal(w, h);
reinitializeBlitter(); reinitializeBlitter();
}
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -183,7 +186,7 @@ void FBSurfaceSDL2::invalidate()
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void FBSurfaceSDL2::free() void FBSurfaceSDL2::free()
{ {
myBlitter.reset(nullptr); myBlitter.reset();
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -203,6 +206,11 @@ void FBSurfaceSDL2::resize(uInt32 width, uInt32 height)
free(); free();
// NOTE: Currently, a resize changes a 'static' surface to 'streaming'
// No code currently does this, but we should at least check for it
if(myIsStatic)
Logger::error("Resizing static texture!");
createSurface(width, height, nullptr); createSurface(width, height, nullptr);
} }
@ -220,10 +228,10 @@ void FBSurfaceSDL2::createSurface(uInt32 width, uInt32 height,
// We start out with the src and dst rectangles containing the same // We start out with the src and dst rectangles containing the same
// dimensions, indicating no scaling or re-positioning // dimensions, indicating no scaling or re-positioning
setSrcPos(0, 0); setSrcPosInternal(0, 0);
setDstPos(0, 0); setDstPosInternal(0, 0);
setSrcSize(width, height); setSrcSizeInternal(width, height);
setDstSize(width, height); setDstSizeInternal(width, height);
//////////////////////////////////////////////////// ////////////////////////////////////////////////////
// These *must* be set for the parent class // These *must* be set for the parent class
@ -231,11 +239,9 @@ void FBSurfaceSDL2::createSurface(uInt32 width, uInt32 height,
myPitch = mySurface->pitch / pf.BytesPerPixel; myPitch = mySurface->pitch / pf.BytesPerPixel;
//////////////////////////////////////////////////// ////////////////////////////////////////////////////
if(data) myIsStatic = data != nullptr;
{ if(myIsStatic)
myIsStatic = true;
SDL_memcpy(mySurface->pixels, data, mySurface->w * mySurface->h * 4); SDL_memcpy(mySurface->pixels, data, mySurface->w * mySurface->h * 4);
}
reinitializeBlitter(); reinitializeBlitter();
} }
@ -243,9 +249,11 @@ void FBSurfaceSDL2::createSurface(uInt32 width, uInt32 height,
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void FBSurfaceSDL2::reinitializeBlitter() void FBSurfaceSDL2::reinitializeBlitter()
{ {
if (!myBlitter && myFB.isInitialized()) myBlitter = BlitterFactory::createBlitter(myFB, scalingAlgorithm(myInterpolationMode)); if (!myBlitter && myFB.isInitialized())
myBlitter = BlitterFactory::createBlitter(myFB, scalingAlgorithm(myInterpolationMode));
if (myBlitter) myBlitter->reinitialize(mySrcR, myDstR, myAttributes, myIsStatic ? mySurface : nullptr); if (myBlitter)
myBlitter->reinitialize(mySrcR, myDstR, myAttributes, myIsStatic ? mySurface : nullptr);
} }
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

View File

@ -66,6 +66,23 @@ class FBSurfaceSDL2 : public FBSurface
void applyAttributes() override; void applyAttributes() override;
private: private:
inline void setSrcPosInternal(uInt32 x, uInt32 y) {
mySrcR.x = x; mySrcR.y = y;
mySrcGUIR.moveTo(x, y);
}
inline void setSrcSizeInternal(uInt32 w, uInt32 h) {
mySrcR.w = w; mySrcR.h = h;
mySrcGUIR.setWidth(w); mySrcGUIR.setHeight(h);
}
inline void setDstPosInternal(uInt32 x, uInt32 y) {
myDstR.x = x; myDstR.y = y;
myDstGUIR.moveTo(x, y);
}
inline void setDstSizeInternal(uInt32 w, uInt32 h) {
myDstR.w = w; myDstR.h = h;
myDstGUIR.setWidth(w); myDstGUIR.setHeight(h);
}
void createSurface(uInt32 width, uInt32 height, const uInt32* data); void createSurface(uInt32 width, uInt32 height, const uInt32* data);
void reinitializeBlitter(); void reinitializeBlitter();

View File

@ -39,8 +39,6 @@ class BilinearBlitter : public Blitter {
virtual void blit(SDL_Surface& surface) override; virtual void blit(SDL_Surface& surface) override;
virtual void free() override;
private: private:
SDL_Texture* myTexture; SDL_Texture* myTexture;
@ -58,6 +56,8 @@ class BilinearBlitter : public Blitter {
private: private:
void free();
void recreateTexturesIfNecessary(); void recreateTexturesIfNecessary();
private: private:

View File

@ -37,8 +37,6 @@ class Blitter {
virtual void blit(SDL_Surface& surface) = 0; virtual void blit(SDL_Surface& surface) = 0;
virtual void free() = 0;
protected: protected:
Blitter() = default; Blitter() = default;

View File

@ -41,8 +41,6 @@ class QisBlitter : public Blitter {
virtual void blit(SDL_Surface& surface) override; virtual void blit(SDL_Surface& surface) override;
virtual void free() override;
private: private:
SDL_Texture* mySrcTexture; SDL_Texture* mySrcTexture;
@ -62,6 +60,8 @@ class QisBlitter : public Blitter {
private: private:
void free();
void recreateTexturesIfNecessary(); void recreateTexturesIfNecessary();
void blitToIntermediate(); void blitToIntermediate();