-1

Basically, I'm trying to loop through every pixel of a picture and check it against every pixel of another image. The problem is that it seems to just do this very slowly (I can no longer interact with the opened window, and Debug.WriteLine works). I want to be sure this is the problem rather than there just being something wrong with my code.

monPic and crop are dimmed as bitmaps at the top of my code.

Private Sub BtnCheck_Click(sender As Object, e As EventArgs) Handles btnCheck.Click
        monPic = New Bitmap("../../../../" & picNum & ".png")
        crop = New Bitmap("../../../../mm.png")
        For x As Integer = 0 To monPic.Width - 1
            Debug.WriteLine("level 1")
            For y As Integer = 0 To monPic.Height - 1
                Debug.WriteLine("level 2")
                If CInt(monPic.GetPixel(x, y).A) <> 0 Then
                    For x2 As Integer = 0 To crop.Width - 1
                        Debug.WriteLine("level 3")
                        For y2 As Integer = 0 To crop.Height - 1
                            Debug.WriteLine("level 4")
                            If monPic.GetPixel(x, y).R = crop.GetPixel(x2, y2).R And monPic.GetPixel(x, y).G = crop.GetPixel(x2, y2).G And monPic.GetPixel(x, y).B = crop.GetPixel(x2, y2).B Then matches += 1
                        Next y2
                    Next x2
                End If
            Next y
        Next x
        lblMatches.Text = CStr(matches)
    End Sub
Purkat
  • 21
  • 3
  • 3
    Start with this: [Bitmap.LockBits](https://learn.microsoft.com/en-us/dotnet/api/system.drawing.bitmap.lockbits). Remove all `Console.WriteLine()` things while processing images. Read the notes here: [Analyze colors of an Image](https://stackoverflow.com/a/59102380/7444103). Don't use paths written in that manner. – Jimi Apr 21 '20 at 21:05
  • 2
    `GetPixel` and `SetPixel` are abstractions on top of GDI+ (C++ code), and do a lot of extra calculations for safety, making them quite slow. See the answer to this [other SO question](https://stackoverflow.com/questions/17208254/how-to-change-pixel-color-of-an-image-in-c-net/17208320#17208320) for information. The gist is that for performance, you're better off grabbing the underlying data directly and looping on it yourself. – Sean Skelly Apr 21 '20 at 21:47
  • @Jimi, what do you mean by "in that manner"? The use of variable or "../../../../"? – Purkat Apr 22 '20 at 01:24
  • @SeanSkelly I think I get most of the code in that linked answer, but `byte* sourceRow = (byte*)sourceData.Scan0 + (y * sourceData.Stride);` and `byte* targetRow = (byte*)targetData.Scan0 + (y * targetData.Stride);` are confusing me. What do they do and how do I use them in vb.net? – Purkat Apr 22 '20 at 01:27
  • 1
    @Purple: You write a little library in C# that can run `unsafe` code to do what you need, and reference it from your VB library. ;) Seriously though, VB doesn't do pointers. So you may have to live with the performance you get from `GetPixel`, unless you also can use C#. But don't forget that the best performing code is the code that works and solves the problem; speed should be a secondary concern. – Sean Skelly Apr 22 '20 at 01:35
  • 2
    As a next step, you might consider trying to reverse your loops to be y first, and then x, since the data underneath I believe is stored row-major. Memory caching is a big deal; the OS 'looks ahead' in memory in an attempt to speed things up. It's an easy swap, and might gain you a little bit of speed. More info found in this [other SO question](https://stackoverflow.com/questions/33722520/why-is-iterating-2d-array-row-major-faster-than-column-major) – Sean Skelly Apr 22 '20 at 01:37
  • Well, I don't technically know that it works, I'm just fairly certain it just takes a while to run. :P The program would need to to this many many times (about 900), so I'm not sure f it would be worth the wait at that point. – Purkat Apr 22 '20 at 01:56
  • 1
    I think @Jimi means for you to use `Path.Combine` – Mary Apr 22 '20 at 02:59
  • 1
    You cannot use that kind of path because the target won't exist when you move you executable somewhere else. Maybe you're just testing, but since you're testing, you should also test the file fetching capabilities of your app. Always use `Path.Combine` to build paths. The only exception is when the path is provided by an `OpenFileDialog`, or manually entered by a User (you use `File.Exists()` in this case). The code I linked is VB.Net, not C#, you can use that code without modifications, it already does what you need. – Jimi Apr 22 '20 at 08:21
  • @Jimi, the code you linked modifies the colors of one image, I need to to compare the pixels from 2 images. I've tried to modify it to do this, but I can't seem to do it. – Purkat Apr 22 '20 at 15:50

1 Answers1

0

This works quickly. It requires

Imports System.Security.Cryptography

Convert the 2 bitmaps to Byte arrays then hash with Sha256. Compare the hash.

Adapted from https://www.codeproject.com/Articles/9299/Comparing-Images-using-GDI

Private Function Compare(bmp1 As Bitmap, bmp2 As Bitmap) As String
    Dim result = "It's a match!"
    If Not (bmp1.Size = bmp2.Size) Then
        result = "It's not even the same size"
    Else
        Dim ic As New ImageConverter
        Dim btImage1(0) As Byte
        btImage1 = CType(ic.ConvertTo(bmp1, btImage1.GetType), Byte())
        Dim btImage2(0) As Byte
        btImage2 = CType(ic.ConvertTo(bmp2, btImage2.GetType), Byte())
        Dim shaM As New SHA256Managed
        Dim hash1 = shaM.ComputeHash(btImage1)
        Dim hash2 = shaM.ComputeHash(btImage2)
        Dim i As Integer = 0
        Do While i < hash1.Length AndAlso i < hash2.Length AndAlso result = "It's a match!"
            If hash1(i) <> hash2(i) Then
                result = "The pixels don't match"
            End If
            i = (i + 1)
        Loop
    End If
    Return result
End Function

Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
    Dim png1 As New Bitmap(path1)
    Dim png2 As New Bitmap(path2)
    Dim message = Compare(png1, png2)
    MessageBox.Show(message)
End Sub
Mary
  • 14,926
  • 3
  • 18
  • 27
  • 1
    This is not the way. You're comparing the bytes of two files, not their pixels' values. Also, this is simply `dim isEqual = btImage1.SequenceEqual(btImage2)`. There's no need to hash anything, you're comparing bytes. The person that wrote that code doesn't know what the code is actually doing: *I looked at the LockBits [...] but it would have meant a journey into the land of unmanaged code*. => 1) there's no unmanaged code involved when using `Bitmap.LockBits()` 2) Hashing and comparing the bytes of two files is meaningless when we need to compare color values (+ the overhead of the hash). – Jimi Apr 22 '20 at 08:07
  • @Jimi I am sure you are correct. I know very little about graphics. In my defense, I test with a 2 different .png's, two dulpicates, and one with only a tiny dot different. – Mary Apr 22 '20 at 08:10
  • 1
    No need to defend, I won't *bite* :) - The point is, comparing file bytes and pixels values are two quite distinct and not really compatible things: `ImageConverter.Convert` simply converts the Bitmap file content to bytes, headers included: if you open up a PNG Bitmap with a hex editor, you'll see that the first 4 bytes, `89 50 4E 47` are included in the converted byte array. `Bitmap.LockBits()` will instead give you the Image data (the pixels values as bytes). – Jimi Apr 22 '20 at 08:27