Surface[] doesn't return correct values.

[zarathoustra]

Hello,

I'm trying to create a plugin for PDN 3.5 beta.

This is my first plugin. I managed to write something simple like applying a threshold on pixels.

But the plugin i'm trying to write is a bit more complicated, and for some reasons Surface[x,y] always returns a black pixel, although there are some white pixels.

The only explanation i could find is that you can't read pixels in the surface outside of roi. If this is it, then how do you implement convolutions ?

I think my issue might lie in the fact i have to scan the whole picture for computing every single pixel.

Here is my code (i omited some code for clarity)

       public override void Render(EffectConfigToken parameters, RenderArgs dstArgs, RenderArgs srcArgs, Rectangle[] rois, int startIndex, int length)
       {
           byte threshold = 127;

           PdnRegion selectionRegion = EnvironmentParameters.GetSelection(srcArgs.Bounds);

           for (int i = startIndex; i < startIndex + length; ++i)
           {
               Rectangle rect = rois[i];

               for (int y0 = rect.Top; y0 < rect.Bottom; ++y0)
               {
                   for (int x0 = rect.Left; x0 < rect.Right; ++x0)
                   {
                       Complex accumulator = new Complex(0, 0);

                       for (int x = -x0; x < rect.Width - x0; x++)
                           for (int y = -y0; y < rect.Height - y0; y++)
                           {
                               ColorBgra readPixel = srcArgs.Surface[x0 + x, y0 + y];

                               if ((readPixel.R > threshold) && (readPixel.G > threshold) && (readPixel.B > threshold))
                                   accumulator += GaborFilterKernel(x, y); // never reached! why ?
                               else
                                   readPixel.ToString();
                           }


                       ColorBgra result = new ColorBgra();
                       result.A = 0xff;
                       result.R = (byte)(accumulator.Magnitude*255);
                       result.G = (byte)(accumulator.Magnitude*255);
                       result.B = (byte)(accumulator.Magnitude*255);
                       //dstArgs.Surface[x0, y0] = result; // will this affect the source surface ???
                   }
               }
           }
       }

Thanks

[zarathoustra]

so noone knows why i have correct values in x0;y0 loops but not in x;y loops ?

[pyrochild]

Explain your loop structure:

for (int y0 = rect.Top; y0     for (int x0 = rect.Left; x0         for (int x = -x0; x             for (int y = -y0; y             { }

You do realize this does absolutely nothing, right?

else
   readPixel.ToString();

[zarathoustra]

for (int y0 = rect.Top; y0 < rect.Bottom; ++y0)
   for (int x0 = rect.Left; x0 < rect.Right; ++x0)

This part is for looping over the roi for writing pixel values.

for (int x = -x0; x < rect.Width - x0; x++)
           for (int y = -y0; y < rect.Height - y0; y++)

This part is for computing the value of a given pixel. (i have to read all pixels in the image to compute the value of a pixel).

You do realize this does absolutely nothing, right?

Yes, i always do that for setting up breakpoints on code that should not be.

[BoltBait]

I'll answer the easy question:

//dstArgs.Surface[x0, y0] = result; // will this affect the source surface ???

No, writing to the dstArgs.Surface only effects the destination canvas. That's what you're supposed to do.

Of course, it will only do that if the line is not commented out.

[zarathoustra]

That was my guess, but i commented it out just to make sure for now, since anyways i'm not reading correct data from the source :?

[pyrochild]

for (int x = -x0; x             for (int y = -y0; y 
This part is for computing the value of a given pixel. (i have to read all pixels in the image to compute the value of a pixel).
A) Why do you need to read all the pixels?
 Why do you need to do it this way, instead of just, I dunno, looping through them? What is the purpose of starting the inner loop at the negative of the outer loop's value?
C) Why are your inner loops nested in the opposite direction of the outer loops?

[zarathoustra]

for (int x = -x0; x < rect.Width - x0; x++)
           for (int y = -y0; y < rect.Height - y0; y++)

This part is for computing the value of a given pixel. (i have to read all pixels in the image to compute the value of a pixel).

A) Why do you need to read all the pixels?

Why do you need to do it this way, instead of just, I dunno, looping through them? What is the purpose of starting the inner loop at the negative of the outer loop's value?

C) Why are your inner loops nested in the opposite direction of the outer loops?

A) because the dst value of a pixel depends on pixels around, not just the src pixel at the same location. This is just like other convolutions.

This point of the code might look weird indeed. It's not that i really HAVE TO do it this way. But this filter is defined mathematically this way, i'm just follwing it here. I start in the negative and go in the opposite direction, because this is how the x and y of the picture and the x and y of the gabor kernel relate. This is the definition of the filter. Note that because the pixel are read at x0 + x, y0+y i never read out of the bounds of the picture. For example, say i'm going to compute the value for the pixel at 10,10 for a picture of size 40x40, then i'll read the pixels at 0,0 to 39,39 but they will be multiply by the corresponding values of G() at -10,29 to -10,29. With G() the gabor kernel function().

C) see B.

[Rick Brewster]

It's probably not being reached because you're not doing any bounds checking. surface[x,y] will throw an exception if x,y is out of bounds. It should come back to you as an error dialog, but depending on how you're doing things this might not happen. For example, I don't know offhand how this gets reported when working in CodeLab.

[I Like Pi]

Your loop is only reading pixels from x = 0 to x = rect.Width - 1 and y = 0 to y = rect.Height - 1 (substitute the inner loop bounds and you'll see that x0 and y0 cancel). Since rect is just an arbitrary rectangle PDN uses to divide up the image*, I think you need to revisit what your loop bounds should be.

* i.e. your program should work the same if all the rectangles were 1 x 1 or if there were only one rectangle representing the whole image.

[zarathoustra]

Right, thank you, i like pi!

Now it works! (although damn slow)

[Rick Brewster]

Now it works! (although damn slow)

Well that's to be expected ... normal minimum runtime will be O(W x H), but yours will be O((W x H)^2). I'm sure there's a numerical approximation, or at least a tricky but semantically equivalent optimization, for your filter that will greatly improve the runtime performance, but it may not be necessary given your needs.

[zarathoustra]

Right. By the way, i posted a bug report about how things go when i cancel my plugin, where i was stating it was running in quadratic time.

For the approximation, i'm not sure if there's one. I thought, although i need to scan the nearby pixels, i might not need to scan the all picture (maybe something depending on the wavelength).

But i don't need it now.

[Rick Brewster]

That's happening because your plugin is taking an impossibly long time to do anything at all. I'm trying it on a 1000x336 picture right now and it's just gobbling up CPU time and producing no results. This is on a quad-core system. The bug is in your plugin -- it's just way too slow to be usable. After 2 minutes I had to kill it. (this is on a Core 2 Quad Q6600 2.4 GHz -- nothing extraordinary but hardly slow)

[zarathoustra]

Yes, same thing here with a 800x600 picture on a quad core.

Seems to work fine up to 150x150.

I wondered if it were a bug in pdn or not because when i click cancel, the dialog disappear, and i thought pdn might have killed the threads running my plugin since there's no point anymore, but i you say it's ok on pdn's side...

[Rick Brewster]

It still has to wait for any remaining render code to stop. It doesn't throw them on the floor or do a Thread.Abort() (which would be disasterous).