QImage read pixels and copy them with scanLine method

mindless

Hi everyone, I am trying to get an image's pixels with the scanLine method. I try to use this method because it's much faster than using setPixel or setPixelsColor. I wrote a code that get all the pixels and simply reapply them. But what I get is weird. It looks like a coordinate mistake but I checked multiple times and saw nothing.. If you have a faster and/or cleaner way to copy the original pixels, It will be nice to advice me !

Here is the original image, and the one after the process
process result

Here is the code:

            QRgb pixelsCopy[img->height()][img->width()];

            for (int y = 0; y < img->height(); ++y) {
                QRgb * line = reinterpret_cast<QRgb *>(img->scanLine(y));
                for (int x = 0; x < img->width(); ++x) {
                    pixelsCopy[y][x] = line[x];
                }
            }

            for (int x = 0; x < img->width(); ++x) {
                for (int y = 0; y < img->height(); ++y) {
                    int r = qRed(pixelsCopy[y][x]);
                    QColor final(r, r, r);
                    img->scanLine(y)[x] = final.rgb();

                }
            }

Chris Kawa

Hi, welcome to the forum.

I would advise against holding pixel data as array of arrays. Just have a pointer to a data block on the heap. Keeping it on the stack will result in stack overflow for larger images.

int size = img->height() * img->width();
QRgb* data = new QRgb[size]; //don't forget to delete it somewhere

If you just want to copy the bits copy the bits. You don't need to do it one pixel at a time. That's slow.

memmove(data, img.bits(), img.height() * img.width() * sizeof(QRgb));

As for the data processing - this:

for (int x = 0; x < img->width(); ++x) {
   for (int y = 0; y < img->height(); ++y) {

is going to be slow. You're going in the most cache unfriendly order. If you really have to do it like that then at least swap the loops:

for (int y = 0; y < img->height(); ++y) {
   for (int x = 0; x < img->width(); ++x) {

but then you're getting a pointer to single element in the scanline each time, which kinda defeats the purpose of the scanline (getting a pointer to whole row at a time).
If you use a single blob of data like I suggested you can just go at it like this:

QRgb* ptr = data;
QRgb* end = ptr + img.width() * img.height();
for (; ptr < end; ++ptr)
    *ptr = qRgb(qRed(*ptr), qRed(*ptr), qRed(*ptr));

It would be even better if you split the work across multiple threads and use SSE to do couple of pixels at a time. How to do that depends on your platform and compiler.

Btw. You don't have to make a copy, apply an effect and copy the pixels back. You can just apply the change in the original image.

Chris Kawa

Also, if you're allergic to raw pointers, you can do it STL style:

//copy
std::vector<QRgb> pixels;
pixels.resize(img.height() * img.width());
memmove(pixels.data(), img.bits(), img.height() * img.width() * sizeof(QRgb));

//process
std::for_each(pixels.begin(), pixels.end(), [](QRgb& c) { c = qRgb(qRed(c), qRed(c), qRed(c)); });

In c++17 you'll even be able to parallelize and vertorize it easily like this:

std::for_each(std::execution::parallel_unsequenced_policy, 
              pixels.begin(), pixels.end(), [](QRgb& c) { c = qRgb(qRed(c), qRed(c), qRed(c)); });

mindless

Thanks @Chris-Kawa for your answer ! It is so useful what you say here. Just one last question, when you say :

Btw. You don't have to make a copy, apply an effect and copy the pixels back. You can just apply the change in the original image.

Does that mean that I can directly modify the pixels of an image without worrying about copying ? Because If the process of a pixel depends on the other pixels, I need to keep the originals right ?

Chris Kawa

@mindless said in QImage read pixels and copy them with scanLine method:

Does that mean that I can directly modify the pixels of an image without worrying about copying ? Because If the process of a pixel depends on the other pixels, I need to keep the originals right ?

It depends what transformation you want to apply. In this particular case you just read one pixel, transform it and write it back so you don't need to copy the whole thing, just modify the pixel directly.

There are of course algorithms that require lookup into other values. It's a common struggle in graphics to create algorithms that can be applied without a copy. Maybe you can only read values ahead, not the ones already modified. Maybe you can traverse the data in order that would make the copy unnecessary. Maye you can apply transformation to a small local block of data etc. It all depends on what you want to do.

mindless

@Chris-Kawa Ok I see, thanks for your help, I understand know how it works.

First, I posted this question on Stackoverflow and I got no answer so this is why I posted here. If you want, you can add your answer also on this post, or I could post it and mention your original post.