4
\$\begingroup\$

I'm trying to write a program to detect the colour violet in an image.

As discussed in my previous Post @Johnbot suggested that I try using HSL / HSV to determine if the pixel is some shade of violet.

this.button2.Click += async (sender, e) =>
{
    Task task = Task.Run(() =>
    {
      ProcessImage(pictureBox1.Image);
    });
      Task.WaitAll(task);
      await task;
    };
}

    private void ProcessImage(Image image)
    {
        if (image == null)
        {
            return;
        }
        using (var selectedImage = new Bitmap(image))
        {
            double total = 0;
            double infected = 0;

            Color violet = Color.FromKnownColor(KnownColor.Violet);
            Color selectedPixel;

            for (int x = 0; x < selectedImage.Width; x++)
            {
                for (int y = 0; y < selectedImage.Height; y++)
                {
                    selectedPixel = selectedImage.GetPixel(x, y);

                    if (selectedPixel.GetHue() == (violet.GetHue()))
                    {
                        infected++;
                    }
                    total++;
                }
            }
            //TODO: Needs to be rounded
            //Change the value eg: 0.0030, 0.0010, 0.0050
            try
            {
                double percentage = (infected / total);
                Console.WriteLine(percentage);

                double result;
                if (Double.TryParse(Convert.ToString(percentage).Substring(0, 6), out result))
                {
                    if (result > 0.0017)
                    {
                        MessageBox.Show(string.Format("Image is infected: {0:P}", percentage));
                    }
                    else
                    {
                        MessageBox.Show("Threshold not reached");
                    }
                }
            }
            catch
            {
                MessageBox.Show("No Violet Pixels Whatsoever were found");
            }
            finally
            {
                selectedImage.Dispose();
            }
        }
    }

Here are two of the images I'm working with.

The first one has the colour violet in it quite a lot with a percentage of 0.64% or 0.00636199367565126
Image0

The second one from what I can tell doesn't have any violet in it, however the application believes it does with 0.36% violet pixels or 0.00355871886120996.
image1

The code above is working so to speak, however is there something I could be doing to improve it for reliability and performance?

I did see a post about Bitmap locking however because its only going to be scanning a image every hour or so for any violet pixels, it didn't implement it, should i still do it?

\$\endgroup\$
  • 1
    \$\begingroup\$ 0.64% seems way below what can be observed from the naked eye. There seems to be more than 10% or even 20%, don't you think? \$\endgroup\$ – Mathias Ettinger Jun 8 '18 at 8:00
  • \$\begingroup\$ I believe so too, I think I need to working out a better way to compare the injected and total pixels. \$\endgroup\$ – user1234433222 Jun 8 '18 at 8:02
  • \$\begingroup\$ Mathias raised a good point. Judging by results you are getting, this implementations hardly qualifies as "working code". You should check the pixel's hue against some range: violetHue - 10 < pixelHue < violetHue + 10 (arbitrary numbers here, you have to figure out the exact range yourself). \$\endgroup\$ – Nikita B Jun 8 '18 at 8:44
  • \$\begingroup\$ As my previous answer you should not write to the UI in a method that calculates. \$\endgroup\$ – paparazzo Jun 8 '18 at 9:05
4
\$\begingroup\$
this.button2.Click += async (sender, e) =>
{
    Task task = Task.Run(() =>
    {
      ProcessImage(pictureBox1.Image);
    });
      Task.WaitAll(task);
      await task;
    };
}

Is the indentation like that in the original, or did it get corrupted by the tab-to-space conversion of StackExchange?

What's going on with the task? I don't know much about async event handlers, but as far as general async/await usage you only need one of Task.WaitAll(task) or await task.

What stops me from mashing the button and spinning off loads of tasks? Normally you would want to prevent a second click by opening a modal dialog (denying access to the button) or disabling the button, and then using ContinueWith to undo the block in the GUI thread when the task finishes.


    private void ProcessImage(Image image)
    {
        if (image == null)
        {
            return;
        }
        using (var selectedImage = new Bitmap(image))

If the image is already a bitmap, this is unnecessary and slow. It's probably worth having two cases and factoring out the code which works with Bitmap to a separate method:

    if (image is Bitmap) Process(image as Bitmap);
    else using (var bitmap = new Bitmap(image)) Process(bitmap);

            double total = 0;
            double infected = 0;

That's an odd choice for counters. Why not use an integer type and coerce to double when dividing?


            for (int x = 0; x < selectedImage.Width; x++)
            {
                for (int y = 0; y < selectedImage.Height; y++)
                {
                    selectedPixel = selectedImage.GetPixel(x, y);

This is slow. A loop over GetPixel is always slow, and it's worse when the outer loop is over x because you lose cache locality. If you care about speed, use LockBits and switch the loops.


                    if (selectedPixel.GetHue() == (violet.GetHue()))

GetHue() returns a float. == comparisons on non-integer numeric types are generally inadvisable. You should almost certainly test whether selectedPixel.GetHue() is within a small distance of violet.GetHue(). How wide a range is a parameter which you may need to tune to get the results you expect.


                if (Double.TryParse(Convert.ToString(percentage).Substring(0, 6), out result))

??! Why? And if the specs require you to round to an exact number of decimal places and that's non-negotiable, why use double instead of decimal?


            catch
            {
                MessageBox.Show("No Violet Pixels Whatsoever were found");
            }

Under what circumstances can this catch anything? The only one I can see is if total == 0, in which case the error message should really be that the image was empty.


            finally
            {
                selectedImage.Dispose();
            }

This is unncessary: it's already in a using statement.

\$\endgroup\$
  • \$\begingroup\$ thank you for your reply mate, Yeah im not too fussed at this stage about the speed of the image processing, as long as it does it at least 90 - 95 % correct. I had added your changes, thanks again! oh just quickly for that catch block, I added that because if there were less infected pixels than total pixels, it would through an error when dividing them, this could be maybe because its not a decimal? \$\endgroup\$ – user1234433222 Jun 8 '18 at 15:01
  • 1
    \$\begingroup\$ Ah, maybe you were getting an exception from Substring(0, 6) because the string was shorter than that. Another reason to ditch the conversion to string... \$\endgroup\$ – Peter Taylor Jun 8 '18 at 17:44
2
\$\begingroup\$

The hardest part is figuring out what violet is. The math gets complex in a hurry and it can be hard to come up with a right answer.

As you know you can't just compare the distance between two colors because red is the same distance from blue as blue is from green, etc. As a completely abstract example just look at the standard basis vectors (*255) in ℝ³.

enter image description here

So if you are comparing like this there will be this fine line where on one side you have violet and on the other side, not violet. I know you're using HSL but I suspect you're having a similar problem and all the tweaking in the world won't help. Trying to work out the math yourself is a losing battle too. The good news is this amazingly awesome and nice website has done the work for us http://www.easyrgb.com/en/math.php.

I used it on my color picker which you might find useful as it finds the closest named color and already has some working code.

\$\endgroup\$
  • \$\begingroup\$ Hey mate, you’re not wrong, it is incredibly difficult to detect all shades of violet and nothing else. \$\endgroup\$ – user1234433222 Jun 9 '18 at 8:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.