Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improved library performance by caching the internal image data, #9

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Improved library performance by caching the internal image data, #9

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad0f4d5
Improved library performance by caching the internal image data,
AquariusPower Mar 28, 2018
835a665
fixed to work as expected with inline modifier
AquariusPower Mar 28, 2018
8b9105e
adding flexibility to other compilers concerning some inline methods and
AquariusPower Apr 4, 2018
d2e4adc
libxbrz improving cache management
AquariusPower Apr 5, 2018
520ca47
libxbrz: fixed error messages.
AquariusPower Apr 5, 2018
580885c
libxbrzscale: improved inline define readability.
AquariusPower Apr 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 87 additions & 19 deletions libxbrzscale.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,35 @@

#include "libxbrzscale.h"

#include <cstdio>
#include <iostream>

#ifndef XBRZLIB_RELATIVEPATHSDL
#include <SDL2/SDL_endian.h>
#include <SDL2/SDL_error.h>
#include <SDL2/SDL_pixels.h>
#include <SDL2/SDL_surface.h>
#include <cstdio>
#else
#include "SDL_endian.h"
#include "SDL_error.h"
#include "SDL_pixels.h"
#include "SDL_surface.h"
#endif

#include "xbrz/xbrz.h"

//#include <cstdio>
//#include <cstdint>
//#include "SDL.h"
//#include "SDL_image.h"
//#include "xbrz/xbrz.h"

bool libxbrzscale::bEnableOutput=false;

bool libxbrzscale::bDbgMsg=false;

bool libxbrzscale::bUseCache=false;

bool libxbrzscale::bFreeInputSurfaceAfterScale=true;
bool libxbrzscale::bFreeOutputSurfaceAfterScale=true;

#ifndef XBRZLIB_NOINLINEGETSETPIX //this may be required to let some compillers linking actually work w/o error: "undefined reference to" these
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you check performance gains of using inline versus more aggressive compiler automatic optimizations? I'm not sure using inline these days is really useful - compilers are usually smarter that we are :)

if you really think it's needed, people usually do this:

#ifdef SOMETHING
#define XXX_INLINE inline
#else
#define XXX_INLINE
#endif

XXX_INLINE foo() {...}
XXX_INLINE bar() {...}

That way code is more readable. XXX should obviously be some library tag.

Copy link
Contributor Author

@AquariusPower AquariusPower Apr 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I actually just learned about inline.
I was just trying to keep things as they originally were (with inline).

I dont know how to actually test it, I guess some profiler would be required, and to disable dynamic CPU throttle, and also a stress test code, but I have only practice in profilling java using NetBeans (that has in integrated profiler that can hook on any java running application, so it is ready for profilling noobs like me :))

So, if you believe inline is unnecessary, I can just remove it xD.
but anyway, your alternative define is indeed much more readable!

what you think? we keep it? do you have practice on profilling cpp?

EDIT: I can't even debug ivan in eclipse yet shame :/ thats why I created this optional code that is a bit troubling to use but works very fine: https://github.com/AquariusPower/CppDebugMessages

inline
#endif
Uint32 libxbrzscale::SDL_GetPixel(SDL_Surface *surface, int x, int y)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since those functions don't touch any instance properties or methods, they don't need to be inside the class.

I think inline class methods should have implementation in the header if the class declaration is there. It is possible this caused IVAN build issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try it. I upload a commit and travisci compile ivanWin (that was failing cuz of inline on cpp file).

{
int bpp = surface->format->BytesPerPixel;
Expand Down Expand Up @@ -61,6 +74,9 @@ Uint32 libxbrzscale::SDL_GetPixel(SDL_Surface *surface, int x, int y)
}
}

#ifndef XBRZLIB_NOINLINEGETSETPIX //this may be required to let some compillers linking actually work w/o error: "undefined reference to" these
inline
#endif
void libxbrzscale::SDL_PutPixel(SDL_Surface *surface, int x, int y, Uint32 pixel)
{
int bpp = surface->format->BytesPerPixel;
Expand Down Expand Up @@ -96,6 +112,7 @@ void libxbrzscale::SDL_PutPixel(SDL_Surface *surface, int x, int y, Uint32 pixel


SDL_Surface* libxbrzscale::createARGBSurface(int w, int h) {
if(bDbgMsg)printf("Creating SDL RGB surface w=%d h=%d\n",w,h);
return SDL_CreateRGBSurface(0, w, h, 32, 0xff0000U, 0xff00U, 0xffU, 0xff000000U);
}

Expand Down Expand Up @@ -134,8 +151,37 @@ void displayImage(SDL_Surface* surface, const char* message) {
}
*/

uint32_t* libxbrzscale::surfaceToUint32(SDL_Surface* img){
uint32_t *data = new uint32_t[img->w * img->h];

unsigned long lUInt32ImgCacheSizeIN=0;
unsigned long lUInt32ImgCacheSizeOUT=0;
uint32_t* pUInt32ImgCacheIN=NULL;
uint32_t* pUInt32ImgCacheOUT=NULL;
void DeleteCache(bool bIn){
if(bIn){
delete [] pUInt32ImgCacheIN;
pUInt32ImgCacheIN=NULL;
lUInt32ImgCacheSizeIN=0;
}else{
delete [] pUInt32ImgCacheOUT;
pUInt32ImgCacheOUT=NULL;
lUInt32ImgCacheSizeOUT=0;
}
}
uint32_t* GetImgUint32Cache(bool bIn, unsigned long lSize){ //as that memory deletion wont happen promptly, it may consume too much memory too fast unnecessarily becoming a bottle neck on performance
if(lSize > (bIn?lUInt32ImgCacheSizeIN:lUInt32ImgCacheSizeOUT)){
if(libxbrzscale::isDbgMsg())std::cerr<<"libxBRZ: increasing "<<(bIn?"IN":"OUT")<<" image cache to "<<lSize<<std::endl;

if((bIn?pUInt32ImgCacheIN:pUInt32ImgCacheOUT)!=NULL)DeleteCache(bIn);

(bIn?lUInt32ImgCacheSizeIN:lUInt32ImgCacheSizeOUT)=lSize;
(bIn?pUInt32ImgCacheIN:pUInt32ImgCacheOUT)=new uint32_t[bIn?lUInt32ImgCacheSizeIN:lUInt32ImgCacheSizeOUT];
}

return (bIn?pUInt32ImgCacheIN:pUInt32ImgCacheOUT);
}

uint32_t* libxbrzscale::surfaceToUint32(bool bIn, SDL_Surface* img){
uint32_t *data = GetImgUint32Cache(bIn, img->w * img->h);

int x, y, offset=0;
Uint8 r, g, b, a;
Expand Down Expand Up @@ -180,29 +226,51 @@ void libxbrzscale::uint32toSurface(uint32_t* ui32src, SDL_Surface* dst_img){
}

SDL_Surface* libxbrzscale::scale(SDL_Surface* src_img, int scale){
return libxbrzscale::scale(NULL, src_img, scale);
}
/**
* dst_img if not null may be re-used, and is also returned
*/
SDL_Surface* libxbrzscale::scale(SDL_Surface* dst_imgCache, SDL_Surface* src_img, int scale){
if(scale<2 || scale>6){
fprintf(stderr, "invalid stretch value min=2, max=6, requested=%d\n", scale);
return NULL;
}

int src_width = src_img->w;
int src_height = src_img->h;
int dst_width = src_width * scale;
int dst_height = src_height * scale;

uint32_t *in_data = surfaceToUint32(src_img);
SDL_FreeSurface(src_img);
uint32_t *in_data = surfaceToUint32(true, src_img);
if(bFreeInputSurfaceAfterScale && src_img!=NULL && src_img->refcount>0){
SDL_FreeSurface(src_img); //previous INPUT surface
src_img=NULL; //just to prevent future troubles here, but pointless outside here
}

if(bEnableOutput)printf("Scaling image...\n");
uint32_t* dest = new uint32_t[dst_width * dst_height];
uint32_t* dest = GetImgUint32Cache(false, dst_width * dst_height);

xbrz::scale(scale, in_data, dest, src_width, src_height, xbrz::ColorFormat::ARGB);
delete [] in_data;
if(!bUseCache){DeleteCache(true);in_data=NULL;}

if(bEnableOutput)printf("Saving image...\n");
SDL_Surface* dst_img = createARGBSurface(dst_width, dst_height);
if (!dst_img) {
delete [] dest;
if(bEnableOutput)fprintf(stderr, "Failed to create SDL surface: %s\n", SDL_GetError());

if(dst_imgCache==NULL || dst_imgCache->w!=dst_width || dst_imgCache->h!=dst_height || dst_imgCache->refcount==0){
if(bFreeOutputSurfaceAfterScale && dst_imgCache!=NULL && dst_imgCache->refcount>0){
SDL_FreeSurface(dst_imgCache); //previous OUTPUT surface
}

dst_imgCache = createARGBSurface(dst_width, dst_height);
}

if (!dst_imgCache) {
if(!bUseCache){DeleteCache(false);dest=NULL;}
fprintf(stderr, "Failed to create SDL surface: %s\n", SDL_GetError()); //error messages must always output
return NULL;
}

uint32toSurface(dest,dst_img);
uint32toSurface(dest,dst_imgCache);

return dst_img;
return dst_imgCache;
}
24 changes: 20 additions & 4 deletions libxbrzscale.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,36 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#ifndef XBRZLIB_RELATIVEPATHSDL
#include <SDL2/SDL_stdinc.h>
#else
#include "SDL_stdinc.h"
#endif

struct SDL_Surface;

class libxbrzscale
{
public:
static inline Uint32 SDL_GetPixel(SDL_Surface *surface, int x, int y);
static inline void SDL_PutPixel(SDL_Surface *surface, int x, int y, Uint32 pixel);
static Uint32 SDL_GetPixel(SDL_Surface *surface, int x, int y);
static void SDL_PutPixel(SDL_Surface *surface, int x, int y, Uint32 pixel);
static SDL_Surface* createARGBSurface(int w, int h);
static SDL_Surface* scale(SDL_Surface* src_img,int scale);
static void setEnableOutput(bool b){bEnableOutput=true;};
static uint32_t* surfaceToUint32(SDL_Surface* img);
static SDL_Surface* scale(SDL_Surface* dst_imgCache,SDL_Surface* src_img,int scale);
static void setEnableOutput(bool b){bEnableOutput=b;};
static void setDebugMsg(bool b){bDbgMsg=b;};
static void setFreeSurfaceAfterScale(bool bInputSurface,bool bOutputSurface){
bFreeInputSurfaceAfterScale=bInputSurface;
bFreeOutputSurfaceAfterScale=bOutputSurface;
};
static void setUseCache(bool b){bUseCache=b;};
static uint32_t* surfaceToUint32(bool bIn, SDL_Surface* img);
static void uint32toSurface(uint32_t* dest, SDL_Surface* dst_img);
static bool isDbgMsg(){return bDbgMsg;}
private:
static bool bEnableOutput;
static bool bDbgMsg;
static bool bUseCache;
static bool bFreeInputSurfaceAfterScale;
static bool bFreeOutputSurfaceAfterScale;
};