I took a look at the svn history of unsharp-mask.c (I'm the original author) and saw that the fix for bug #166406 stopped using a lookup table called ctable. This was because it slowed things down a lot for large values of radius, due to cache overloading.
http://bugzilla.gnome.org/show_bug.cgi?id=166406

After the fix, the lookup table is no longer being used for the blurring algorithm, but it's still being generated and traversed during the blur, so I have a patch that removes all that useless code, without affecting functionality at all. What's the best way to submit it, as I can't commit to the SVN repository?

Also, I've read recently that a three-pass box blur is close to a true gaussian blur (within 3% when std_dev>2.0) so I'm considering implementing that for the unsharp mask. It should be much faster while still looking very good.

More info on the 3-pass box blur here: http://www.w3.org/TR/SVG/filters.html#feGaussianBlur The algorithm listed here, d = floor(s * 3*sqrt(2*pi)/4 + 0.5), doesn't have infinite precision; the smallest step (in standard deviations, AKA radius) it can take is 0.42 pixels, so I think maybe it would be best to use the triple-box-blur for values of s that are, say, 10 and higher, and use the true gaussian for smaller values. But if there's a good reason to use a different threshold value, please let me know.

Thoughts on all this?

Also, I've read recently that a three-pass box blur is close to a true gaussian blur (within 3% when std_dev>2.0) so I'm considering implementing that for the unsharp mask. It should be much faster while still looking very good.

More info on the 3-pass box blur here: http://www.w3.org/TR/SVG/filters.html#feGaussianBlur The algorithm listed here, d = floor(s * 3*sqrt(2*pi)/4 + 0.5), doesn't have infinite precision; the smallest step (in standard deviations, AKA radius) it can take is 0.42 pixels, so I think maybe it would be best to use the triple-box-blur for values of s that are, say, 10 and higher, and use the true gaussian for smaller values. But if there's a good reason to use a different threshold value, please let me know.

Thoughts on all this?

Well, I'll answer my own question... It works pretty well. I still have to run some more tests to make sure I don't have weird off-by-one errors or things like that.

Notes:
I used a threshold of 10 pixels to switch from the original gaussian method to the new boxblur method. The smallest steps with the new method is about .42, as mentioned in my previous post.

Speed: I used a timer that I had coded into the plugin ages ago. I tested this on my Core 2 Quad 2.4GHz. All these tests were run with amount=0.50 and threshold=0, on a 554x841 RGB image (a photograph), and there might be a little randomness in the results (on the order of hundredths of seconds). I compared the new triple-box-blur to the original true gaussian method.

radius=11 boxblur: .40s
gaussian: .65s

radius=24
boxblur .38s
gaussian 1.02s

radius=80
boxblur 0.42s
gaussian 2.30s

radius=105
boxblur 0.41s
gaussian 2.87s

The algorithm is close to constant time, with respect to radius. It should be O(n) with respect to the number of pixels.

Correctness: I did an unsharp mask with the new and old algorithms, overlaid them on top of eachother and used difference blending. Running the eyedropper over the image showed that the difference in each channel was between 0 and 3 for almost all pixels. I did see a handful of 4's. That's close enough for me...

I have some ideas about how to optimize the code a bit more using pointer arithmetic instead of indexing into arrays the usual way. I recall this having a big effect when I originally wrote the plugin a decade ago, but perhaps compilers have improved a lot since then, making this unnecessary. Anyone out there know about this?

Dang. I wish I knew that before I coded up the box blur...

Does this mean that I would have to code up a new plugin from scratch, or is there a GEGL version of it out there already? (Sorry, I'm totally unfamiliar with this GEGL stuff.)

-Winston

On Thu, Jan 15, 2009 at 1:43 AM, Martin Nordholts wrote:

Winston Chang wrote:

I took a look at the svn history of unsharp-mask.c (I'm the original author) and saw that the fix for bug #166406 stopped using a lookup table called ctable. This was because it slowed things down a lot for large values of radius, due to cache overloading. http://bugzilla.gnome.org/show_bug.cgi?id=166406

After the fix, the lookup table is no longer being used for the blurring algorithm, but it's still being generated and traversed during the blur, so I have a patch that removes all that useless code, without affecting functionality at all. What's the best way to submit it, as I can't commit to the SVN repository?

Hi!

I want to mention that since we are in the progress of migrating to GEGL we generally prefer that people make sure the GEGL counterpart works well rather than maintaining the legacy 8 bpc code. A patch for GEGL is much more attractive than a patch for an old GIMP plug-in.

That being said, the best way to submit patches is to open a bug report at bugs.gimp.org and attach it there.

BR, Martin

Hi!

I want to mention that since we are in the progress of migrating to GEGL we generally prefer that people make sure the GEGL counterpart works well rather than maintaining the legacy 8 bpc code. A patch for GEGL is much more attractive than a patch for an old GIMP plug-in.

There is no point in discouraging people from fixing / improving code that will be in use for at least one or two more releases, despite all GEGL goodness.

ciao, --mitch

OK - I'll submit the patches via bugzilla, then.

I've looked at the GEGL code now and played with it a bit, and I have to say it looks very nice and seems to be quite fast for gaussian blur and unsharp mask.

-Winston