Image processing and sizes

ZapB

As an aside the number of cores is irrelevent here as you are only using one thread. Having said that it should not take that long to process such modestly sized image. Have you tried breaking the execution using the debugger when it freezes to see where it is getting stuck?

Anticross

is there a difference between pixel loops in different types of images ?

ZapB

What does QImage::format() return for an image that works and one that doesn't?

Anticross

I've got massages in output window of my debbuger
QImage::pixel: coordinate (0,2336) out of range
QImage::pixel: coordinate (0,2337) out of range
QImage::pixel: coordinate (0,2338) out of range
QImage::pixel: coordinate (0,2339) out of range
QImage::pixel: coordinate (0,2340) out of range
QImage::pixel: coordinate (0,2341) out of range
...
So it's happens when I processed 3504*2336 image. why it not limited by size of image or I have a mistake in my loops limits?

ZapB

You have you're i and j loop counters back to front in the call to image.pixel(). It should be:

@
QRgb currentPixel = image.pixel( j, i );
@

since you are using j to loop over the x-coord and i to iterate over the y coord. See how the debugger helps you ;-)

ZapB

You have made the same mistake in the call to setPixel() too.

ZapB

So with all the above in place your code should look like this:

@
void ImageWidget::proccessImage(QColor color)
{
// Search algorithm

QImage image = m_image;
int match = 5;

for(int i = 0; i<image.height(); i++)
{
for(int j = 0; j<image.width(); j++)
{
QRgb currentPixel = image.pixel(j,i);

QColor pixColor = QColor::fromRgb(currentPixel);

if(qAbs(pixColor.red()-color.red()) > match && 
   qAbs(pixColor.green()-color.green()) > match &&
   qAbs(pixColor.blue()-color.blue()) > match)
    image.setPixel(j,i,qRgb(255,255,255));

}
}
m_imageLabel->setPixmap(QPixmap::fromImage(image));
}
@

Anticross

Damn, it's so easy :) I need to be more attentive. Thanks for the help and some code optimization.

giesbert

Hi Anticross,

One idea to make the code a bit more readable und to easier find such problems (use x and y instead of i and j), make some renaming. You can also use QRgb directly.

@
void ImageWidget::proccessImage(QColor color)
{
// Search algorithm

QImage image = m_image;
int difference = 5;

for(int y = 0; y < image.height(); y++)
{
    for(int x = 0; x < image.width(); x++)
    {
        QRgb currentPixel = image.pixel(x,y);

        if( qAbs(qRed(currentPixel) - color.red()) > difference &&
            qAbs(qGreen(currentPixel) - color.green()) > difference &&
            qAbs(qBlue(currentPixel) - color.blue()) > difference)
        {
            image.setPixel(x,y,qRgb(255,255,255));
        }
    }
}
m_imageLabel->setPixmap(QPixmap::fromImage(image));

}
@

ZapB

I usually tend to use i for iterating over x and j for iterating over ysince it reminds me of i and j unit vectors. I also tend to use epsilon or delta for small values such as your match or difference variables. It's just taste though and this is my background in mathematics prejudicing me. ;-)

giesbert

As we are talking about coordinates, and height and width, it's usually x and y. That's why I changed that. i is usually (with the developers I work with :-) ) a variable for a normal for loop. For cascading, I normally use row and column or s9imilar things, por x and y if they are coordinated (which they are in this case).

But it's a matter of taste.

And match was (for me) just nothing that was clear from the name. match for me would be a boolean (matches or not)

ZapB

Yeah for me it also depends upon the context in which the loop(s) occur. If I am iterating over rows then I'll use "row". Usually I try to use something that makes it obvious to which the counter refers.