Hija,

this is my suggested patch for getting the speeds improvements as mentioned in the other thread by having a thread-local PRNG initialized with a seed from the still existing blend tool local RNG.

It looks bigger than it is because I took the liberty to remove the nasty special casing on the existence of the RNG inside the innermost loop because we now have it outside already.

There seems to be more room for obvious optimisations in the loops. Also I would recommend splitting the two cases into two separate functions to make the code easier to read, and remove more conditionals.

PS: If this is okay, I'd do exactly the same for the gray blending stuff...

diff -u -r1.155 gimpdrawable-blend.c --- app/core/gimpdrawable-blend.c 26 Feb 2005 23:55:49 -0000 1.155
+++ app/core/gimpdrawable-blend.c 27 Feb 2005 14:55:36 -0000 @@ -1051,7 +1051,68 @@
gint endx = PR->x + PR->w;
gint endy = PR->y + PR->h;
gint x, y;
+
+ if (rbd->dither_rand)
+ {
+ GRand *localrand = g_rand_new_with_seed (g_rand_int (rbd->dither_rand));
+
+ for (y = PR->y; y < endy; y++) + {
+ for (x = PR->x; x < endx; x++) + {
+ GimpRGB color;
+ gdouble dither_prob;
+ gdouble ftmp;
+ gint itmp;
+
+ gradient_render_pixel (x, y, &color, rbd); +
+ ftmp = color.r * 255.0; + itmp = ftmp;
+ dither_prob = ftmp - itmp; +
+ if (g_rand_double (localrand) < dither_prob) + color.r += (1.0 / 255.0); +
+ ftmp = color.g * 255.0; + itmp = ftmp;
+ dither_prob = ftmp - itmp; +
+ if (g_rand_double (localrand) < dither_prob) + color.g += (1.0 / 255.0); +
+ ftmp = color.b * 255.0; + itmp = ftmp;
+ dither_prob = ftmp - itmp;

+ if (g_rand_double (localrand) < dither_prob) + color.b += (1.0 / 255.0); +
+ ftmp = color.a * 255.0; + itmp = ftmp;
+ dither_prob = ftmp - itmp; +
+ if (g_rand_double (localrand) < dither_prob) + color.a += (1.0 / 255.0); +
+ if (color.r > 1.0) color.r = 1.0; + if (color.g > 1.0) color.g = 1.0; + if (color.b > 1.0) color.b = 1.0; + if (color.a > 1.0) color.a = 1.0; +
+ *dest++ = color.r * 255.0; + *dest++ = color.g * 255.0; + *dest++ = color.b * 255.0; + *dest++ = color.a * 255.0; + }
+ }
+
+ g_rand_free (localrand);
+
+ return;
+ }
+
+ /* Fall-through for non-dithering case */ for (y = PR->y; y < endy; y++)
{
for (x = PR->x; x < endx; x++) @@ -1059,46 +1120,6 @@
GimpRGB color;

gradient_render_pixel (x, y, &color, rbd); -
- if (rbd->dither_rand)
- {
- gdouble dither_prob;
- gdouble ftmp;
- gint itmp;
-
- ftmp = color.r * 255.0; - itmp = ftmp;
- dither_prob = ftmp - itmp; -
- if (g_rand_double (rbd->dither_rand) < dither_prob) - color.r += (1.0 / 255.0); -
- ftmp = color.g * 255.0; - itmp = ftmp;
- dither_prob = ftmp - itmp; -
- if (g_rand_double (rbd->dither_rand) < dither_prob) - color.g += (1.0 / 255.0); -
- ftmp = color.b * 255.0; - itmp = ftmp;
- dither_prob = ftmp - itmp; -
- if (g_rand_double (rbd->dither_rand) < dither_prob) - color.b += (1.0 / 255.0); -
- ftmp = color.a * 255.0; - itmp = ftmp;
- dither_prob = ftmp - itmp; -
- if (g_rand_double (rbd->dither_rand) < dither_prob) - color.a += (1.0 / 255.0); -
- if (color.r > 1.0) color.r = 1.0; - if (color.g > 1.0) color.g = 1.0; - if (color.b > 1.0) color.b = 1.0; - if (color.a > 1.0) color.a = 1.0; - }

*dest++ = color.r * 255.0; *dest++ = color.g * 255.0;

Servus, Daniel

On 27.02.2005, at 16:42, Robert L Krawitz wrote:

In addition to being very slow, this will also yield noisy results. There are a lot of dither algorithms that are both much faster and yield better results. While you may not need full-blown Floyd-Steinberg or EvenTone dithering for this purpose (and they're hard to use in a multi-threaded fashion), a simple dither matrix is fast, free of most artifacts, trivial to parallelize (you're only reading from the matrix, so no serialization is necessary), and reasonably low noise.

This is just the first step to get better performance. In addition to the patch posted, I've already done and tested the separation of the dither and non-dither blending code, so it's a bit easier to simply change the algorithm from a randomized dither to a different one.

If we also do the change to remove the local PRNGs with a global one which can be used to get a smaller number of pseudo-random numbers or to setup and seed a thread-local PRNG, the only needed change for your proposal would be to touch two or three (one for the setup) functions.

Let me know if you're interested.

Honestly I've no idea why the blending code does dithering at all; the dithering is completely invisible in 24bit RGB colorspace anyway.

But if you can speed that up; be my guest... ;)

Servus, Daniel

On 27.02.2005, at 20:01, Robert L Krawitz wrote:

OK. The code Daniel posted shouldn't be too hard to convert. The only thing it needs is to have the absolute row and column, to index into the matrix.

That hasn't made it into CVS because Sven was faster...

Here's the current code: static void
gradient_fill_single_region_rgb_dither (RenderBlendData *rbd, PixelRegion *PR) {
GRand *dither_rand = g_rand_new_with_seed (g_rand_int (rbd->seed)); guchar *dest = PR->data;
gint endx = PR->x + PR->w; gint endy = PR->y + PR->h; gint x, y;

for (y = PR->y; y < endy; y++) for (x = PR->x; x < endx; x++)
{
GimpRGB color;

gradient_render_pixel (x, y, &color, rbd);

gradient_dither (dest, dither_rand, color.r); gradient_dither (dest, dither_rand, color.g); gradient_dither (dest, dither_rand, color.b); gradient_dither (dest, dither_rand, color.a); }

g_rand_free (dither_rand); }

If needed you can setup your LUT globally or create it on demand and have a pointer in RenderBlendData, depending on how big and/or complex it is to compute and replace the inner loop by your code.

Would it make sense to have several (selectable) dither algorithms?

Will I be able to compile it against a current (stable) GIMP installation?

Huh?

Servus,
Daniel