nanogui: Thread: [patch] text (and bitmap) speedups

the following patch gives close to 60% speedup for one of my text draw
benchmarks.  my 25MHz 68k target with SED1355 video controller (8bpp
dumb framebuffer) was taking over 1.2ms per character.  for one
benchmark I was able to speed that up by 60%, to under 0.5ms per
character.

there are three parts:

* eliminate clipping checks in GdBitmap():

the "size considerations" noted under the CLIP_VISIBLE case were heavily
outweighed by the slowness of per-point clipping checks.  this change
alone gave me over 20% speedup.

* amortize the background draw in corefont_drawtext():

again, the "size considerations" excuse in the CLIP_VISIBLE case.  if
there are a lot of characters to draw, we can use FillRect for the whole
area under the block of characters we're drawing instead of clearing one
rectangle per character.  this gave a cumulative 59% speedup with the
above change.

* use a zero-copy for character data in gen_gettextsize():

probably not for everybody... we can avoid having to copy font data if
we let gen_gettextsize set a pointer for us.  this gave a cumulative 60%
speedup with the above two changes.  not as dramatic, but why copy data
if you can avoid it?

long term my goal is to implement hardware-specific psd->DrawBitmap and
psd->DrawText, but the above changes should be fairly portable.

comments, criticisms, and ideas are welcome.

-- 
  Aaron J. Grier  |   Frye Electronics, Tigard, OR   |  ####@####.####

Index: 0-89.2/src/engine/devdraw.c
--- 0-89.2/src/engine/devdraw.c Wed, 12 Feb 2003 11:42:54 -0800 aaron (microwin/g/6_devdraw.c 1.11 644)
+++ 0-89pre8.3(w)/src/engine/devdraw.c Thu, 13 Feb 2003 14:02:38 -0800 aaron (microwin/g/6_devdraw.c 1.11 644)
@@ -478,7 +478,7 @@ GdFillRect(PSD psd, MWCOORD x1, MWCOORD 
  */
 void
 GdBitmap(PSD psd, MWCOORD x, MWCOORD y, MWCOORD width, MWCOORD height,
-	MWIMAGEBITS *imagebits)
+	const MWIMAGEBITS *imagebits)
 {
   MWCOORD minx;
   MWCOORD maxx;
@@ -488,16 +488,39 @@ GdBitmap(PSD psd, MWCOORD x, MWCOORD y, 
 
   switch (GdClipArea(psd, x, y, x + width - 1, y + height - 1)) {
       case CLIP_VISIBLE:
-	/*
-	 * For size considerations, there's no low-level bitmap
-	 * draw so we've got to draw everything with per-point
-	 * clipping for the time being.
+	/* "size considerations" be damned.  per-point clipping is
+	 * godawful slow. */
 	if (gr_usebg)
 		psd->FillRect(psd, x, y, x + width - 1, y + height - 1,
 			gr_background);
+	/* FIXME think of the speedups if this existed...
 	psd->DrawBitmap(psd, x, y, width, height, imagebits, gr_foreground);
 	return;
 	*/
+
+	minx = x;
+	maxx = x + width - 1;
+	bitcount = 0;
+	while (height > 0) {
+	  if (bitcount <= 0) {
+	    bitcount = MWIMAGE_BITSPERIMAGE;
+	    bitvalue = *imagebits++;
+	  }
+	  /* look, ma, no clipping checks! */
+	  if (MWIMAGE_TESTBIT(bitvalue))
+	    psd->DrawPixel(psd, x, y, gr_foreground);
+	  bitvalue = MWIMAGE_SHIFTBIT(bitvalue);
+	  bitcount--;
+	  if (x++ == maxx) {
+	    x = minx;
+	    y++;
+	    height--;
+	    bitcount = 0;
+	  }
+	}
+	GdFixCursor(psd);
+	return;
+	
 	break;
 
       case CLIP_INVISIBLE:
Index: 0-89.2/src/drivers/genfont.h
--- 0-89.2/src/drivers/genfont.h Wed, 12 Feb 2003 11:42:54 -0800 aaron (microwin/k/27_genfont.h 1.8 644)
+++ 0-89pre8.3(w)/src/drivers/genfont.h Thu, 13 Feb 2003 14:01:09 -0800 aaron (microwin/k/27_genfont.h 1.8 644)
@@ -8,13 +8,13 @@
  * These routines are screen driver entry points.
  */
 
 #define NUMBER_FONTS	4	/* number of compiled-in fonts*/
 
 /* entry points*/
 MWBOOL	gen_getfontinfo(PMWFONT pfont, PMWFONTINFO pfontinfo);
 void	gen_gettextsize(PMWFONT pfont, const void *text, int cc,
 		MWCOORD *pwidth, MWCOORD *pheight, MWCOORD *pbase);
-void	gen_gettextbits(PMWFONT pfont, int ch, MWIMAGEBITS *retmap,
+void	gen_gettextbits(PMWFONT pfont, int ch, const MWIMAGEBITS **retmap,
 		MWCOORD *pwidth, MWCOORD *pheight, MWCOORD *pbase);
 void	gen_unloadfont(PMWFONT pfont);
 
Index: 0-89.2/src/drivers/genfont.c
--- 0-89.2/src/drivers/genfont.c Wed, 12 Feb 2003 11:42:54 -0800 aaron (microwin/k/30_genfont.c 1.14 644)
+++ 0-89pre8.3(w)/src/drivers/genfont.c Thu, 13 Feb 2003 14:00:53 -0800 aaron (microwin/k/30_genfont.c 1.14 644)
@@ -163,12 +186,12 @@ gen_gettextsize(PMWFONT pfont, const voi
  * with a character.  Handles fixed and proportional fonts.
  */
 void
-gen_gettextbits(PMWFONT pfont, int ch, MWIMAGEBITS *retmap,
+gen_gettextbits(PMWFONT pfont, int ch, const MWIMAGEBITS **retmap,
 	MWCOORD *pwidth, MWCOORD *pheight, MWCOORD *pbase)
 {
 	PMWCFONT	pf = ((PMWCOREFONT)pfont)->cfont;
-	int 		n, count, width;
-	MWIMAGEBITS *	bits;
+	int 		count, width;
+	const MWIMAGEBITS *	bits;
 
 #if HAVE_BIG5_SUPPORT
 	/* decode chinese big5*/
@@ -337,9 +360,7 @@ gen_gettextbits(PMWFONT pfont, int ch, M
  	width = pf->width ? pf->width[ch] : pf->maxwidth;
 	count = MWIMAGE_WORDS(width) * pf->height; 
 
-	for(n=0; n<count; ++n) {
-	  *retmap++ = *bits++;
-	}
+	*retmap = bits;
 
 	/* return width depending on fixed pitch or not*/
 	*pwidth = width; 
Index: 0-89.2/src/include/device.h
--- 0-89.2/src/include/device.h Wed, 12 Feb 2003 11:42:54 -0800 aaron (microwin/m/18_device.h 1.9 644)
+++ 0-89pre8.3(w)/src/include/device.h Thu, 13 Feb 2003 14:02:49 -0800 aaron (microwin/m/18_device.h 1.9 644)
@@ -50,7 +50,7 @@ typedef struct {
 	MWBOOL	(*GetFontInfo)(PMWFONT pfont, PMWFONTINFO pfontinfo);
 	void 	(*GetTextSize)(PMWFONT pfont, const void *text, int cc,
 			MWCOORD *pwidth, MWCOORD *pheight, MWCOORD *pbase);
-	void	(*GetTextBits)(PMWFONT pfont, int ch, MWIMAGEBITS *retmap,
+	void	(*GetTextBits)(PMWFONT pfont, int ch, const MWIMAGEBITS **retmap,
 			MWCOORD *pwidth, MWCOORD *pheight, MWCOORD *pbase);
 	void	(*DestroyFont)(PMWFONT pfont);
 	void	(*DrawText)(PMWFONT pfont, PSD psd, MWCOORD x, MWCOORD y,
@@ -442,7 +442,7 @@ void	GdLine(PSD psd,MWCOORD x1,MWCOORD y
 void	GdRect(PSD psd,MWCOORD x, MWCOORD y, MWCOORD width, MWCOORD height);
 void	GdFillRect(PSD psd,MWCOORD x, MWCOORD y, MWCOORD width, MWCOORD height);
 void	GdBitmap(PSD psd,MWCOORD x,MWCOORD y,MWCOORD width,MWCOORD height,
-		MWIMAGEBITS *imagebits);
+		const MWIMAGEBITS *imagebits);
 MWBOOL	GdColorInPalette(MWCOLORVAL cr,MWPALENTRY *palette,int palsize);
 void	GdMakePaletteConversionTable(PSD psd,MWPALENTRY *palette,int palsize,
 		MWPIXELVAL *convtable,int fLoadType);
Index: 0-89.2/src/include/nano-X.h
--- 0-89.2/src/include/nano-X.h Wed, 12 Feb 2003 11:42:54 -0800 aaron (microwin/m/19_nano-X.h 1.9 644)
+++ 0-89pre8.3(w)/src/include/nano-X.h Wed, 12 Feb 2003 17:01:14 -0800 aaron (microwin/m/19_nano-X.h 1.9 644)
@@ -628,7 +629,7 @@ void		GrReparentWindow(GR_WINDOW_ID wid,
 void		GrGetWindowInfo(GR_WINDOW_ID wid, GR_WINDOW_INFO *infoptr);
 void		GrSetWMProperties(GR_WINDOW_ID wid, GR_WM_PROPERTIES *props);
 void		GrGetWMProperties(GR_WINDOW_ID wid, GR_WM_PROPERTIES *props);
-GR_FONT_ID	GrCreateFont(GR_CHAR *name, GR_COORD height,
+GR_FONT_ID	GrCreateFont(const GR_CHAR *name, GR_COORD height,
 			GR_LOGFONT *plogfont);
 void		GrGetFontList(GR_FONTLIST ***fonts, int *numfonts);
 void		GrFreeFontList(GR_FONTLIST ***fonts, int num);
Index: 0-89.2/src/engine/devfont.c
--- 0-89.2/src/engine/devfont.c Wed, 12 Feb 2003 11:42:54 -0800 aaron (microwin/B/38_devfont.c 1.1 644)
+++ 0-89pre8.3(w)/src/engine/devfont.c Thu, 13 Feb 2003 14:01:58 -0800 aaron (microwin/B/38_devfont.c 1.1 644)
@@ -11,6 +11,7 @@
  */
 /*#define NDEBUG*/
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include "device.h"
 #include "devfont.h"
@@ -302,8 +303,8 @@ corefont_drawtext(PMWFONT pfont, PSD psd
 	MWCOORD 	height;			/* height of text area */
 	MWCOORD		base;			/* baseline of text*/
 	MWCOORD		startx, starty;
-						/* bitmap for characters */
-	MWIMAGEBITS bitmap[MAX_CHAR_HEIGHT*MAX_CHAR_WIDTH/MWIMAGE_BITSPERIMAGE];
+	const MWIMAGEBITS	*bitmap;	/* bitmap for characters */
+	MWBOOL		bgstate;
 
 	pfont->fontprocs->GetTextSize(pfont, str, cc, &width, &height, &base);
 	
@@ -316,17 +317,45 @@ corefont_drawtext(PMWFONT pfont, PSD psd
 
 	switch (GdClipArea(psd, x, y, x + width - 1, y + height - 1)) {
 	case CLIP_VISIBLE:
-		/*
-		 * For size considerations, there's no low-level text
-		 * draw, so we've got to draw all text
-		 * with per-point clipping for the time being
 		if (gr_usebg)
 			psd->FillRect(psd, x, y, x + width - 1, y + height - 1,
 				gr_background);
+		/* FIXME if we had a low-level text drawer, it would
+		 * plug in here:
 		psd->DrawText(psd, x, y, str, cc, gr_foreground, pfont);
-		GdFixCursor(psd);
 		return;
 		*/
+
+		/* already cleared out the background; no need to do it
+		 * further */
+		bgstate = gr_usebg;
+		gr_usebg = FALSE;
+
+		/* don't worry about clipping */
+		while (--cc >= 0) {
+			unsigned int ch = *str++;
+			pfont->fontprocs->GetTextBits(pfont, ch, &bitmap,
+				&width, &height, &base);
+#if defined(HAVE_BIG5_SUPPORT) | defined(HAVE_GB2312_SUPPORT)
+#error FIXME someone fill me in please
+#endif
+			/* FIXME still have a per-character clipping
+			 * check for calling this.  a low-level bitmap
+			 * routine would mitigate this.  */
+			GdBitmap(psd, x, y, width, height, bitmap);
+
+			x += width;
+		}
+
+		/* underline if we need to */
+		if (pfont->fontattr & MWTF_UNDERLINE)
+			GdLine(psd, startx, starty, x, starty, FALSE);
+
+		/* restore state */
+		gr_usebg = bgstate;
+		GdFixCursor(psd);
+
+		return;
 		break;
 
 	case CLIP_INVISIBLE:
@@ -386,7 +415,7 @@ corefont_drawtext(PMWFONT pfont, PSD psd
                                 --cc;
                 }
 #endif
-		pfont->fontprocs->GetTextBits(pfont, ch, bitmap, &width,
+		pfont->fontprocs->GetTextBits(pfont, ch, &bitmap, &width,
 			&height, &base);
 
 		/* note: change to bitmap*/

Hi,

>long term my goal is to implement hardware-specific psd->DrawBitmap and
>psd->DrawText, but the above changes should be fairly portable.

I've sent Greg patches to implement hardware-specific text drawing.  I
used the psd->DrawArea entrypoint, but it's the same idea.  I did this
because I wanted *real* anti-aliased text - i.e. drawing using an 8-bit
alpha map.  Without hardware-specific code, performance would seriously
suck.  While I was at it, I also implemented hardware-specific support
for drawing mono bitmaps.  If you want my patches, just ask.

Kind regards,

Jon Foster
--