Multithreaded per pixel operations on QImage

[wysota]

setPixel is slow, use QImage::scanLine() instead. You won't need to protect it with a mutex.

[N¤X]

Thx for the clue. I just implemented it and ... the same patterns again! And again a mutex solves the pattern problem but slows the whole thing down
This works now, and with ~7sec runtime its faster then with setPixel, but still slower than without multithreading (~2sec) -_-

void Scan::convertToGrayscale(int from, int to) {
   int temp;
   for (int i=from; i<to; i++) {
      for (int j=0; j<imgOrig->height(); j++) {
         temp = qGray(imgOrig->pixel(i, j));
         mutex.lock();
         //imgDone->setPixel(i, j, qRgb(temp, temp, temp));
         *((QRgb*) imgDone->scanLine(j)+i) = qRgb(temp, temp, temp);
         mutex.unlock();
      }
   }
}

void Scan::convertToGrayscale(int from, int to) {
   int temp;
   for (int i=from; i<to; i++) {
      for (int j=0; j<imgOrig->height(); j++) {
         temp = qGray(imgOrig->pixel(i, j));
         mutex.lock();
         //imgDone->setPixel(i, j, qRgb(temp, temp, temp));
         *((QRgb*) imgDone->scanLine(j)+i) = qRgb(temp, temp, temp);
         mutex.unlock();
      }
   }
}

To copy to clipboard, switch view to plain text mode 

I'll replace the pixel() function with scanline too, but the general problem of being slower with multiple threads than with only one still exists :/ Any Ideas about that? ^^

[wysota]

This works for me quite well:

#include <QtGui>
 
 
struct ImageRef {
  QRgb *data;
  int width;
};
 
void convertToGS(const ImageRef &ref){
  for(int i=0;i<ref.width;i++){
    int gray = qGray(ref.data[i]);
    ref.data[i] = qRgb(gray, gray, gray);
  }
}
 
int main(int argc, char **argv){
  if(argc<2) return 1;
  QApplication app(argc, argv);
  QImage img(argv[1]);
  QList<ImageRef> refs;
  for(int i=0;i<img.height();i++){
    ImageRef ref;
    ref.data = (QRgb*)img.scanLine(i);
    ref.width = img.width();
    refs << ref;
  }
  QtConcurrent::blockingMap(refs, convertToGS);
  QLabel l;
  l.setPixmap(QPixmap::fromImage(img));
  l.show();
  return app.exec();
}

#include <QtGui>


struct ImageRef {
  QRgb *data;
  int width;
};

void convertToGS(const ImageRef &ref){
  for(int i=0;i<ref.width;i++){
    int gray = qGray(ref.data[i]);
    ref.data[i] = qRgb(gray, gray, gray);
  }
}

int main(int argc, char **argv){
  if(argc<2) return 1;
  QApplication app(argc, argv);
  QImage img(argv[1]);
  QList<ImageRef> refs;
  for(int i=0;i<img.height();i++){
    ImageRef ref;
    ref.data = (QRgb*)img.scanLine(i);
    ref.width = img.width();
    refs << ref;
  }
  QtConcurrent::blockingMap(refs, convertToGS);
  QLabel l;
  l.setPixmap(QPixmap::fromImage(img));
  l.show();
  return app.exec();
}

To copy to clipboard, switch view to plain text mode 

Multithreaded version takes 7ms as opposed to singlethreaded version that takes 12ms to convert a 1600x1200 image to grayscale.

[N¤X]

Thank you, this really looks nice and clean! I will use this in the programm
But there still remains one problem: The slowest function of my programm mainly consists of a 2-pass algorithm that runs on scanlines, in one pass on horizontal ones and in the other pass on vertical ones. Due to the structure of QImages the direct accessible scanlines are all horizontal, so I can only use your solution in one pass.
I'll try if it still works if you don't overwrite the picture data but paint the result in a new one (the write-to-another-picture-operation was the problem from the beginning -_-). Then i could write the result of the first pass transposed into another Picture and just run the same operations over the new picture to get the final result... I'll write when I tried this!

[wysota]

Thank you, this really looks nice and clean! I will use this in the programm
But there still remains one problem: The slowest function of my programm mainly consists of a 2-pass algorithm that runs on scanlines, in one pass on horizontal ones and in the other pass on vertical ones. Due to the structure of QImages the direct accessible scanlines are all horizontal, so I can only use your solution in one pass.

No, that's not true. After the horizontal pass transpose the image and use scanlines again. Then transpose it back. You'll get improved cpu cache performance as well (thanks to Ariya for making me aware of that) and as a result it will probably be faster than operating on the picture directly.