Fixes graphical artifacts on toggling stack queue visibility

Battlefield rendering now uses local coordinates

client/battle/BattleAnimationClasses.cpp

@@ -928,7 +928,7 @@ bool EffectAnimation::init()
 	{
 		for(int i=0; i * first->width() < owner.fieldController->pos.w ; ++i)
 			for(int j=0; j * first->height() < owner.fieldController->pos.h ; ++j)
-				positions.push_back(Point( owner.fieldController->pos.x + i * first->width(), owner.fieldController->pos.y + j * first->height()));
+				positions.push_back(Point( i * first->width(), j * first->height()));
 	}
 
 	BattleEffect be;
@@ -942,29 +942,29 @@ bool EffectAnimation::init()
 		bool hasPosition = i < positions.size();
 
 		if (hasTile && !forceOnTop())
-			be.position = battlehexes[i];
+			be.tile = battlehexes[i];
 		else
-			be.position = BattleHex::INVALID;
+			be.tile = BattleHex::INVALID;
 
 		if (hasPosition)
 		{
-			be.x = positions[i].x;
-			be.y = positions[i].y;
+			be.pos.x = positions[i].x;
+			be.pos.y = positions[i].y;
 		}
 		else
 		{
 			const CStack * destStack = owner.getCurrentPlayerInterface()->cb->battleGetStackByPos(battlehexes[i], false);
-			Rect tilePos = owner.fieldController->hexPositionAbsolute(battlehexes[i]);
+			Rect tilePos = owner.fieldController->hexPositionLocal(battlehexes[i]);
 
-			be.x = tilePos.x + tilePos.w/2 - first->width()/2;
+			be.pos.x = tilePos.x + tilePos.w/2 - first->width()/2;
 
 			if(destStack && destStack->doubleWide()) // Correction for 2-hex creatures.
-				be.x += (destStack->side == BattleSide::ATTACKER ? -1 : 1)*tilePos.w/2;
+				be.pos.x += (destStack->side == BattleSide::ATTACKER ? -1 : 1)*tilePos.w/2;
 
 			if (alignToBottom())
-				be.y = tilePos.y + tilePos.h - first->height();
+				be.pos.y = tilePos.y + tilePos.h - first->height();
 			else
-				be.y = tilePos.y - first->height()/2;
+				be.pos.y = tilePos.y - first->height()/2;
 		}
 		owner.effectsController->battleEffects.push_back(be);
 	}
@@ -1071,7 +1071,7 @@ void HeroCastAnimation::initializeProjectile()
 	// targeted spells should have well, target
 	assert(tile.isValid());
 
-	Point srccoord = hero->pos.center();
+	Point srccoord = hero->pos.center() - hero->parent->pos.topLeft();
 	Point destcoord = owner.stacksController->getStackPositionAtHex(tile, target); //position attacked by projectile
 
 	destcoord += Point(222, 265); // FIXME: what are these constants?

client/battle/BattleEffectsController.cpp

@@ -126,14 +126,14 @@ void BattleEffectsController::collectRenderableObjects(BattleRenderer & renderer
 {
 	for (auto & elem : battleEffects)
 	{
-		renderer.insert( EBattleFieldLayer::EFFECTS, elem.position, [&elem](BattleRenderer::RendererRef canvas)
+		renderer.insert( EBattleFieldLayer::EFFECTS, elem.tile, [&elem](BattleRenderer::RendererRef canvas)
 		{
 			int currentFrame = static_cast<int>(floor(elem.currentFrame));
 			currentFrame %= elem.animation->size();
 
 			auto img = elem.animation->getImage(currentFrame);
 
-			canvas.draw(img, Point(elem.x, elem.y));
+			canvas.draw(img, elem.pos);
 		});
 	}
 }

client/battle/BattleEffectsController.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "../../lib/battle/BattleHex.h"
+#include "../gui/Geometries.h"
 #include "BattleConstants.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -29,11 +30,11 @@ class EffectAnimation;
 /// Struct for battle effect animation e.g. morale, prayer, armageddon, bless,...
 struct BattleEffect
 {
-	int x, y; //position on the screen
+	Point pos; //position on the screen
 	float currentFrame;
 	std::shared_ptr<CAnimation> animation;
 	int effectID; //uniqueID equal ot ID of appropriate CSpellEffectAnim
-	BattleHex position; //Indicates if effect which hex the effect is drawn on
+	BattleHex tile; //Indicates if effect which hex the effect is drawn on
 };
 
 /// Controls rendering of effects in battle, e.g. from spells, abilities and various other actions like morale

client/battle/BattleFieldController.cpp

@@ -115,13 +115,15 @@ void BattleFieldController::mouseMoved(const SDL_MouseMotionEvent &event)
 
 void BattleFieldController::renderBattlefield(Canvas & canvas)
 {
-	showBackground(canvas);
+	Canvas clippedCanvas(canvas, pos);
+
+	showBackground(clippedCanvas);
 
 	BattleRenderer renderer(owner);
 
-	renderer.execute(canvas);
+	renderer.execute(clippedCanvas);
 
-	owner.projectilesController->showProjectiles(canvas);
+	owner.projectilesController->showProjectiles(clippedCanvas);
 }
 
 void BattleFieldController::showBackground(Canvas & canvas)
@@ -137,19 +139,19 @@ void BattleFieldController::showBackground(Canvas & canvas)
 
 void BattleFieldController::showBackgroundImage(Canvas & canvas)
 {
-	canvas.draw(background, pos.topLeft());
+	canvas.draw(background, Point(0, 0));
 
-	owner.obstacleController->showAbsoluteObstacles(canvas, pos.topLeft());
+	owner.obstacleController->showAbsoluteObstacles(canvas);
 	if ( owner.siegeController )
-		owner.siegeController->showAbsoluteObstacles(canvas, pos.topLeft());
+		owner.siegeController->showAbsoluteObstacles(canvas);
 
 	if (settings["battle"]["cellBorders"].Bool())
-		canvas.draw(*cellBorders, pos.topLeft());
+		canvas.draw(*cellBorders, Point(0, 0));
 }
 
 void BattleFieldController::showBackgroundImageWithHexes(Canvas & canvas)
 {
-	canvas.draw(*backgroundWithHexes.get(), pos.topLeft());
+	canvas.draw(*backgroundWithHexes.get(), Point(0, 0));
 }
 
 void BattleFieldController::redrawBackgroundWithHexes()
@@ -166,9 +168,9 @@ void BattleFieldController::redrawBackgroundWithHexes()
 
 	//prepare background graphic with hexes and shaded hexes
 	backgroundWithHexes->draw(background, Point(0,0));
-	owner.obstacleController->showAbsoluteObstacles(*backgroundWithHexes, Point(0,0));
+	owner.obstacleController->showAbsoluteObstacles(*backgroundWithHexes);
 	if ( owner.siegeController )
-		owner.siegeController->showAbsoluteObstacles(*backgroundWithHexes, Point(0,0));
+		owner.siegeController->showAbsoluteObstacles(*backgroundWithHexes);
 
 	if (settings["battle"]["stackRange"].Bool())
 	{
@@ -186,7 +188,7 @@ void BattleFieldController::redrawBackgroundWithHexes()
 
 void BattleFieldController::showHighlightedHex(Canvas & canvas, BattleHex hex, bool darkBorder)
 {
-	Point hexPos = hexPositionAbsolute(hex).topLeft();
+	Point hexPos = hexPositionLocal(hex).topLeft();
 
 	canvas.draw(cellShade, hexPos);
 	if(!darkBorder && settings["battle"]["cellBorders"].Bool())
@@ -548,11 +550,7 @@ void BattleFieldController::show(SDL_Surface * to)
 	owner.stacksController->update();
 	owner.obstacleController->update();
 
-	SDL_Rect buf;
-
 	Canvas canvas(to);
-	SDL_GetClipRect(to, &buf);
-	SDL_SetClipRect(to, &pos);
+
 	renderBattlefield(canvas);
-	SDL_SetClipRect(to, &buf); //restoring previous clip_rect
 }

client/battle/BattleInterface.cpp

@@ -418,8 +418,8 @@ void BattleInterface::spellCast(const BattleSpellCast * sc)
 	//mana absorption
 	if (sc->manaGained > 0)
 	{
-		Point leftHero = Point(15, 30) + fieldController->pos;
-		Point rightHero = Point(755, 30) + fieldController->pos;
+		Point leftHero = Point(15, 30);
+		Point rightHero = Point(755, 30);
 		bool side = sc->side;
 
 		executeOnAnimationCondition(EAnimationEvents::AFTER_HIT, true, [=](){

client/battle/BattleInterfaceClasses.cpp

@@ -186,8 +186,8 @@ void BattleHero::render(Canvas & canvas)
 	auto flagFrame = flagAnimation->getImage(flagCurrentFrame, 0, true);
 	auto heroFrame = animation->getImage(currentFrame, groupIndex, true);
 
-	Point heroPosition = pos.center() - heroFrame->dimensions() / 2;
-	Point flagPosition = pos.center() - flagFrame->dimensions() / 2;
+	Point heroPosition = pos.center() - parent->pos.topLeft() - heroFrame->dimensions() / 2;
+	Point flagPosition = pos.center() - parent->pos.topLeft() - flagFrame->dimensions() / 2;
 
 	if(defender)
 		flagPosition += Point(-4, -41);

client/battle/BattleObstacleController.cpp

@@ -107,7 +107,7 @@ void BattleObstacleController::obstaclePlaced(const std::vector<std::shared_ptr<
 	}
 }
 
-void BattleObstacleController::showAbsoluteObstacles(Canvas & canvas, const Point & offset)
+void BattleObstacleController::showAbsoluteObstacles(Canvas & canvas)
 {
 	//Blit absolute obstacles
 	for(auto & oi : owner.curInt->cb->battleGetAllObstacles())
@@ -116,7 +116,7 @@ void BattleObstacleController::showAbsoluteObstacles(Canvas & canvas, const Poin
 		{
 			auto img = getObstacleImage(*oi);
 			if(img)
-				canvas.draw(img, Point(offset.x + oi->getInfo().width, offset.y + oi->getInfo().height));
+				canvas.draw(img, Point(oi->getInfo().width, oi->getInfo().height));
 		}
 	}
 }
@@ -171,7 +171,7 @@ Point BattleObstacleController::getObstaclePosition(std::shared_ptr<IImage> imag
 {
 	int offset = obstacle.getAnimationYOffset(image->height());
 
-	Rect r = owner.fieldController->hexPositionAbsolute(obstacle.pos);
+	Rect r = owner.fieldController->hexPositionLocal(obstacle.pos);
 	r.y += 42 - image->height() + offset;
 
 	return r.topLeft();

client/battle/BattleObstacleController.h

@@ -53,7 +53,7 @@ public:
 	void obstaclePlaced(const std::vector<std::shared_ptr<const CObstacleInstance>> & oi);
 
 	/// renders all "absolute" obstacles
-	void showAbsoluteObstacles(Canvas & canvas, const Point & offset);
+	void showAbsoluteObstacles(Canvas & canvas);
 
 	/// adds all non-"absolute" visible obstacles for rendering
 	void collectRenderableObjects(BattleRenderer & renderer);

client/battle/BattleSiegeController.cpp

@@ -106,13 +106,13 @@ std::string BattleSiegeController::getWallPieceImageName(EWallVisual::EWallVisua
 	}
 }
 
-void BattleSiegeController::showWallPiece(Canvas & canvas, EWallVisual::EWallVisual what, const Point & offset)
+void BattleSiegeController::showWallPiece(Canvas & canvas, EWallVisual::EWallVisual what)
 {
 	auto & ci = town->town->clientInfo;
 	auto const & pos = ci.siegePositions[what];
 
 	if ( wallPieceImages[what])
-		canvas.draw(wallPieceImages[what], offset + Point(pos.x, pos.y));
+		canvas.draw(wallPieceImages[what], Point(pos.x, pos.y));
 }
 
 std::string BattleSiegeController::getBattleBackgroundName() const
@@ -205,10 +205,10 @@ Point BattleSiegeController::getTurretCreaturePosition( BattleHex position ) con
 
 	if (posID != 0)
 	{
-		Point result = owner.fieldController->pos.topLeft();
-		result.x += town->town->clientInfo.siegePositions[posID].x;
-		result.y += town->town->clientInfo.siegePositions[posID].y;
-		return result;
+		return {
+			town->town->clientInfo.siegePositions[posID].x,
+			town->town->clientInfo.siegePositions[posID].y
+		};
 	}
 
 	assert(0);
@@ -249,13 +249,13 @@ void BattleSiegeController::gateStateChanged(const EGateState state)
 		CCS->soundh->playSound(soundBase::DRAWBRG);
 }
 
-void BattleSiegeController::showAbsoluteObstacles(Canvas & canvas, const Point & offset)
+void BattleSiegeController::showAbsoluteObstacles(Canvas & canvas)
 {
 	if (getWallPieceExistance(EWallVisual::MOAT))
-		showWallPiece(canvas, EWallVisual::MOAT, offset);
+		showWallPiece(canvas, EWallVisual::MOAT);
 
 	if (getWallPieceExistance(EWallVisual::MOAT_BANK))
-		showWallPiece(canvas, EWallVisual::MOAT_BANK, offset);
+		showWallPiece(canvas, EWallVisual::MOAT_BANK);
 }
 
 BattleHex BattleSiegeController::getTurretBattleHex(EWallVisual::EWallVisual wallPiece) const
@@ -301,11 +301,11 @@ void BattleSiegeController::collectRenderableObjects(BattleRenderer & renderer)
 				owner.stacksController->showStack(canvas, getTurretStack(wallPiece));
 			});
 			renderer.insert( EBattleFieldLayer::BATTLEMENTS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererRef canvas){
-				showWallPiece(canvas, wallPiece, owner.fieldController->pos.topLeft());
+				showWallPiece(canvas, wallPiece);
 			});
 		}
 		renderer.insert( EBattleFieldLayer::WALLS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererRef canvas){
-			showWallPiece(canvas, wallPiece, owner.fieldController->pos.topLeft());
+			showWallPiece(canvas, wallPiece);
 		});
 
 

client/battle/BattleSiegeController.h

@@ -84,7 +84,7 @@ class BattleSiegeController
 	/// returns true if chosen wall piece should be present in current battle
 	bool getWallPieceExistance(EWallVisual::EWallVisual what) const;
 
-	void showWallPiece(Canvas & canvas, EWallVisual::EWallVisual what, const Point & offset);
+	void showWallPiece(Canvas & canvas, EWallVisual::EWallVisual what);
 
 	BattleHex getTurretBattleHex(EWallVisual::EWallVisual wallPiece) const;
 	const CStack * getTurretStack(EWallVisual::EWallVisual wallPiece) const;
@@ -97,7 +97,7 @@ public:
 	void stackIsCatapulting(const CatapultAttack & ca);
 
 	/// call-ins from other battle controllers
-	void showAbsoluteObstacles(Canvas & canvas, const Point & offset);
+	void showAbsoluteObstacles(Canvas & canvas);
 	void collectRenderableObjects(BattleRenderer & renderer);
 
 	/// queries from other battle controllers

client/battle/BattleStacksController.cpp

@@ -825,7 +825,7 @@ Point BattleStacksController::getStackPositionAtHex(BattleHex hexNum, const CSta
 		}
 	}
 	//returning
-	return ret + owner.fieldController->pos.topLeft();
+	return ret;
 }
 
 void BattleStacksController::setStackColorFilter(const ColorFilter & effect, const CStack * target, const CSpell * source, bool persistent)

client/gui/Canvas.cpp

@@ -17,24 +17,42 @@
 #include "../Graphics.h"
 
 Canvas::Canvas(SDL_Surface * surface):
-	surface(surface)
+	surface(surface),
+	renderOffset(0,0)
 {
 	surface->refcount++;
 }
 
 Canvas::Canvas(Canvas & other):
-	surface(other.surface)
+	surface(other.surface),
+	renderOffset(other.renderOffset)
 {
 	surface->refcount++;
 }
 
-Canvas::Canvas(const Point & size)
+Canvas::Canvas(Canvas & other, const Rect & newClipRect):
+	Canvas(other)
+{
+	clipRect.emplace();
+	SDL_GetClipRect(surface, clipRect.get_ptr());
+
+	Rect currClipRect = newClipRect + renderOffset;
+	SDL_SetClipRect(surface, &currClipRect);
+
+	renderOffset += newClipRect.topLeft();
+}
+
+Canvas::Canvas(const Point & size):
+	renderOffset(0,0)
 {
 	surface = CSDL_Ext::newSurface(size.x, size.y);
 }
 
 Canvas::~Canvas()
 {
+	if (clipRect)
+		SDL_SetClipRect(surface, clipRect.get_ptr());
+
 	SDL_FreeSurface(surface);
 }
 
@@ -42,40 +60,40 @@ void Canvas::draw(std::shared_ptr<IImage> image, const Point & pos)
 {
 	assert(image);
 	if (image)
-		image->draw(surface, pos.x, pos.y);
+		image->draw(surface, renderOffset.x + pos.x, renderOffset.y + pos.y);
 }
 
 void Canvas::draw(std::shared_ptr<IImage> image, const Point & pos, const Rect & sourceRect)
 {
 	assert(image);
 	if (image)
-		image->draw(surface, pos.x, pos.y, &sourceRect);
+		image->draw(surface, renderOffset.x + pos.x, renderOffset.y + pos.y, &sourceRect);
 }
 
 void Canvas::draw(std::shared_ptr<IImage> image, const Point & pos, const Rect & sourceRect, uint8_t alpha)
 {
 	assert(image);
 	if (image)
-		image->draw(surface, pos.x, pos.y, &sourceRect, alpha);
+		image->draw(surface, renderOffset.x + pos.x, renderOffset.y + pos.y, &sourceRect, alpha);
 }
 
 void Canvas::draw(Canvas & image, const Point & pos)
 {
-	blitAt(image.surface, pos.x, pos.y, surface);
+	blitAt(image.surface, renderOffset.x + pos.x, renderOffset.y + pos.y, surface);
 }
 
 void Canvas::drawLine(const Point & from, const Point & dest, const SDL_Color & colorFrom, const SDL_Color & colorDest)
 {
-	CSDL_Ext::drawLine(surface, from.x, from.y, dest.x, dest.y, colorFrom, colorDest);
+	CSDL_Ext::drawLine(surface, renderOffset.x + from.x, renderOffset.y + from.y, renderOffset.x + dest.x, renderOffset.y + dest.y, colorFrom, colorDest);
 }
 
 void Canvas::drawText(const Point & position, const EFonts & font, const SDL_Color & colorDest, ETextAlignment alignment, const std::string & text )
 {
 	switch (alignment)
 	{
-	case ETextAlignment::TOPLEFT:      return graphics->fonts[font]->renderTextLeft  (surface, text, colorDest, position);
-	case ETextAlignment::CENTER:       return graphics->fonts[font]->renderTextCenter(surface, text, colorDest, position);
-	case ETextAlignment::BOTTOMRIGHT:  return graphics->fonts[font]->renderTextRight (surface, text, colorDest, position);
+	case ETextAlignment::TOPLEFT:      return graphics->fonts[font]->renderTextLeft  (surface, text, colorDest, renderOffset + position);
+	case ETextAlignment::CENTER:       return graphics->fonts[font]->renderTextCenter(surface, text, colorDest, renderOffset + position);
+	case ETextAlignment::BOTTOMRIGHT:  return graphics->fonts[font]->renderTextRight (surface, text, colorDest, renderOffset + position);
 	}
 }
 
@@ -83,9 +101,9 @@ void Canvas::drawText(const Point & position, const EFonts & font, const SDL_Col
 {
 	switch (alignment)
 	{
-	case ETextAlignment::TOPLEFT:      return graphics->fonts[font]->renderTextLinesLeft  (surface, text, colorDest, position);
-	case ETextAlignment::CENTER:       return graphics->fonts[font]->renderTextLinesCenter(surface, text, colorDest, position);
-	case ETextAlignment::BOTTOMRIGHT:  return graphics->fonts[font]->renderTextLinesRight (surface, text, colorDest, position);
+	case ETextAlignment::TOPLEFT:      return graphics->fonts[font]->renderTextLinesLeft  (surface, text, colorDest, renderOffset + position);
+	case ETextAlignment::CENTER:       return graphics->fonts[font]->renderTextLinesCenter(surface, text, colorDest, renderOffset + position);
+	case ETextAlignment::BOTTOMRIGHT:  return graphics->fonts[font]->renderTextLinesRight (surface, text, colorDest, renderOffset + position);
 	}
 }
 

client/gui/Canvas.h

@@ -19,8 +19,15 @@ enum EFonts : int;
 /// Class that represents surface for drawing on
 class Canvas
 {
+	/// Target surface
 	SDL_Surface * surface;
 
+	/// Clip rect that was in use on surface originally and needs to be restored on destruction
+	boost::optional<Rect> clipRect;
+
+	/// Current rendering area offset, all rendering operations will be moved into selected area
+	Point renderOffset;
+
 	Canvas & operator = (Canvas & other) = delete;
 public:
 
@@ -30,6 +37,9 @@ public:
 	/// copy contructor
 	Canvas(Canvas & other);
 
+	/// creates canvas that only covers specified subsection of a surface
+	Canvas(Canvas & other, const Rect & clipRect);
+
 	/// constructs canvas of specified size
 	Canvas(const Point & size);