[Pixman] [PATCH 0/5] Improved precision in gradient walker

Søren Sandmann sandmann at cs.au.dk
Fri Mar 15 22:30:41 PDT 2013


There were some aspects of this series I wasn't happy about, so here is
a new version. Changes are mostly superficial and described in "V2"
annotations in the commit messages. Of note is that with the new
patches, the code is now actually than before.

While updating the patch series I ran into a mystery that I haven't been
able to solve. I wanted to also include the patch below which
consolidates all the scaling of color channels in one place. It's a
straightforward cleanup that, if anything, should be a tiny speedup.

But it isn't. It's a substantial slowdown (about 10%) on a Sandy Bridge
i3 when compiled with gcc 4.7 and I have no idea why. There is nothing
particularly strange about the assembly generated; only one division is
generated in both cases, and the number of multiplications is about the
same. There is just no reason that I can see that there should be any
significant difference. If anyone can figure this one out, I'd be
grateful.

The patch:

diff --git a/pixman/pixman-gradient-walker.c b/pixman/pixman-gradient-walker.c
index 5944a55..3a950ca 100644
--- a/pixman/pixman-gradient-walker.c
+++ b/pixman/pixman-gradient-walker.c
@@ -128,14 +128,14 @@ gradient_walker_reset (pixman_gradient_walker_t *walker,
      * be in the [0, 255] interval.
      */
     la = (left_c->alpha * (1.0f/257.0f));
-    lr = (left_c->red * (1.0f/257.0f));
-    lg = (left_c->green * (1.0f/257.0f));
-    lb = (left_c->blue * (1.0f/257.0f));
+    lr = (left_c->red * (1.0f/65535.0f));
+    lg = (left_c->green * (1.0f/65535.0f));
+    lb = (left_c->blue * (1.0f/65535.0f));
 
     ra = (right_c->alpha * (1.0f/257.0f));
-    rr = (right_c->red * (1.0f/257.0f));
-    rg = (right_c->green * (1.0f/257.0f));
-    rb = (right_c->blue * (1.0f/257.0f));
+    rr = (right_c->red * (1.0f/65535.0f));
+    rg = (right_c->green * (1.0f/65535.0f));
+    rb = (right_c->blue * (1.0f/65535.0f));
     
     lx = left_x * (1.0f/65536.0f);
     rx = right_x * (1.0f/65536.0f);
@@ -144,23 +144,23 @@ gradient_walker_reset (pixman_gradient_walker_t *walker,
     {
 	walker->a_s = walker->r_s = walker->g_s = walker->b_s = 0.0f;
 	walker->a_b = (la + ra) / 2.0f;
-	walker->r_b = (lr + rr) / 510.0f;
-	walker->g_b = (lg + rg) / 510.0f;
-	walker->b_b = (lb + rb) / 510.0f;
+	walker->r_b = (lr + rr) / 2.0f;
+	walker->g_b = (lg + rg) / 2.0f;
+	walker->b_b = (lb + rb) / 2.0f;
     }
     else
     {
 	float w_rec = 1.0f / (rx - lx);
 
 	walker->a_b = (la * rx - ra * lx) * w_rec;
-	walker->r_b = (lr * rx - rr * lx) * w_rec * (1.0f/255.0f);
-	walker->g_b = (lg * rx - rg * lx) * w_rec * (1.0f/255.0f);
-	walker->b_b = (lb * rx - rb * lx) * w_rec * (1.0f/255.0f);
+	walker->r_b = (lr * rx - rr * lx) * w_rec;
+	walker->g_b = (lg * rx - rg * lx) * w_rec;
+	walker->b_b = (lb * rx - rb * lx) * w_rec;
 
 	walker->a_s = (ra - la) * w_rec;
-	walker->r_s = (rr - lr) * w_rec * (1.0f/255.0f);
-	walker->g_s = (rg - lg) * w_rec * (1.0f/255.0f);
-	walker->b_s = (rb - lb) * w_rec * (1.0f/255.0f);
+	walker->r_s = (rr - lr) * w_rec;
+	walker->g_s = (rg - lg) * w_rec;
+	walker->b_s = (rb - lb) * w_rec;
     }
    
     walker->left_x = left_x;

Also available here:

     http://www.daimi.au.dk/~sandmann/multiplications.patch


Søren


More information about the Pixman mailing list