Re: Trivial patch reducing fp mults in pango-cairo

[Behdad Esfahbod <behdad behdad org>]

On Wed, 2006-12-13 at 15:20 -0800, Daniel Amelang wrote:
> On 12/13/06, Behdad Esfahbod <behdad behdad org> wrote:
> > Well, this is kinda hitting the limit.  You are basically rewriting soft
> > float routines.  First, I'm not sure it's much faster (ok, you can skip
> > some details, so it's got to be faster), second, you are mostly shifting
> > time from __mul to library functions.  I'll rather leave these to the
> > compiler.  Has anyone tested compiling recent pango+cairo with
> > softfloats on small systems?
> 
> I'm going to guess that you haven't looked over the softfloat source
> code very carefully :)

I've not :).

>  What I'm proposing is so much simpiler, and
> will pipeline so much better that to say that I'm "basically rewriting
> soft float routines" is a stretch. This is pretty similar to what I
> did with cairo_lround, and I saw a 5x speedup on ARM for that function
> alone after I converted it to use an approach similar to the one
> above.

Right, but that was not compared to softfloat, was it?

>  Usually, you get a bunch of simple integer instructions w/ few
> little branches, if any, which is really fast on most systems. Either
> way, we can't say for sure until someone codes it up :)
> 
> > > Once that is done, pangocairo should be pretty much FP free for the
> > > typical code paths that I would expect to see on the 770. On
> > > timetext.c or the torturer's GtkTextView, I don't think you'll see
> > > _that_ much improvement (percentage-wise) from this change until you
> > > get Xan's XRender glyph optimization into cairo, as that is a bigger
> > > bottleneck ATM, I think.
> >
> > Yeah, if you compare the overall profiles with pangocairo ones,
> > pangocairo is taking like less than 5% of the time (possibly much less).
> > Nothing to be gained here.
> 
> Here, I totally agree with you. This is why I haven't bother to code
> it up yet. But since Jorn was looking into eliminating FP from
> pangocairo, I thought I'd share what I think is the best way to do so,
> given that's what you want.

I went on and coded my two ideas however.  Slightly improves performance
on my laptop, but hardly measurable.  Attaching to see if they are worth
committing.

> Dan
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759


Index: pango/pangocairo-render.c
===================================================================
RCS file: /cvs/gnome/pango/pango/pangocairo-render.c,v
retrieving revision 1.32
diff -u -p -d -r1.32 pangocairo-render.c
--- pango/pangocairo-render.c	4 Dec 2006 20:23:35 -0000	1.32
+++ pango/pangocairo-render.c	13 Dec 2006 22:52:53 -0000
@@ -196,6 +196,8 @@ pango_cairo_renderer_draw_glyphs (PangoR
   int x_position = 0;
   cairo_glyph_t *cairo_glyphs;
   cairo_glyph_t stack_glyphs[MAX_STACK];
+  double base_x = crenderer->x_offset + (double)x / PANGO_SCALE;
+  double base_y = crenderer->y_offset + (double)y / PANGO_SCALE;
 
   cairo_save (crenderer->cr);
   if (!crenderer->do_path)
@@ -209,10 +211,10 @@ pango_cairo_renderer_draw_glyphs (PangoR
 
 	  if (gi->glyph != PANGO_GLYPH_EMPTY)
 	    {
-	      double cx = crenderer->x_offset +
-			  (double)(x + x_position + gi->geometry.x_offset) / PANGO_SCALE;
-	      double cy = crenderer->y_offset +
-			  (double)(y + gi->geometry.y_offset) / PANGO_SCALE;
+	      double cx = base_x + (double)(x_position + gi->geometry.x_offset) / PANGO_SCALE;
+	      double cy = gi->geometry.y_offset == 0 ?
+			  base_y :
+			  base_y + (double)(gi->geometry.y_offset) / PANGO_SCALE;
 
 	      _pango_cairo_renderer_draw_unknown_glyph (crenderer, font, gi, cx, cy);
 	    }	  
@@ -234,10 +236,10 @@ pango_cairo_renderer_draw_glyphs (PangoR
 
       if (gi->glyph != PANGO_GLYPH_EMPTY)
         {
-          double cx = crenderer->x_offset +
-		      (double)(x + x_position + gi->geometry.x_offset) / PANGO_SCALE;
-          double cy = crenderer->y_offset +
-		      (double)(y + gi->geometry.y_offset) / PANGO_SCALE;
+          double cx = base_x + (double)(x_position + gi->geometry.x_offset) / PANGO_SCALE;
+          double cy = gi->geometry.y_offset == 0 ?
+		      base_y :
+		      base_y + (double)(gi->geometry.y_offset) / PANGO_SCALE;
 
           if (gi->glyph & PANGO_GLYPH_UNKNOWN_FLAG)
 	    _pango_cairo_renderer_draw_unknown_glyph (crenderer, font, gi, cx, cy);

Index: pango/pangocairo-fcfont.c
===================================================================
RCS file: /cvs/gnome/pango/pango/pangocairo-fcfont.c,v
retrieving revision 1.46
diff -u -p -d -r1.46 pangocairo-fcfont.c
--- pango/pangocairo-fcfont.c	28 Nov 2006 21:17:39 -0000	1.46
+++ pango/pangocairo-fcfont.c	13 Dec 2006 22:57:48 -0000
@@ -46,8 +46,6 @@ typedef struct _GlyphExtentsCacheEntry  
 #define GLYPH_CACHE_NUM_ENTRIES 256 /* should be power of two */
 #define GLYPH_CACHE_MASK (GLYPH_CACHE_NUM_ENTRIES - 1)
 
-#define PANGO_UNITS(Double) ((int)floor((Double) * PANGO_SCALE + 0.5))
-
 /* An entry in the fixed-size cache for the glyph -> ink_rect mapping.
  * The cache is indexed by the lower N bits of the glyph (see
  * GLYPH_CACHE_NUM_ENTRIES).  For scripts with few glyphs,
Index: pango/pangocairo-private.h
===================================================================
RCS file: /cvs/gnome/pango/pango/pangocairo-private.h,v
retrieving revision 1.13
diff -u -p -d -r1.13 pangocairo-private.h
--- pango/pangocairo-private.h	28 Nov 2006 21:47:51 -0000	1.13
+++ pango/pangocairo-private.h	13 Dec 2006 22:57:48 -0000
@@ -113,6 +113,8 @@ struct _PangoCairoWarningHistory {
 
 extern PangoCairoWarningHistory _pango_cairo_warning_history;
 
+#define PANGO_UNITS(Double) ((int)floor((Double) * PANGO_SCALE + 0.5))
+
 G_END_DECLS
 
 #endif /* __PANGOCAIRO_PRIVATE_H__ */
Index: pango/pangocairo-render.c
===================================================================
RCS file: /cvs/gnome/pango/pango/pangocairo-render.c,v
retrieving revision 1.32
diff -u -p -d -r1.32 pangocairo-render.c
--- pango/pangocairo-render.c	4 Dec 2006 20:23:35 -0000	1.32
+++ pango/pangocairo-render.c	13 Dec 2006 22:57:48 -0000
@@ -196,6 +196,9 @@ pango_cairo_renderer_draw_glyphs (PangoR
   int x_position = 0;
   cairo_glyph_t *cairo_glyphs;
   cairo_glyph_t stack_glyphs[MAX_STACK];
+  double base_x = crenderer->x_offset + (double)x / PANGO_SCALE;
+  double base_y = crenderer->y_offset + (double)y / PANGO_SCALE;
+  int fixed_x = PANGO_UNITS (base_x);
 
   cairo_save (crenderer->cr);
   if (!crenderer->do_path)
@@ -209,10 +212,10 @@ pango_cairo_renderer_draw_glyphs (PangoR
 
 	  if (gi->glyph != PANGO_GLYPH_EMPTY)
 	    {
-	      double cx = crenderer->x_offset +
-			  (double)(x + x_position + gi->geometry.x_offset) / PANGO_SCALE;
-	      double cy = crenderer->y_offset +
-			  (double)(y + gi->geometry.y_offset) / PANGO_SCALE;
+	      double cx = (double)(fixed_x + x_position + gi->geometry.x_offset) / PANGO_SCALE;
+	      double cy = gi->geometry.y_offset == 0 ?
+			  base_y :
+			  base_y + (double)(gi->geometry.y_offset) / PANGO_SCALE;
 
 	      _pango_cairo_renderer_draw_unknown_glyph (crenderer, font, gi, cx, cy);
 	    }	  
@@ -234,10 +237,10 @@ pango_cairo_renderer_draw_glyphs (PangoR
 
       if (gi->glyph != PANGO_GLYPH_EMPTY)
         {
-          double cx = crenderer->x_offset +
-		      (double)(x + x_position + gi->geometry.x_offset) / PANGO_SCALE;
-          double cy = crenderer->y_offset +
-		      (double)(y + gi->geometry.y_offset) / PANGO_SCALE;
+          double cx = (double)(fixed_x + x_position + gi->geometry.x_offset) / PANGO_SCALE;
+          double cy = gi->geometry.y_offset == 0 ?
+		      base_y :
+		      base_y + (double)(gi->geometry.y_offset) / PANGO_SCALE;
 
           if (gi->glyph & PANGO_GLYPH_UNKNOWN_FLAG)
 	    _pango_cairo_renderer_draw_unknown_glyph (crenderer, font, gi, cx, cy);