Refactoring of CPicture class to improve encapsulation

client/lobby/CBonusSelection.cpp

@@ -492,8 +492,8 @@ CBonusSelection::CRegion::CRegion(int id, bool accessible, bool selectable, cons
 	graphicsSelected->disable();
 	graphicsStriped = std::make_shared<CPicture>(prefix + "Co" + suffix + ".BMP");
 	graphicsStriped->disable();
-	pos.w = graphicsNotSelected->bg->w;
-	pos.h = graphicsNotSelected->bg->h;
+	pos.w = graphicsNotSelected->pos.w;
+	pos.h = graphicsNotSelected->pos.h;
 
 }
 

client/lobby/CSelectionBase.cpp

@@ -312,7 +312,7 @@ CChatBox::CChatBox(const Rect & rect)
 	type |= REDRAW_PARENT;
 
 	const int height = static_cast<int>(graphics->fonts[FONT_SMALL]->getLineHeight());
-	inputBox = std::make_shared<CTextInput>(Rect(0, rect.h - height, rect.w, height));
+	inputBox = std::make_shared<CTextInput>(Rect(0, rect.h - height, rect.w, height), EFonts::FONT_SMALL, 0);
 	inputBox->removeUsedEvents(KEYBOARD);
 	chatHistory = std::make_shared<CTextBox>("", Rect(0, 0, rect.w, rect.h - height), 1);
 

client/mainmenu/CMainMenu.cpp

@@ -76,11 +76,7 @@ CMenuScreen::CMenuScreen(const JsonNode & configNode)
 
 	background = std::make_shared<CPicture>(config["background"].String());
 	if(config["scalable"].Bool())
-	{
-		if(background->bg->format->palette)
-			background->convertToScreenBPP();
 		background->scaleTo(Point(screen->w, screen->h));
-	}
 
 	pos = background->center();
 

client/widgets/AdventureMapClasses.cpp

@@ -182,7 +182,7 @@ CHeroList::CEmptyHeroItem::CEmptyHeroItem()
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 	movement = std::make_shared<CAnimImage>("IMOBIL", 0, 0, 0, 1);
-	portrait = std::make_shared<CPicture>("HPSXXX", movement->pos.w + 1);
+	portrait = std::make_shared<CPicture>("HPSXXX", movement->pos.w + 1, 0);
 	mana = std::make_shared<CAnimImage>("IMANA", 0, 0, movement->pos.w + portrait->pos.w + 2, 1 );
 
 	pos.w = mana->pos.w + mana->pos.x - pos.x;
@@ -213,7 +213,7 @@ void CHeroList::CHeroItem::update()
 
 std::shared_ptr<CIntObject> CHeroList::CHeroItem::genSelection()
 {
-	return std::make_shared<CPicture>("HPSYYY", movement->pos.w + 1);
+	return std::make_shared<CPicture>("HPSYYY", movement->pos.w + 1, 0);
 }
 
 void CHeroList::CHeroItem::select(bool on)
@@ -780,7 +780,7 @@ CInfoBar::VisibleComponentInfo::VisibleComponentInfo(const Component & compToDis
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 
-	background = std::make_shared<CPicture>("ADSTATOT", 1);
+	background = std::make_shared<CPicture>("ADSTATOT", 1, 0);
 
 	comp = std::make_shared<CComponent>(compToDisplay);
 	comp->moveTo(Point(pos.x+47, pos.y+50));

client/widgets/Images.cpp

@@ -35,24 +35,34 @@
 #include "../../lib/CGeneralTextHandler.h" //for Unicode related stuff
 #include "../../lib/CRandomGenerator.h"
 
-CPicture::CPicture( SDL_Surface *BG, int x, int y, bool Free )
+CPicture::CPicture(SDL_Surface *BG, const Point & position)
+	: bg(BG)
+	, visible(true)
+	, needRefresh(false)
 {
-	init();
-	bg = BG;
-	freeSurf = Free;
-	pos.x += x;
-	pos.y += y;
+	BG->refcount += 1;
+	pos += position;
 	pos.w = BG->w;
 	pos.h = BG->h;
 }
 
 CPicture::CPicture( const std::string &bmpname, int x, int y )
+	: CPicture(bmpname, Point(x,y))
+{}
+
+CPicture::CPicture( const std::string &bmpname )
+	: CPicture(bmpname, Point(0,0))
+{}
+
+CPicture::CPicture( const std::string &bmpname, const Point & position )
+	: bg(BitmapHandler::loadBitmap(bmpname))
+	, visible(true)
+	, needRefresh(false)
 {
-	init();
-	bg = BitmapHandler::loadBitmap(bmpname);
-	freeSurf = true;
-	pos.x += x;
-	pos.y += y;
+	pos.x += position.x;
+	pos.y += position.y;
+
+	assert(bg);
 	if(bg)
 	{
 		pos.w = bg->w;
@@ -64,29 +74,12 @@ CPicture::CPicture( const std::string &bmpname, int x, int y )
 	}
 }
 
-CPicture::CPicture(const Rect &r, const SDL_Color &color, bool screenFormat)
-{
-	init();
-	createSimpleRect(r, screenFormat, SDL_MapRGB(bg->format, color.r, color.g,color.b));
-}
-
-CPicture::CPicture(const Rect &r, ui32 color, bool screenFormat)
-{
-	init();
-	createSimpleRect(r, screenFormat, color);
-}
-
-CPicture::CPicture(SDL_Surface * BG, const Rect &SrcRect, int x, int y, bool free)
+CPicture::CPicture(SDL_Surface * BG, const Rect &SrcRect, int x, int y)
+	: CPicture(BG, Point(x,y))
 {
-	visible = true;
-	needRefresh = false;
-	srcRect = new Rect(SrcRect);
-	pos.x += x;
-	pos.y += y;
+	srcRect = SrcRect;
 	pos.w = srcRect->w;
 	pos.h = srcRect->h;
-	bg = BG;
-	freeSurf = free;
 }
 
 void CPicture::setSurface(SDL_Surface *to)
@@ -106,16 +99,7 @@ void CPicture::setSurface(SDL_Surface *to)
 
 CPicture::~CPicture()
 {
-	if(freeSurf)
-		SDL_FreeSurface(bg);
-	delete srcRect;
-}
-
-void CPicture::init()
-{
-	visible = true;
-	needRefresh = false;
-	srcRect = nullptr;
+	SDL_FreeSurface(bg);
 }
 
 void CPicture::show(SDL_Surface * to)
@@ -146,31 +130,16 @@ void CPicture::convertToScreenBPP()
 void CPicture::setAlpha(int value)
 {
 	CSDL_Ext::setAlpha (bg, value);
+	SDL_SetSurfaceBlendMode(bg,SDL_BLENDMODE_BLEND);
 }
 
 void CPicture::scaleTo(Point size)
 {
 	SDL_Surface * scaled = CSDL_Ext::scaleSurface(bg, size.x, size.y);
 
-	if(freeSurf)
-		SDL_FreeSurface(bg);
+	SDL_FreeSurface(bg);
 
 	setSurface(scaled);
-	freeSurf = false;
-}
-
-void CPicture::createSimpleRect(const Rect &r, bool screenFormat, ui32 color)
-{
-	pos += r.topLeft();
-	pos.w = r.w;
-	pos.h = r.h;
-	if(screenFormat)
-		bg = CSDL_Ext::newSurface(r.w, r.h);
-	else
-		bg = SDL_CreateRGBSurface(0, r.w, r.h, 8, 0, 0, 0, 0);
-
-	SDL_FillRect(bg, nullptr, color);
-	freeSurf = true;
 }
 
 void CPicture::colorize(PlayerColor player)

client/widgets/Images.h

@@ -27,11 +27,20 @@ class IImage;
 class CPicture : public CIntObject
 {
 	void setSurface(SDL_Surface *to);
-public:
+
 	SDL_Surface * bg;
-	Rect * srcRect; //if nullptr then whole surface will be used
-	bool freeSurf; //whether surface will be freed upon CPicture destruction
-	bool needRefresh;//Surface needs to be displayed each frame
+
+	void convertToScreenBPP();
+
+public:
+	/// if set, only specified section of internal image will be rendered
+	boost::optional<Rect> srcRect;
+
+	/// If set to true, iamge will be redrawn on each frame
+	bool needRefresh;
+
+	/// If set to false, image will not be rendered
+	/// Deprecated, use CIntObject::disable()/enable() instead
 	bool visible;
 
 	SDL_Surface * getSurface()
@@ -39,24 +48,28 @@ public:
 		return bg;
 	}
 
-	CPicture(const Rect & r, const SDL_Color & color, bool screenFormat = false); //rect filled with given color
-	CPicture(const Rect & r, ui32 color, bool screenFormat = false); //rect filled with given color
-	CPicture(SDL_Surface * BG, int x = 0, int y=0, bool Free = true); //wrap existing SDL_Surface
-	CPicture(const std::string &bmpname, int x=0, int y=0);
-	CPicture(SDL_Surface *BG, const Rect &SrcRext, int x = 0, int y = 0, bool free = false); //wrap subrect of given surface
+	/// wrap existing SDL_Surface
+	/// deprecated, do not use
+	CPicture(SDL_Surface * BG, const Point & position);
+
+	/// wrap section of existing SDL_Surface
+	CPicture(SDL_Surface *BG, const Rect &SrcRext, int x = 0, int y = 0); //wrap subrect of given surface
+
+	/// Loads image from specified file name
+	CPicture(const std::string & bmpname);
+	CPicture(const std::string & bmpname, const Point & position);
+	CPicture(const std::string & bmpname, int x, int y);
+
 	~CPicture();
-	void init();
 
-	//set alpha value for whole surface. Note: may be messed up if surface is shared
-	// 0=transparent, 255=opaque
+	/// set alpha value for whole surface. Note: may be messed up if surface is shared
+	/// 0=transparent, 255=opaque
 	void setAlpha(int value);
-
 	void scaleTo(Point size);
-	void createSimpleRect(const Rect &r, bool screenFormat, ui32 color);
+	void colorize(PlayerColor player);
+
 	void show(SDL_Surface * to) override;
 	void showAll(SDL_Surface * to) override;
-	void convertToScreenBPP();
-	void colorize(PlayerColor player);
 };
 
 /// area filled with specific texture

client/widgets/TextControls.cpp

@@ -415,7 +415,7 @@ CGStatusBar::CGStatusBar(int x, int y, std::string name, int maxw)
 	{
 		//execution of this block when maxw is incorrect breaks text centralization (issue #3151)
 		vstd::amin(pos.w, maxw);
-		background->srcRect = new Rect(0, 0, maxw, pos.h);
+		background->srcRect = Rect(0, 0, maxw, pos.h);
 	}
 	autoRedraw = false;
 }
@@ -502,12 +502,7 @@ CTextInput::CTextInput(const Rect & Pos, SDL_Surface * srf)
 	pos += Pos.topLeft();
 	captureAllKeys = true;
 	OBJ_CONSTRUCTION;
-	background = std::make_shared<CPicture>(Pos, 0, true);
-	Rect hlp = Pos;
-	if(srf)
-		CSDL_Ext::blitSurface(srf, hlp, background->getSurface(), Point(0,0));
-	else
-		SDL_FillRect(background->getSurface(), nullptr, 0);
+	background = std::make_shared<CPicture>(srf, Pos);
 	pos.w = background->pos.w;
 	pos.h = background->pos.h;
 	background->pos = pos;

client/widgets/TextControls.h

@@ -218,7 +218,7 @@ public:
 
 	CTextInput(const Rect & Pos, EFonts font, const CFunctionList<void(const std::string &)> & CB);
 	CTextInput(const Rect & Pos, const Point & bgOffset, const std::string & bgName, const CFunctionList<void(const std::string &)> & CB);
-	CTextInput(const Rect & Pos, SDL_Surface * srf = nullptr);
+	CTextInput(const Rect & Pos, SDL_Surface * srf);
 
 	void clickLeft(tribool down, bool previousState) override;
 	void keyPressed(const SDL_KeyboardEvent & key) override;

client/windows/CAdvmapInterface.cpp

@@ -478,8 +478,8 @@ CResDataBar::CResDataBar(const std::string & defname, int x, int y, int offx, in
 	background = std::make_shared<CPicture>(defname, 0, 0);
 	background->colorize(LOCPLINT->playerID);
 
-	pos.w = background->bg->w;
-	pos.h = background->bg->h;
+	pos.w = background->pos.w;
+	pos.h = background->pos.h;
 
 	txtpos.resize(8);
 	for (int i = 0; i < 8 ; i++)
@@ -502,8 +502,8 @@ CResDataBar::CResDataBar()
 	background = std::make_shared<CPicture>(ADVOPT.resdatabarG, 0, 0);
 	background->colorize(LOCPLINT->playerID);
 
-	pos.w = background->bg->w;
-	pos.h = background->bg->h;
+	pos.w = background->pos.w;
+	pos.h = background->pos.h;
 
 	txtpos.resize(8);
 	for (int i = 0; i < 8 ; i++)

client/windows/CCastleInterface.cpp

@@ -1168,7 +1168,7 @@ CCastleInterface::CCastleInterface(const CGTownInstance * Town, const CGTownInst
 	garr->addSplitBtn(split);
 
 	Rect barRect(9, 182, 732, 18);
-	auto statusbarBackground = std::make_shared<CPicture>(panel->getSurface(), barRect, 9, 555, false);
+	auto statusbarBackground = std::make_shared<CPicture>(panel->getSurface(), barRect, 9, 555);
 	statusbar = CGStatusBar::create(statusbarBackground);
 	resdatabar = std::make_shared<CResDataBar>("ARESBAR", 3, 575, 32, 2, 85, 85);
 
@@ -1365,7 +1365,7 @@ CHallInterface::CHallInterface(const CGTownInstance * Town):
 	resdatabar->moveBy(pos.topLeft(), true);
 	Rect barRect(5, 556, 740, 18);
 
-	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 5, 556, false);
+	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 5, 556);
 	statusbar = CGStatusBar::create(statusbarBackground);
 
 	title = std::make_shared<CLabel>(399, 12, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, town->town->buildings.at(BuildingID(town->hallLevel()+BuildingID::VILLAGE_HALL))->getNameTranslated());
@@ -1588,7 +1588,7 @@ CFortScreen::CFortScreen(const CGTownInstance * town):
 
 	Rect barRect(4, 554, 740, 18);
 
-	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 4, 554, false);
+	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 4, 554);
 	statusbar = CGStatusBar::create(statusbarBackground);
 }
 
@@ -1729,7 +1729,7 @@ CMageGuildScreen::CMageGuildScreen(CCastleInterface * owner,std::string imagem)
 
 	Rect barRect(7, 556, 737, 18);
 
-	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 7, 556, false);
+	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 7, 556);
 	statusbar = CGStatusBar::create(statusbarBackground);
 
 	exit = std::make_shared<CButton>(Point(748, 556), "TPMAGE1.DEF", CButton::tooltip(CGI->generaltexth->allTexts[593]), [&](){ close(); }, SDLK_RETURN);
@@ -1796,7 +1796,7 @@ CBlacksmithDialog::CBlacksmithDialog(bool possible, CreatureID creMachineID, Art
 
 	Rect barRect(8, pos.h - 26, pos.w - 16, 19);
 
-	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 8, pos.h - 26, false);
+	auto statusbarBackground = std::make_shared<CPicture>(background->getSurface(), barRect, 8, pos.h - 26);
 	statusbar = CGStatusBar::create(statusbarBackground);
 
 	animBG = std::make_shared<CPicture>("TPSMITBK", 64, 50);

client/windows/CSpellWindow.cpp

@@ -206,9 +206,9 @@ CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _m
 	temp_rect = CSDL_Ext::genRect(36, 56, 549 + pos.x, 330 + pos.y);
 	interactiveAreas.push_back(std::make_shared<InteractiveArea>(temp_rect, std::bind(&CSpellWindow::selectSchool, this, 4), 458, this));
 
-	temp_rect = CSDL_Ext::genRect(leftCorner->bg->h, leftCorner->bg->w, 97 + pos.x, 77 + pos.y);
+	temp_rect = CSDL_Ext::genRect(leftCorner->pos.h, leftCorner->pos.w, 97 + pos.x, 77 + pos.y);
 	interactiveAreas.push_back(std::make_shared<InteractiveArea>(temp_rect, std::bind(&CSpellWindow::fLcornerb, this), 450, this));
-	temp_rect = CSDL_Ext::genRect(rightCorner->bg->h, rightCorner->bg->w, 487 + pos.x, 72 + pos.y);
+	temp_rect = CSDL_Ext::genRect(rightCorner->pos.h, rightCorner->pos.w, 487 + pos.x, 72 + pos.y);
 	interactiveAreas.push_back(std::make_shared<InteractiveArea>(temp_rect, std::bind(&CSpellWindow::fRcornerb, this), 451, this));
 
 	//areas for spells

client/windows/CTradeWindow.cpp

@@ -679,7 +679,7 @@ CMarketplaceWindow::CMarketplaceWindow(const IMarket * Market, const CGHeroInsta
 
 			// create image that copies part of background containing slot MISC_1 into position of slot MISC_5
 			// this is workaround for bug in H3 files where this slot for ragdoll on this screen is missing
-			images.push_back(std::make_shared<CPicture>(background->bg, Rect(20, 187, 47, 47), 18, 339 ));
+			images.push_back(std::make_shared<CPicture>(background->getSurface(), Rect(20, 187, 47, 47), 18, 339 ));
 			sliderNeeded = false;
 			break;
 		default:

client/windows/CWindowObject.cpp

@@ -213,11 +213,14 @@ void CWindowObject::setShadow(bool on)
 		{
 			OBJECT_CONSTRUCTION_CUSTOM_CAPTURING(255-DISPOSE);
 
-			shadowParts.push_back(std::make_shared<CPicture>(shadowCorner, shadowPos.x, shadowPos.y));
-			shadowParts.push_back(std::make_shared<CPicture>(shadowRight, shadowPos.x, shadowStart.y));
-			shadowParts.push_back(std::make_shared<CPicture>(shadowBottom, shadowStart.x, shadowPos.y));
+			shadowParts.push_back(std::make_shared<CPicture>(shadowCorner, Point(shadowPos.x,   shadowPos.y)));
+			shadowParts.push_back(std::make_shared<CPicture>(shadowRight,  Point(shadowPos.x,   shadowStart.y)));
+			shadowParts.push_back(std::make_shared<CPicture>(shadowBottom, Point(shadowStart.x, shadowPos.y)));
 
 		}
+		SDL_FreeSurface(shadowCorner);
+		SDL_FreeSurface(shadowBottom);
+		SDL_FreeSurface(shadowRight);
 	}
 }
 

client/windows/GUIClasses.cpp

@@ -1208,7 +1208,7 @@ CExchangeWindow::CExchangeWindow(ObjectInstanceID hero1, ObjectInstanceID hero2,
 	questlogButton[1] = std::make_shared<CButton>(Point(740, qeLayout ? 39 : 44), "hsbtns4.def", CButton::tooltip(CGI->generaltexth->heroscrn[0]), std::bind(&CExchangeWindow::questlog, this, 1));
 
 	Rect barRect(5, 578, 725, 18);
-	statusbar = CGStatusBar::create(std::make_shared<CPicture>(background->getSurface(), barRect, 5, 578, false));
+	statusbar = CGStatusBar::create(std::make_shared<CPicture>(background->getSurface(), barRect, 5, 578));
 
 	//garrison interface
 
@@ -1368,7 +1368,6 @@ CPuzzleWindow::CPuzzleWindow(const int3 & GrailPos, double discoveredRatio)
 			piecesToRemove.push_back(piece);
 			piece->needRefresh = true;
 			piece->recActions = piece->recActions & ~SHOWALL;
-			SDL_SetSurfaceBlendMode(piece->bg,SDL_BLENDMODE_BLEND);
 		}
 		else
 		{