PDA

View Full Version : multithreaded QImageReader



dcole
23rd June 2011, 21:59
Hello,

I am trying to create image tiles that load asynchronously as they are displaying in a QGraphicsView. I have subclassed QGraphicsItem, and overloaded the pain method with the following:


void RasterTile::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,QWidget *widget)
{
if(image.isNull())
{
TilePainter=painter;
TileOption=option;
TileWidget=widget;
future = QtConcurrent::run(this, &RasterTile::LoadTilePixmap);
watcher.setFuture(future);

}else
{
QRectF imageRect = image.rect();
painter->drawImage(imageRect, image);
}

}

Which calls the following in its own thread:


QImage RasterTile::LoadTilePixmap()
{
QMutexLocker locker(&mutex);

QImage img(nBlockXSize, nBlockYSize, QImage::Format_RGB32);

QRect rect(tilePosX*nBlockXSize, tilePosY*nBlockYSize, nBlockXSize, nBlockYSize);

reader->setClipRect(rect);
reader->read(&img);
if(reader->error())
{
qDebug("Not null error");
qDebug()<<"Error string is: "<<reader->errorString();
}
return img;

}

mutex is a static member of my class which is defined in the .h, and in the cpp

QMutex RasterTile::mutex;

This code seems to malfunction. If i have like 5 tiles of an image, they all seem to get some data in them, but it is often scrambled and such. Occaisionally one will load all of the image data correctly. This leads me to believe I have something going wrong with the threading. Each tile has its own instance of a QImageReader, all pointing to the same file.

What am I doing wrong here?

wysota
23rd June 2011, 22:04
What is the use of the mutex?

dcole
23rd June 2011, 22:10
Well, it was leftover sort of. I had originally created one QImageReader, passed that pointer to all tiles, and used it to keep multiple threads for reading from the same file at once because I was getting some errors from libjpeg, libpng, etc. This made me think that was the issue.

Presently I get no errors, the image just doesnt load and look correct.

Well, it was leftover sort of. I had originally created one QImageReader, passed that pointer to all tiles, and used it to keep multiple threads for reading from the same file at once because I was getting some errors from libjpeg, libpng, etc. This made me think that was the issue.

Presently I get no errors, the image just doesnt load and look correct.

Here is the full code from my first try - passing a pointer to the reader into each tile:

Header:



class RasterTile : public Tile
{
public:
RasterTile (QImageReader *reader, int nBlocksX, int nBlocksY, int xoffset, int yoffset, int nXBlockSize, int nYBlockSize);

protected:
QImageReader *reader;
void paint(QPainter *painter, const QStyleOptionGraphicsItem *option,QWidget *widget);
QImage LoadTilePixmap();
private:
mutable QMutex mutex;
};

Cpp:


#include "rastertile.h"

RasterTile::RasterTile(QImageReader *reader, int nBlocksX, int nBlocksY, int xoffset, int yoffset, int nXBlockSize, int nYBlockSize)
: Tile(nBlocksX, nBlocksY, xoffset, yoffset, nXBlockSize, nYBlockSize)
{
this->reader = reader;
connect(&watcher,SIGNAL(finished()),this,SLOT(updateSceneSl ot()));
}


void RasterTile::paint(QPainter *painter, const QStyleOptionGraphicsItem *option,QWidget *widget)
{
if(image.isNull())
{
TilePainter=painter;
TileOption=option;
TileWidget=widget;
future = QtConcurrent::run(this, &RasterTile::LoadTilePixmap);
watcher.setFuture(future);

}else
{
QRectF imageRect = image.rect();
painter->drawImage(imageRect, image);
}

}

QImage RasterTile::LoadTilePixmap()
{
QMutexLocker locker(&mutex);
QImage img(nBlockXSize, nBlockYSize, QImage::Format_RGB32);
QRect rect(tilePosX*nBlockXSize, tilePosY*nBlockYSize, nBlockXSize, nBlockYSize);
reader->setClipRect(rect);
reader->read(&img);
return img;

}



How I am instantiating the classes:


for(int i =0; i < nXBlocks; i++)
{
for(int j = 0; j < nYBlocks; j++)
{
RasterTile *ipi = new RasterTile(reader,nXBlocks, nYBlocks, i, j, nXBlockSize, nYBlockSize);

}
}

After this try, is when i started trying to make the mutex private, and then also passing in just the file name so each Tile could create it's own reader.

"image" is a QImage that gets set to the img from "LoadTilePixmap()" and is set in the Tile::updateSceneSlot()

jeanremi
23rd June 2011, 22:14
and overloaded the pain method
Weeeell when you overload the pain method that's what you get :-D
(Sorry i had to have a go, nice one)

OK, back to the problem. Well, in my opinion, by the description of your case, you do not need threading, you can do without (i.e. you're only reading from the image at different locations - not read/write). You can still create your RasterTiles at different times and still be able to read from the same image.
In other words, from the description of your problem, there is no basis for threading.

Best of lunch ... I mean luck ;-)

Jean

dcole
23rd June 2011, 22:23
Why do you say I wouldnt need threading? I am loading really large images. I have a loop in my GUI that figures out how many tiles I need, creates them all, groups and positions them in the right place, and then allows the user to go do other things while the image is loading. If I try to move the image, and a whole bunch of tiles need to load, the program will freeze while all of those tiles load. This will prevent a user from panning to a certain location in the image, and having the tiles load as they pan. It makes for a nicer user experience (think of google maps)

Added after 5 minutes:

I did just have a thought - when I call "setClipRect" on the reader, does that effectively trash all of the data that was not within the rect? Maybe reader is having it's data truncated, and that is why other threads have problems reading?

wysota
23rd June 2011, 22:26
I think that you can't access the same file concurrently from within more than one thread. Apparently, despite the mutex, you have some concurrent access somewhere. And it seems all your thread are using the same image reader which also might be leading to problems. Try creating the reader in the LoadTilePixmap method.

jeanremi
23rd June 2011, 23:07
..freeze while all of..

From this quoted description, you do need threading. Your first description/text/explanation didnot show anywhere that you needed it. That's what I meant.

However, having said that, from what you've just said, your threading model is still not justified i.e. you don't need to thread up your RasterTile class for what you're trying to do with it. Instead, you need to thread up between your RasterTile and the rest of your GUI (the user experience part).
You don't need to thread up your RasterTile class because you are reading different parts from your image, even if they are huge like the world!

The fact that some parts are returned scrambled is nothing to do with threading unless this dictates tile coordinates in your file, which from your description is nowhere mentioned.

Fix/check your reader first then Choose your threading model after.

All the best.

wysota
23rd June 2011, 23:16
Actually he doesn't need threading. There is a number of solutions one can use to avoid freezing the GUI -- Keeping the GUI Responsive. The "step by step" approach seems applicable here.