Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    primarykey
    data
    text
    <p>Are you really passing in a floating point number as a percentage to <code>isMatching</code>?</p> <p>I looked at your code for isMatching on GitHub and well, yikes. You ported this from Java, right? C# uses <code>bool</code> not <code>Boolean</code> and while I don't know for sure, I don't like the looks of code that does that much boxing and unboxing. Further, you're doing a ton of floating point multiplication and comparison when you don't need to:</p> <pre><code>public static bool IsMatching(Color a, Color b, int percent) { //this method is used to identify whether two pixels, //of color a and b match, as in they can be considered //a solid color based on the acceptance value (percent) int thresh = (int)(percent * 255); return Math.Abs(a.R - b.R) &lt; thresh &amp;&amp; Math.Abs(a.G - b.G) &lt; thresh &amp;&amp; Math.Abs(a.B - b.B) &lt; thresh; } </code></pre> <p>This will cut down the amount of work you're doing per pixel. I still don't like it because I try to avoid method calls in the middle of a per-pixel loop especially an 8x per-pixel loop. I made the method static to cut down on an instance being passed in that isn't used. These changes alone will probably double your performance since we're doing only 1 multiply, no boxing, and are now using the inherent short-circuit of &amp;&amp; to cut down the work.</p> <p>If I were doing this, I'd be more likely to do something like this:</p> <pre><code>// assert: bitmap.Height &gt; 2 &amp;&amp; bitmap.Width &gt; 2 BitmapData data = bitmap.LockBits(new Rectangle(0, 0, bitmap.Width, bitmap.Height), ImageLockMode.ReadWrite, PixelFormat.Format24bppRgb); int scaledPercent = percent * 255; unsafe { byte* prevLine = (byte*)data.Scan0; byte* currLine = prevLine + data.Stride; byte* nextLine = currLine + data.Stride; for (int y=1; y &lt; bitmap.Height - 1; y++) { byte* pp = prevLine + 3; byte* cp = currLine + 3; byte* np = nextLine + 3; for (int x = 1; x &lt; bitmap.Width - 1; x++) { if (IsEdgeOptimized(pp, cp, np, scaledPercent)) { // do what you need to do } pp += 3; cp += 3; np += 3; } prevLine = currLine; currLine = nextLine; nextLine += data.Stride; } } private unsafe static bool IsEdgeOptimized(byte* pp, byte* cp, byte* np, int scaledPecent) { return IsMatching(cp, pp - 3, scaledPercent) &amp;&amp; IsMatching(cp, pp, scaledPercent) &amp;&amp; IsMatching(cp, pp + 3, scaledPercent) &amp;&amp; IsMatching(cp, cp - 3, scaledPercent) &amp;&amp; IsMatching(cp, cp + 3, scaledPercent) &amp;&amp; IsMatching(cp, np - 3, scaledPercent) &amp;&amp; IsMatching(cp, np, scaledPercent) &amp;&amp; IsMatching(cp, np + 3, scaledPercent); } private unsafe static bool IsMatching(byte* p1, byte* p2, int thresh) { return Math.Abs(p1++ - p2++) &lt; thresh &amp;&amp; Math.Abs(p1++ - p2++) &lt; thresh &amp;&amp; Math.Abs(p1 - p2) &lt; thresh; } </code></pre> <p>Which now does all kinds of horrible pointer mangling to cut down on array accesses and so on. If all of this pointer work makes you feel uncomfortable, you can allocate byte arrays for prevLine, currLine and nextLine and do a Marshal.Copy for each row as you go.</p> <p>The algorithm is this: start one pixel in from the top and left and iterate over every pixel in the image except the outside edge (no edge conditions! Yay!). I keep pointers to the starts of each line, prevLine, currLine, and nextLine. Then when I start the x loop, I make up pp, cp, np which are previous pixel, current pixel and next pixel. current pixel is really the one we care about. pp is the pixel directly above it, np directly below it. I pass those into IsEdgeOptimized which looks around cp, calling IsMatching for each.</p> <p>Now this all assume 24 bits per pixel. If you're looking at 32 bits per pixel, all those magic 3's in there need to be 4's, but other than that the code doesn't change. You could parameterize the number of bytes per pixel if you want so it could handle either.</p> <p>FYI, the channels in the pixels are typically b, g, r, (a).</p> <p> Colors <em>are</em> stored as bytes in memory. Your actual Bitmap, if it is a 24 bit image is stored as a block of bytes. Scanlines are <code>data.Stride</code> bytes wide, which is at least as large as 3 * the number of pixels in a row (it may be larger because scan lines are often padded).</p> <p>When I declare a variable of type <code>byte *</code> in C#, I'm doing a few things. First, I'm saying that this variable contains the address of a location of a byte in memory. Second, I'm saying that I'm about to violate all the safety measures in .NET because I could now read and write any byte in memory, which can be dangerous.</p> <p>So when I have something like:</p> <pre><code>Math.Abs(*p1++ - *p2++) &lt; thresh </code></pre> <p>What it says is (and this will be long):</p> <ol> <li>Take the byte that p1 points to and hold onto it</li> <li>Add 1 to p1 (this is the ++ - it makes the pointer point to the next byte)</li> <li>Take the byte that p2 points to and hold onto it</li> <li>Add 1 to p2</li> <li>Subtract step 3 from step 1</li> <li>Pass that to <code>Math.Abs</code>.</li> </ol> <p>The reasoning behind this is that, historically, reading the contents of a byte and moving forward is a very common operation and one that many CPUs build into a single operation of a couple instructions that pipeline into a single cycle or so.</p> <p>When we enter <code>IsMatching</code>, <code>p1</code> points to pixel 1, <code>p2</code> points to pixel 2 and in memory they are laid out like this:</p> <pre><code>p1 : B p1 + 1: G p1 + 2: R p2 : B p2 + 1: G p2 + 2: R </code></pre> <p>So <code>IsMatching</code> just does the the absolute difference while stepping through memory.</p> <p>Your follow-on question tells me that you don't really understand pointers. That's OK - you can probably learn them. Honestly, the concepts really aren't that hard, but the problem with them is that without a lot of experience, you are quite likely to shoot yourself in the foot, and perhaps you should consider just using a profiling tool on your code and cooling down the worst hot spots and call it good.</p> <p>For example, you'll note that I look from the first row to the penultimate row and the first column to the penultimate column. This is intentional to avoid having to handle the case of "I can't read above the 0th line", which eliminates a big class of potential bugs which would involve reading outside a legal memory block, which may be benign under many runtime conditions.</p>
    singulars
    1. This table or related slice is empty.
    plurals
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. VO
      singulars
      1. This table or related slice is empty.
    2. VO
      singulars
      1. This table or related slice is empty.
    3. VO
      singulars
      1. This table or related slice is empty.
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload