Probem Tiling an image with QPixmap

[LuckyBlade]

Hey guys,

I'm trying to cut an QPixmap image into 32x32 QPixmap tiles using the following function:

bool copyTileContainerFromImageFile( QString &fileString, QVector<Tile*> *sourceVector )
{
	if(fileString.isEmpty())
		return false;
 
	QRect rect(0,0,32,32);
	QPixmap originalImage(fileString);
	//goal = amount of possible tiles in this image.
	int goal = (originalImage.width() * originalImage.height()) / 1024; //1024 Pixels fits in one Tile
	sourceVector->reserve(goal); //reserves the amount of images in the vector.
	for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
 
		if(rect.x() + 32 >= originalImage.width()) {
			rect.setY(rect.y() + 32);
			rect.setX(0);
		}else
			rect.setX(rect.x() + 32);
 
	}
	return true;
}

bool copyTileContainerFromImageFile( QString &fileString, QVector<Tile*> *sourceVector )
{
	if(fileString.isEmpty())
		return false;

	QRect rect(0,0,32,32);
	QPixmap originalImage(fileString);
	//goal = amount of possible tiles in this image.
	int goal = (originalImage.width() * originalImage.height()) / 1024; //1024 Pixels fits in one Tile
	sourceVector->reserve(goal); //reserves the amount of images in the vector.
	for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
	
		if(rect.x() + 32 >= originalImage.width()) {
			rect.setY(rect.y() + 32);
			rect.setX(0);
		}else
			rect.setX(rect.x() + 32);

	}
	return true;
}

To copy to clipboard, switch view to plain text mode 

Am I just not seeing the woods for the trees and the function works incorrect or is it not possible to cut a QPixmap with the copy function?

[high_flyer]

and the function works incorrect

What is the result you expect, and what do you get?

You have some small errors:

for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
 
		if(rect.x() + 32 >= (originalImage.width() - 32)) { //otherwise go beyond the width - it would be better if you do that in the upper for: for(x=0; x<originalImage.width()-32; x+=32)
			rect.setY(rect.y() + 32); //here the same thing, rect.y() < originalImage.height() - 32
			rect.setX(0);
		}else
			rect.setX(rect.x() + 32);
 
	}

for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
	
		if(rect.x() + 32 >= (originalImage.width() - 32)) { //otherwise go beyond the width - it would be better if you do that in the upper for: for(x=0; x<originalImage.width()-32; x+=32)
			rect.setY(rect.y() + 32); //here the same thing, rect.y() < originalImage.height() - 32
			rect.setX(0);
		}else
			rect.setX(rect.x() + 32);

	}

To copy to clipboard, switch view to plain text mode 

[LuckyBlade]

What is the result you expect, and what do you get?

The result I expect should look like this:


The result I actually get looks like this:


The idea behind

if(rect.x() + 32 >= originalImage.width())

if(rect.x() + 32 >= originalImage.width())

To copy to clipboard, switch view to plain text mode 

is to check if the rect would go beyond the images width if a next tile gets cut out on the x asis.If it fails the x coordinate of the rect gets reset to 0 and the y coordinate gets increased by 32.

[high_flyer]

is to check if the rect would go beyond the images width if a next tile gets cut out on the x asis.

I am well aware of that.
But it allows rect.x() to be larger than originalImage.width()-32, which will cut a a rect which is beyond originalWidth().
So for any width which is not a multiple of 32, this will not work right.
You really should do something like:

for(int y=0; y<originalImage.height() - iRectHeight; y+=iRectHeight){
   for(int x=0; x<originalImage.width() -iRectWidth; x+=iRectWidth){
      rect.setTopLeft(QPos(x,y);
      sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
   }
}

for(int y=0; y<originalImage.height() - iRectHeight; y+=iRectHeight){
   for(int x=0; x<originalImage.width() -iRectWidth; x+=iRectWidth){
      rect.setTopLeft(QPos(x,y);
      sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
   }
}

To copy to clipboard, switch view to plain text mode 

But is just as side note.

Regarding the result you are getting, I'd say it has more to do with how you compose your result image than the way you cut it.
Can you show how you are composing your result image?

[LuckyBlade]

Ah okay that's a good point. Thanks for the hint.
I only tested the tiling with images of the size 128x128 or 512x512.

Here's the paintEvent() function:

for(int i = 0;i != curTileset->getTileContainer()->size();i++)
  painter.drawPixmap(*curTileset->getTileContainer()->at(i)->getPosition(),*curTileset->getTileContainer()->at(i)->getTileImage());

for(int i = 0;i != curTileset->getTileContainer()->size();i++)
  painter.drawPixmap(*curTileset->getTileContainer()->at(i)->getPosition(),*curTileset->getTileContainer()->at(i)->getTileImage());

To copy to clipboard, switch view to plain text mode 

getPosition returns a pointer to the rect (the rect is the rect you set as argument when creating a new Tile object)from the tile and getTileImage returns a pointer to the pixmap.

[high_flyer]

Can you show getPosition() and getTileImage()?

[LuckyBlade]

QPixmap * Tile::getTileImage()
{
	return &tileImage;
}

QPixmap * Tile::getTileImage()
{
	return &tileImage;
}

To copy to clipboard, switch view to plain text mode 

QRect *Tile::getPosition()
{
	return &position;
}

QRect *Tile::getPosition()
{
	return &position;
}

To copy to clipboard, switch view to plain text mode 

and the constructor:

Tile::Tile(QRect rect,int id,QPixmap tileImage)
{
	this->position = rect;
	this->tilesetID = id;
	this->tileImage = tileImage;
}

Tile::Tile(QRect rect,int id,QPixmap tileImage)
{
	this->position = rect;
	this->tilesetID = id;
	this->tileImage = tileImage;
}

To copy to clipboard, switch view to plain text mode 

[high_flyer]

Your for loop statement is not good:

for(int i = 0;i </*!=*/ curTileset->getTileContainer()->size();i++)

for(int i = 0;i </*!=*/ curTileset->getTileContainer()->size();i++)

To copy to clipboard, switch view to plain text mode 

[LuckyBlade]

As far as I know that's the common way to iterate through vector containers / containers in general.
I read it in the book "C++ Primer" and "QT 4.6 - Gui Entwicklung mit C++" (a german book)

[high_flyer]

Sorry I was thinking about something else.
Your version is ok, but dangerous, since it will also allow i> maxVal.
With '<' you are on the safe side.

I read it in the book "C++ Primer" and "QT 4.6 - Gui Entwicklung mit C++" (a german book)

Did they really use != for this case, or did they use an iterator and compared it to <container>.end() (with !=)?
Its not the same case!

But true, this is not the problem here.

Can you show your full paintEvent()? the problem must be somewhere there...

[LuckyBlade]

I'm not sure right now if they used != only for iteration with iterators in "C++ Primer", but I'm quite sure that they did it in "QT 4.6 - Gui Entwicklung" without iterators.

But anyway, here's the whole paintEvent:

void K15_TilesetEditWidget::paintEvent(QPaintEvent *pe)
{	
	QPainter painter(this);
	if(curTileset != NULL) {
			//1.Tileset gets drawn.
			for(int i = 0;i != curTileset->getTileContainer()->size();i++)
				painter.drawPixmap(*curTileset->getTileContainer()->at(i)->getPosition(),*curTileset->getTileContainer()->at(i)->getTileImage());
 
			//2.Collision gets drawn (if collisionMode is true)
			if(this->mode == selectCollision)
				for(int i = 0;i != curTileset->getTileContainer()->size();i++) {
					//Attention!Multiline Code
					painter.drawPixmap*(curTileset->getTileContainer()->at(i)->getPosition(),
						*curTileset->getTileContainer()->at(i)->getCollisionType()->getPixmap());
					}
 
			//3.TileID gets drawn (if true)
			if(this->tileIDsVisible) 
				for(int i = 0;i != curTileset->getTileContainer()->size();i++) 
					//Attention!Multiline Code
					painter.drawText(curTileset->getTileContainer()->at(i)->getPosition()->x() + 12,
						curTileset->getTileContainer()->at(i)->getPosition()->y() + 12,
						QString().setNum(curTileset->getTileContainer()->at(i)->getTilesetID()));
 
			//4.Grid gets drawn (if true)
			if(this->gridVisible) {
				for(int i = 0;i <= curTileset->getImage()->height();i += 32)
					painter.drawLine(0,i,curTileset->getImage()->width(),i);
 
				for(int j = 0;j <= curTileset->getImage()->width();j += 32) 
					painter.drawLine(j,0,j,curTileset->getImage()->height());	
				}
		}
	}

void K15_TilesetEditWidget::paintEvent(QPaintEvent *pe)
{	
	QPainter painter(this);
	if(curTileset != NULL) {
			//1.Tileset gets drawn.
			for(int i = 0;i != curTileset->getTileContainer()->size();i++)
				painter.drawPixmap(*curTileset->getTileContainer()->at(i)->getPosition(),*curTileset->getTileContainer()->at(i)->getTileImage());
			
			//2.Collision gets drawn (if collisionMode is true)
			if(this->mode == selectCollision)
				for(int i = 0;i != curTileset->getTileContainer()->size();i++) {
					//Attention!Multiline Code
					painter.drawPixmap*(curTileset->getTileContainer()->at(i)->getPosition(),
						*curTileset->getTileContainer()->at(i)->getCollisionType()->getPixmap());
					}

			//3.TileID gets drawn (if true)
			if(this->tileIDsVisible) 
				for(int i = 0;i != curTileset->getTileContainer()->size();i++) 
					//Attention!Multiline Code
					painter.drawText(curTileset->getTileContainer()->at(i)->getPosition()->x() + 12,
						curTileset->getTileContainer()->at(i)->getPosition()->y() + 12,
						QString().setNum(curTileset->getTileContainer()->at(i)->getTilesetID()));

			//4.Grid gets drawn (if true)
			if(this->gridVisible) {
				for(int i = 0;i <= curTileset->getImage()->height();i += 32)
					painter.drawLine(0,i,curTileset->getImage()->width(),i);

				for(int j = 0;j <= curTileset->getImage()->width();j += 32) 
					painter.drawLine(j,0,j,curTileset->getImage()->height());	
				}
		}
	}

To copy to clipboard, switch view to plain text mode 

Thanks btw for your help so far.

[high_flyer]

I'm not sure right now if they used != only for iteration with iterators in "C++ Primer", but I'm quite sure that they did it in "QT 4.6 - Gui Entwicklung" without iterators.

Bad!

Well, you are over writting your pixmap that you drew in the first for loop.
Comment all the sections except the first one, and see if it gets written correctly.

[LuckyBlade]

The pixmap won't get over written completly.
The pixmap that get's drawn in the second loop is just a little image that is nearly complete transparent (I'm working on a leveleditor for a 2D game and the image that gets drawn in the second loop is a little image that shows if the current tile is passable by the player or not.).

I commented anyway but that didn't change anything :-/

EDIT:
Whats interesting though, is that also the little "collsionimages" gets drawn incorrect.
I pretent that the error must be in the function where the original image gets tiled.
I'll add an image in a few minutes.

[high_flyer]

Ok, leave it commented out, so that we can easily find the problem.
In the screenshot you posted, there are 4 pixmaps - you get all 4 even with only one for loop in your paintEvent()?

[LuckyBlade]

Thats what I talked about in the last post.If you look at the "collisionimages" you'll see that they're also on the wrong position.

EDIT:
Yeah I get 4 images if I leave it commented.The result is the same.

[high_flyer]

I am sorry, how your answer is answering my question?

[LuckyBlade]

Sorry.
The result is the same as shown above in the image I posted.

[high_flyer]

This means that there is more code influencing your vector tiles, since more is drawn then is filled in the code you posted so far.

[LuckyBlade]

Holy maccaroni!
I fixed it.

I changed

for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
 
		if(rect.x() + 32 >= originalImage.width()) {
			rect.setY(rect.y() + 32);
			rect.setX(0);
		}else
			rect.setX(rect.x() + 32);
 
	}

for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
	
		if(rect.x() + 32 >= originalImage.width()) {
			rect.setY(rect.y() + 32);
			rect.setX(0);
		}else
			rect.setX(rect.x() + 32);

	}

To copy to clipboard, switch view to plain text mode 

to

for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
 
		if(rect.x() + 32 >= originalImage.width()) {
			rect.moveTop(32 + rect.y());
			rect.moveLeft(0);
		}else
			rect.moveLeft(32 + rect.x());
 
	}

for(int i = 0;i != goal;i++) {
		sourceVector->append(new Tile(rect,i,originalImage.copy(rect)));
	
		if(rect.x() + 32 >= originalImage.width()) {
			rect.moveTop(32 + rect.y());
			rect.moveLeft(0);
		}else
			rect.moveLeft(32 + rect.x());

	}

To copy to clipboard, switch view to plain text mode 

Everything works fine!

[high_flyer]

If you had used my code from post 4 you'd had that solved since I was using setTopLeft().