LocalHistogramEffect.cs

[spectorius]

                int maxArea = GetMaxAreaForRadius(rad);

Hi! What is the point of this line??? Var maxArea is not used anywhere.

file: LocalHistogramEffect.cs

line: 161; function: RenderRect

And, BTW, what is minus in:

                int top = -Math.Min(rad, y);

-1 * Math.Min ?

Thanks! 8)

[pyrochild]

                int maxArea = GetMaxAreaForRadius(rad);

Hi! What is the point of this line???

If it's not used then it's either there from past or for future use.

And, BTW, what is minus in:

                int top = -Math.Min(rad, y);

-1 * Math.Min ?

Just a plain old negative sign...

[spectorius]

[code]                int top = -Math.Min(rad, y);

-1 * Math.Min ?

Just a plain old negative sign...

So, It`s mean -1 * Math.Min() ?

[pyrochild]

That's what negative signs tend to mean, yes.

[Rick Brewster]

This is already fixed in the 3.30 code base. That line of code does impact performance, but otherwise causes no harm.

[spectorius]

        public static byte ClampToByte(double x) 
       {
           if (x > 255)
           {
               return 255;
           }
           else if (x < 0)
           {
               return 0;
           }
           else
           {
               return (byte)x;
           }
       }

another thing. i think the next code is some better :wink:

public static byte ClampToByte(double x) 
{
       return (byte)Math.Min(Math.Max(x, 0), 255);
}

That line of code does impact performance, but otherwise causes no harm.

Why this line was there? Is it a part of the algorithm ?

ANd, where i can read about used algoritm for Reduce Noise Effect. Thanks :wink:

[PhilipLB]

I think the original code is more performant.

[Rick Brewster]

Regarding ClampToByte, correct. There is really no benefit to nesting calls to Math.Min/Math.Max, although it may be more succinct that way. The benefit is in having the ClampToByte method in the first place, and there isn't much to be had by changing the way it's implemented.

[spectorius]

Actually, by performance it is the same, cause behind max and min there are same if else.

[Rick Brewster]

Right but those are function calls, which may or may not be inlined.

The performance will be "the same or worse." In any case it doesn't really matter, we could creatively debate this all day

[spectorius]

we could creatively debate this all day

my time is over midnight already (1:18 am)

                    if (intensity + this.brightness < 128)
                   {
                       this.rgbTable[intensity] = 0;
                   }
                   else
                   {
                       this.rgbTable[intensity] = 255;
                   }

into

this.rgbTable[intensity] = (intensity + this.brightness < 128) ? 0 : 255;

p.s. i like simplicity very match

[pyrochild]

fewer lines and simplicity are not the same thing.

Your code is harder to read.

[Ed Harvey]

FYI, for those of us interested in perfomance.

The current implementation of the JIT/NGEN compilers will not inline methods with value type arguments, local variables, or return values.

[Rick Brewster]

I'm not sure if that applies to something like Int32 or Byte though.

In any case, w/ .NET 3.5 SP1 + .NET 2.0 SP2 it will all be fixed. Should provide a very healthy performance boost for Paint.NET, as just inlining methods involving the ColorBgra class will be a huge benefit.

[spectorius]

fewer lines and simplicity are not the same thing.

Your code is harder to read.

For me it is easy

I ported Reduce Noise and Brightness & Contrast Adjustment effects to PHP, now trying Pencil Sketch

[Ed Harvey]

I'm not sure if that applies to something like Int32 or Byte though.

No, 'built-in' types such as integer and floating point numbers are exempt from this limitation. Interestingly however, the System.Math methods are not inlined. I'm not 100% sure why, perhaps because they are ngen'd or in the CAG ?

In any case, w/ .NET 3.5 SP1 + .NET 2.0 SP2 it will all be fixed.

Cool. Last I heard they weren't due to fix that until the next version bump, so if it'll get into the SP that will be excellent... 8)

Should provide a very healthy performance boost for Paint.NET, as just inlining methods involving the ColorBgra class will be a huge benefit.

Certainly will help (at least for the smaller, more commonly used, methods)

[Rick Brewster]

I'm not sure if that applies to something like Int32 or Byte though.

No, 'built-in' types such as integer and floating point numbers are exempt from this limitation. Interestingly however, the System.Math methods are not inlined. I'm not 100% sure why, perhaps because they are ngen'd or in the CAG ?

(I assume by CAG that you mean GAC )

I know that, at least for 64-bit, there was an issue with some of the Math methods where performance was grossly lower than it should have been. I'm not sure if that's been fixed or if it's slated for the 3.5 SP1 / 2.0 SP2 release.

I've always meant to experiment with isolating some of the Render() functions into a C DLL and P/Invoking them, but I just never get around to it. Would be fun to try out SSE4.2 optimization as well; I guess I've bet more on multithreading for performance than on assembly language. I think I made the right cost trade-off... here comes Intel's Nehalem with 8-cores on the desktop for Q4 2008