8:28 a.m.
Implementing one of the todo items from the redisplay-x.c code, the
following patch keeps an XftDraw structure in the X frame object, and
uses that for all drawing related to that frame.
The Xft library uses this structure for, among other things, keeping a
cache of rendered characters in the server. Retaining the object means
that they stick around, rather than being recreated every time text
needs to be drawn.
This should improve performance everywhere, but most significantly on
remote X sessions by avoiding a huge number of X requests to create and
destroy the pixmaps associated with the frame.
Daniel
5:08 a.m.
>>>> "Daniel" == Daniel Pittman
<daniel(a)rimspace.net> writes:
Daniel> The Xft library uses this structure for, among other
Daniel> things, keeping a cache of rendered characters in the
Daniel> server.
I'd been wondering about that; it takes forever even over ADSL, and I
bet the speed up will be noticable even on a local connection for a
moderately slow box. I guess I'm going to have to find the
documentation for Xft and add that to the general-docs package, too, I
just have no idea about the stuff you're finding at this point.
Daniel> Retaining the object means that they stick around, rather
Daniel> than being recreated every time text needs to be drawn.
I would call this "lazy initialization" rather than "caching".
I've got your other patch in my development workspace now, but in my
usage pattern it doesn't make any difference. I'll review and apply
this and push ASAP.
BTW, I'm seeing these:
X Error of failed request: BadValue (integer parameter out of range for operation)
Major opcode of failed request: 12 (X_ConfigureWindow)
Value in failed request: 0x0
Serial number of failed request: 11519370
Current serial number in output stream: 11519377
all the time when starting to compose mail or return to reading in
Gnus. Seems to have something to do with window configuration. I bet
it's whatever Michael fixed a month ago, so probably a 21.5.22 release
of the mainline and a synch of sjt-xft to that is in order.
--
School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp
University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN
Ask not how you can "do" free software business;
ask what your business can "do for" free software.
9:20 a.m.
New subject: [SAC sjt-xft] "Fix an Xft leak..." and "Performance enhancement..."
SUPERSEDES APPROVE COMMIT sjt-xft
Courtesy notice, private branch.
This patch supersedes both of Daniel's patches:
<87zmqs7z2v.fsf(a)rimspace.net> and <87zmqs7z2v.fsf(a)rimspace.net>.
There should be a many orders of magnitude improvement in redisplay
speed due to Daniel making the XftDraw structure a persistent member of
struct x_frame. Daniel has also plugged a leak of XftDraw structures
from the menubar, but I would guess that the following tasks remain:
(1) Use the .xftDraw member of the associated struct x_frame.
(2) Audit and fix xlwtabs.c and maybe the progress bar too.
Daniel also changed the fallbacks for font not found in
x_open_font_name(). On the agenda:
(3) Probably that function should get the axe.
My contributions were minor, fixing the heuristic for determining if a
name is an XLFD, and changing some of the style to fix my coding
tastes. ;-)
This patch also contains some follow-on changes to a recent patch of
mine aimed at getting rid of some fontconfig cruft that needn't be
LISP-visible in fontconfig.el and xft-fonts.c. Basically, I moved the
deallocator for FcPatterns into C and made some simplifications in
functions now that the deallocator is not accessible from LISP.
Index: lisp/ChangeLog.Xft
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/lisp/Attic/ChangeLog.Xft,v
retrieving revision 1.1.2.2
diff -u -U0 -r1.1.2.2 ChangeLog.Xft
--- lisp/ChangeLog.Xft 3 Aug 2005 06:09:18 -0000 1.1.2.2
+++ lisp/ChangeLog.Xft 3 Sep 2005 17:43:30 -0000
＠＠ -0,0 +1,10 ＠＠
+2005-08-17 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ * fontconfig.el (fc-try-font): Improve docstring, add todo comment.
+ (make-fc-pattern): Use defalias, not defun.
+
+2005-08-04 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ * fontconfig.el (make-fc-pattern): Don't add finalizer, now
+ finalizer is in lrecord implementation.
+
Index: lwlib/ChangeLog.Xft
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/lwlib/Attic/ChangeLog.Xft,v
retrieving revision 1.1.2.1
diff -u -U0 -r1.1.2.1 ChangeLog.Xft
--- lwlib/ChangeLog.Xft 15 Mar 2005 05:22:25 -0000 1.1.2.1
+++ lwlib/ChangeLog.Xft 6 Sep 2005 06:58:17 -0000
＠＠ -0,0 +1,15 ＠＠
+2005-09-06 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ * lwlib-fonts.c (xft_open_font_by_name): Fix heuristic for XLFDs.
+
+ * xlwmenu.c (string_draw_range): Recommend persistent xftDraw.
+
+2005-09-02 Daniel Pittman <daniel(a)rimspace.net>
+
+ * lwlib-fonts.c (xft_open_font_by_name): Eliminate code to avoid
+ use of scaled fonts (probably bogus, anyway). Add error message
+ in case of fatal failure to find any fonts at all.
+ <87ll2fucw1.fsf(a)rimspace.net>
+
+ * xlwmenu.c (string_draw_range): Plug leak of XftDraw.
+
Index: src/ChangeLog.Xft
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/src/Attic/ChangeLog.Xft,v
retrieving revision 1.1.2.6
diff -u -U0 -r1.1.2.6 ChangeLog.Xft
--- src/ChangeLog.Xft 3 Aug 2005 06:09:32 -0000 1.1.2.6
+++ src/ChangeLog.Xft 6 Sep 2005 07:29:21 -0000
＠＠ -0,0 +1,30 ＠＠
+2005-09-06 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ * console-x-impl.h (struct x_frame): Rewrite comment.
+
+ * redisplay-x.c (x_output_string):
+ * frame-x.c (x_delete_frame):
+ Improve style, let compiler optimize.
+
+2005-09-05 Daniel Pittman <daniel(a)rimspace.net>
+
+ * console-x-impl.h (struct x_frame): New member xftDraw.
+ (FRAME_X_XFTDRAW): Accessor for xftDraw member.
+
+ * redisplay-x.c (x_output_string): Lazily initialize frame's
+ xftDraw member, and don't destroy it here.
+
+ * frame-x.c (x_delete_frame): Destroy xftDraw here.
+
+2005-08-17 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ * xft-fonts.c (Ffc_pattern_create): Improve docstring, remove todo.
+ (fc_intern): Add todo.
+
+2005-08-04 Stephen J. Turnbull <stephen(a)xemacs.org>
+
+ * xft-fonts.c (finalize_fc_pattern): New static function.
+ Add it to lcrecord implementation of fc_pattern.
+
+ * xft-fonts.c (Ffc_pattern_destroy): Disable with #if 0.
+
Index: lisp/fontconfig.el
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/lisp/Attic/fontconfig.el,v
retrieving revision 1.1.2.4
diff -u -r1.1.2.4 fontconfig.el
--- lisp/fontconfig.el 3 Aug 2005 06:09:19 -0000 1.1.2.4
+++ lisp/fontconfig.el 6 Sep 2005 05:49:53 -0000
＠＠ -267,16 +267,20 ＠＠
(let ((pair (assoc str fc-font-name-weight-mapping-string-reverse)))
(if pair (cdr pair))))
-(defun make-fc-pattern ()
- "Make a Xft pattern record."
- (let ((pattern (fc-pattern-create)))
- (add-finalizer pattern 'fc-pattern-destroy)
- pattern))
+;; #### it might make sense to generalize `make-fc-pattern' by allowing
+;; a plist of properties as an optional argument.
+
+;; A LISP-y alias for the pattern constructor function.
+(defalias 'make-fc-pattern 'fc-pattern-create)
+
+;; #### it might make sense to generalize `fc-try-font' by having a
+;; global variable that contains a list of font name parsers. They are
+;; tried in order, and the first one to return an fc-pattern is matched.
(defun fc-try-font (font &optional device)
"Return list of pattern objects matching FONT on DEVICE.
-FONT may be a fontconfig pattern object or a string (typically an XLFD).
+FONT may be a fontconfig pattern object or a fontconfig font name (a string).
Optional DEVICE is the device object to query, defaulting to the currently
selected device."
(fc-list-fonts-pattern-objects (or device (default-x-device))
Index: lwlib/lwlib-fonts.c
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/lwlib/Attic/lwlib-fonts.c,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 lwlib-fonts.c
--- lwlib/lwlib-fonts.c 25 Nov 2004 12:44:05 -0000 1.1.2.1
+++ lwlib/lwlib-fonts.c 4 Sep 2005 11:51:39 -0000
＠＠ -26,6 +26,8 ＠＠
/* Synched up with: Not in GNU Emacs. */
#include <config.h>
+#include <stdio.h>
+#include <stdlib.h>
#include "lwlib-fonts.h"
#if 0
＠＠ -45,7 +47,23 ＠＠
/* helper function to correctly open Xft/core fonts by name
#### Can't we use FcParseName here?
- #### Is this done so often that the logic needs to be hard-coded in C? */
+ #### Is this done so often that the logic needs to be hard-coded in C?
+
+ Daniel Pittman sez: Older code tried to enforce that an XLFD font was
+ not scaled, while this version just doesn't care. I think that is a
+ better behavior, since if someone really wants a scaled font we should
+ oblige them.
+
+ Stephen sez: This whole function was ill-conceived, and I'm not sure it
+ succeeds at any of the things it attempts to do. First, we should be
+ using fontconfig directly. I'm not sure what Xft (or fontconfig) will
+ try to do if passed an XLFD. As for scaled fonts, both options are
+ equally bad. The problem is that the X server will often scale bitmap
+ fonts willy-nilly; it's worth trying to avoid that, but I can't say
+ whether that's worth overriding somebody who knows what they're doing.
+ In any case, I think we should find out what Xft (fontconfig?) is able
+ and willing to do with XLFDs, and probably move the logic to LISP.
+*/
XftFont *
xft_open_font_by_name (Display *dpy, char *name)
{
＠＠ -62,29 +80,29 ＠＠
pos++;
}
- if (count < 14 && !(count >= 5 && index (name, '*')))
- /* #### hard-coding DefaultScreen is evil! */
- res = XftFontOpenName (dpy, DefaultScreen (dpy), name);
+ /* #### hard-coding DefaultScreen is evil! */
+ if (count == 14 /* fully-qualified XLFD */
+ || (count < 14 /* heuristic for wildcarded XLFD */
+ && count >= 5
+ && index (name, '*')))
+ res = XftFontOpenXlfd (dpy, DefaultScreen (dpy), name);
else
- {
- FcPattern *pat = FcPatternCreate ();
- /* This is the magic pattern to open core fonts ... */
- /* Dudes, I love Xft!!! */
- FcPatternAddString (pat, XFT_XLFD, (FcChar8 *) name);
- /* #### Is this core fonts, or is this using the core protocol when
- Xrender is unavailable? */
- FcPatternAddBool (pat, XFT_CORE, True);
- FcPatternAddBool (pat, FC_SCALABLE, False);
- res = XftFontOpenPattern (dpy, pat);
- }
+ res = XftFontOpenName (dpy, DefaultScreen (dpy), name);
+
+ /* Try for a generic monospace font
+ #### Why? Menus don't need to line up in columns! */
+ if (!res)
+ res = XftFontOpenName (dpy, DefaultScreen (dpy), "monospace");
+ /* Try for anything we can get */
if (!res)
- /* this is our last try ... */
res = XftFontOpenName (dpy, DefaultScreen (dpy), "");
if (!res)
{
/* #### This is Just So Wrong ... ! */
/* sorry folks ... */
+ fprintf (stderr,
+ "Unable to find any usable XFT font, even the defaults!\n");
abort ();
return 0;
}
Index: lwlib/xlwmenu.c
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/lwlib/xlwmenu.c,v
retrieving revision 1.37.2.2
diff -u -r1.37.2.2 xlwmenu.c
--- lwlib/xlwmenu.c 10 Dec 2004 06:43:25 -0000 1.37.2.2
+++ lwlib/xlwmenu.c 6 Sep 2005 06:58:34 -0000
＠＠ -879,6 +879,7 ＠＠
Visual *visual = DefaultVisualOfScreen (XtScreen (mw));
Colormap cmap = mw->core.colormap;
XftFont *renderFont = mw->menu.renderFont;
+ /* #### should use parent frame's .xftDraw? */
XftDraw *xftDraw = XftDrawCreate (display, window, visual, cmap);
/* draw background rect */
XftDrawRect (xftDraw, colorBg,
＠＠ -894,6 +895,8 ＠＠
XftTextExtents8 (display, renderFont, &string[start], end - start,
&glyphinfo);
+ /* #### should use parent frame's .xftDraw */
+ XftDrawDestroy (xftDraw);
return glyphinfo.xOff;
}
#else
Index: src/console-x-impl.h
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/src/console-x-impl.h,v
retrieving revision 1.3
diff -u -r1.3 console-x-impl.h
--- src/console-x-impl.h 4 Nov 2004 23:06:18 -0000 1.3
+++ src/console-x-impl.h 6 Sep 2005 07:10:40 -0000
＠＠ -290,6 +290,17 ＠＠
#endif /* XIM_XLIB */
#endif /* HAVE_XIM */
+#ifdef USE_XFT
+ /* The Xft Drawable wrapper for this device.
+ #### Should this be per-device, or per-frame? */
+ /* This is persistent to take advantage of the ability of Xft's glyph
+ cache in the server, and avoid rendering the font again and again...
+
+ This is created the first time through redisplay, and destroyed when our
+ connection to the X display is destroyed. */
+ XftDraw *xftDraw;
+#endif
+
/* 1 if the frame is completely visible on the display, 0 otherwise.
if 0 the frame may have been iconified or may be totally
or partially hidden by another X window */
＠＠ -339,6 +350,10 ＠＠
#define FRAME_X_GEOM_FREE_ME_PLEASE(f) (FRAME_X_DATA (f)->geom_free_me_please)
+#ifdef USE_XFT
+#define FRAME_X_XFTDRAW(f) (FRAME_X_DATA (f)->xftDraw)
+#endif
+
#define FRAME_X_TOTALLY_VISIBLE_P(f) (FRAME_X_DATA (f)->totally_visible_p)
#define FRAME_X_TOP_LEVEL_FRAME_P(f) (FRAME_X_DATA (f)->top_level_frame_p)
Index: src/frame-x.c
===================================================================
RCS file: /Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/src/frame-x.c,v
retrieving revision 1.68.2.6
diff -u -r1.68.2.6 frame-x.c
--- src/frame-x.c 13 Jul 2005 04:46:45 -0000 1.68.2.6
+++ src/frame-x.c 6 Sep 2005 07:19:09 -0000
＠＠ -2613,6 +2613,19 ＠＠
DtDndDropUnregister (FRAME_X_TEXT_WIDGET (f));
#endif /* HAVE_CDE */
+#ifdef USE_XFT
+ /* If we have an XftDraw structure, we need to free it here.
+ We can't ever have an XftDraw without a Display, so we are safe
+ to free it in here, and we avoid too much playing around with the
+ malloc checking hooks this way. */
+ if (FRAME_X_XFTDRAW (f))
+ {
+ XftDrawDestroy (FRAME_X_XFTDRAW (f));
+ FRAME_X_XFTDRAW (f) = NULL;
+ }
+#endif
+
+
assert (FRAME_X_SHELL_WIDGET (f) != 0);
dpy = XtDisplay (FRAME_X_SHELL_WIDGET (f));
Index: src/redisplay-x.c
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/src/redisplay-x.c,v
retrieving revision 1.39.2.5
diff -u -r1.39.2.5 redisplay-x.c
--- src/redisplay-x.c 15 Mar 2005 05:24:42 -0000 1.39.2.5
+++ src/redisplay-x.c 6 Sep 2005 07:17:33 -0000
＠＠ -937,9 +937,13 ＠＠
Colormap cmap = DEVICE_X_COLORMAP (d);
Visual *visual = DEVICE_X_VISUAL (d);
static XftColor fg, bg;
- /* We probably want to cache that, but how and where? */
- /* Move it to frame-impl.h, Dude! XXX */
- XftDraw *xftDraw = XftDrawCreate (dpy, x_win, visual, cmap);
+ XftDraw *xftDraw;
+
+ /* Lazily initialize frame's xftDraw member. */
+ if (!FRAME_X_XFTDRAW (f)) {
+ FRAME_X_XFTDRAW (f) = XftDrawCreate (dpy, x_win, visual, cmap);
+ }
+ xftDraw = FRAME_X_XFTDRAW (f);
/* #### This will probably cause asserts when passed a Lisp integer for a
color. See ca. line 759 this file.
＠＠ -1464,7 +1468,6 ＠＠
}
#ifdef USE_XFT
- XftDrawDestroy (xftDraw);
#undef XFT_FROB_LISP_COLOR
#endif
Index: src/xft-fonts.c
===================================================================
RCS file:
/Users/steve/Software/Repositories/cvs.xemacs.org/XEmacs/xemacs/src/Attic/xft-fonts.c,v
retrieving revision 1.1.2.13
diff -u -r1.1.2.13 xft-fonts.c
--- src/xft-fonts.c 3 Aug 2005 06:09:32 -0000 1.1.2.13
+++ src/xft-fonts.c 3 Sep 2005 17:43:30 -0000
＠＠ -86,13 +86,24 ＠＠
* FcPattern objects *
****************************************************************/
+static void
+finalize_fc_pattern (void *header, int UNUSED (for_disksave))
+{
+ struct fc_pattern *p = (struct fc_pattern *) header;
+ if (p->fcpatPtr)
+ {
+ FcPatternDestroy (p->fcpatPtr);
+ p->fcpatPtr = 0;
+ }
+}
+
static const struct memory_description fcpattern_description [] = {
/* #### nothing here, is this right?? */
{ XD_END }
};
DEFINE_LRECORD_IMPLEMENTATION("fc-pattern", fc_pattern,
- 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, finalize_fc_pattern, 0, 0,
fcpattern_description,
struct fc_pattern);
＠＠ -147,6 +158,9 ＠＠
static struct hash_table *fc_property_name_hash_table;
+/* #### Maybe fc_intern should be exposed to LISP? The idea is that
+ fc-pattern-add could warn or error if the property isn't interned. */
+
static FcChar8 *
fc_intern (Lisp_Object property)
{
＠＠ -175,7 +189,7 ＠＠
}
DEFUN("fc-pattern-create", Ffc_pattern_create, 0, 0, 0, /*
-Create a fresh and empty fc-pattern object.
+Return a new, empty fc-pattern object.
*/
())
{
＠＠ -202,7 +216,7 ＠＠
alloc_lcrecord_type (struct fc_pattern, &lrecord_fc_pattern);
#endif
- CHECK_STRING(name);
+ CHECK_STRING(name); /* #### MEMORY LEAK!! maybe not ... */
fcpat->fcpatPtr = FcNameParse (extract_fcapi_string (name));
return wrap_fcpattern(fcpat);
＠＠ -474,7 +488,8 ＠＠
}
}
-#if 1
+#if 0
+/* #### delete this after some testing!! don't forget the DEFSUBR */
/* #### This is a big mistake, no? --- crap, there's no implicit finalizer */
DEFUN("fc-pattern-destroy", Ffc_pattern_destroy, 1, 1, 0, /*
Explicitly deallocate a fc pattern object PATTERN. */
＠＠ -511,7 +526,7 ＠＠
#else
alloc_lcrecord_type (struct fc_pattern, &lrecord_fc_pattern);
#endif
- CHECK_FCPATTERN(pattern);
+ CHECK_FCPATTERN(pattern); /* #### MEMORY LEAKS!!! */
if (NILP(device))
return Qnil;
CHECK_X_DEVICE(device);
＠＠ -629,6 +644,8 ＠＠
}
}
+/* #### this actually is an Xft function, should split those out
+ or get rid of them entirely? */
/* #### be consistent about argument order. */
DEFUN("fc-font-real-pattern", Ffc_font_real_pattern, 2, 2, 0, /*
Temporarily open FONTNAME (a string) and return the actual
＠＠ -645,7 +662,7 ＠＠
alloc_lcrecord_type (struct fc_pattern, &lrecord_fc_pattern);
#endif
- CHECK_STRING (fontname);
+ CHECK_STRING (fontname); /* #### MEMORY LEAK?! maybe not ... */
if (NILP(xdevice))
return Qnil;
CHECK_X_DEVICE (xdevice);
＠＠ -828,7 +845,9 ＠＠
DEFSUBR(Ffc_pattern_add);
DEFSUBR(Ffc_pattern_del);
DEFSUBR(Ffc_pattern_get);
+#if 0
DEFSUBR(Ffc_pattern_destroy);
+#endif
DEFSUBR(Ffc_list_fonts_pattern_objects);
DEFSUBR(Ffc_font_sort);
DEFSUBR(Ffc_font_match);
--
School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp
University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN
Ask not how you can "do" free software business;
ask what your business can "do for" free software.
7046
days inactive
7047
days old
2 comments
2 participants
participants (2)
-
Daniel Pittman -
Stephen J. Turnbull