Package: emacs Version: 23.0.60 Severity: wishlist Tags: patch Ideally, underlining would be able to break at (leave gaps for) descenders (in latin script typically gjpqy and some old-style numerals), to improve legibility. See attached screenshots of a proof-of-concept implementation - with "the quick brown fox..." and the M-x view-hello-file HELLO (I don't know about actual conventional underline positions for the non-latin scripts, just provided a source of descenders for testing) I've attached the proof-of-concept patch where I tried an old "cheat" way to do this, by drawing the text jittered horizontally and vertically by one display pixel in background color several times "over" the underline (and clipped to the underline area) but "under" the final foreground text. This is obviously quite inefficient and not perfect (e.g. will draw underline inside looping descenders if loop is big enough, and as it stands the patch doesn't jitter prev/next string's overhanging descenders if any). I also only used it and tested with the xft FontBackend. There are probably many better ways to do this than this patch, but I only intended it as a proof-of-concept.
Index: src/xterm.c
===================================================================
RCS file: /sources/emacs/emacs/src/xterm.c,v
retrieving revision 1.994
diff -U 8 -r1.994 xterm.c
--- src/xterm.c 26 May 2008 11:37:42 -0000 1.994
+++ src/xterm.c 27 May 2008 03:21:33 -0000
@@ -178,16 +178,27 @@
/* Non-zero means make use of UNDERLINE_POSITION font properties. */
int x_use_underline_position_properties;
/* Non-zero means to draw the underline at the same place as the descent line.
*/
int x_underline_at_descent_line;
+/* Require underline to be at least this many screen pixels below baseline
+ This to avoid underline "merging" with the base of letters at small
+ font sizes, particularly when x_use_underline_position_properties is on. */
+
+int x_underline_minimum_display_offset;
+
+/* Non-zero means try to break underline drawing at descenders. Prettier
+ but potentially resource-intensive */
+
+int x_underline_break_at_descenders;
+
/* This is a chain of structures for all the X displays currently in
use. */
struct x_display_info *x_display_list;
/* This is a list of cons cells, each of the form (NAME
. FONT-LIST-CACHE), one for each element of x_display_list and in
the same order. NAME is the name of the frame. FONT-LIST-CACHE
@@ -932,16 +943,17 @@
static void x_set_glyph_string_clipping P_ ((struct glyph_string *));
static void x_set_glyph_string_gc P_ ((struct glyph_string *));
static void x_draw_glyph_string_background P_ ((struct glyph_string *,
int));
static void x_draw_glyph_string_foreground P_ ((struct glyph_string *));
static void x_draw_composite_glyph_string_foreground P_ ((struct glyph_string
*));
static void x_draw_glyph_string_box P_ ((struct glyph_string *));
+static void x_draw_glyph_string_underline P_ ((struct glyph_string *));
static void x_draw_glyph_string P_ ((struct glyph_string *));
static void x_compute_glyph_string_overhangs P_ ((struct glyph_string *));
static void x_set_cursor_gc P_ ((struct glyph_string *));
static void x_set_mode_line_face_gc P_ ((struct glyph_string *));
static void x_set_mouse_face_gc P_ ((struct glyph_string *));
static int x_alloc_lighter_color P_ ((struct frame *, Display *, Colormap,
unsigned long *, double, int));
static void x_setup_relief_color P_ ((struct frame *, struct relief *,
@@ -2615,16 +2627,158 @@
if (background_width > 0)
x_draw_glyph_string_bg_rect (s, x, s->y, background_width, s->height);
}
s->background_filled_p = 1;
}
+/* Draw underline for glyph string S. */
+
+static void
+x_draw_glyph_string_underline (s)
+ struct glyph_string *s;
+{
+ /* Draw underline. */
+ if (!s->for_overlaps && s->face->underline_p)
+ {
+ unsigned long thickness, position;
+ int y;
+
+ if (s->prev && s->prev->face->underline_p)
+ {
+ /* We use the same underline style as the previous one. */
+ thickness = s->prev->underline_thickness;
+ position = s->prev->underline_position;
+ }
+ else
+ {
+ /* Get the underline thickness. Default is 1 pixel. */
+ if (s->font && s->font->underline_thickness > 0)
+ thickness = s->font->underline_thickness;
+ else
+ thickness = 1;
+ if (x_underline_at_descent_line)
+ position = (s->height - thickness) - (s->ybase - s->y);
+ else
+ {
+ /* Get the underline position. This is the recommended
+ vertical offset in pixels from the baseline to the top of
+ the underline. This is a signed value according to the
+ specs, and its default is
+
+ ROUND ((maximum descent) / 2), with
+ ROUND(x) = floor (x + 0.5) */
+
+ if (x_use_underline_position_properties
+ && s->font && s->font->underline_position >= 0)
+ position = s->font->underline_position;
+ else if (s->font)
+ position = (s->font->descent + 1) / 2;
+ }
+ if (x_underline_minimum_display_offset)
+ position = max (position, eabs
(x_underline_minimum_display_offset));
+ }
+ /* Check the sanity of thickness and position. We should
+ avoid drawing underline out of the current line area. */
+ if (s->y + s->height <= s->ybase + position)
+ position = (s->height - 1) - (s->ybase - s->y);
+ if (s->y + s->height < s->ybase + position + thickness)
+ thickness = (s->y + s->height) - (s->ybase + position);
+ s->underline_thickness = thickness;
+ s->underline_position = position;
+ y = s->ybase + position;
+ if (s->face->underline_defaulted_p)
+ XFillRectangle (s->display, s->window, s->gc,
+ s->x, y, s->width, thickness);
+ else
+ {
+ XGCValues xgcv;
+ XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+ XSetForeground (s->display, s->gc, s->face->underline_color);
+ XFillRectangle (s->display, s->window, s->gc,
+ s->x, y, s->width, thickness);
+ XSetForeground (s->display, s->gc, xgcv.foreground);
+ }
+
+ /* Break underline at descenders, by cheating:
+ Repeatedly draws glyphs in background color, jittered
+ left/right/up/down, clipped to underline area.
+ Yes, this method is very wasteful.
+ Underline drawing must happen before glyph foreground drawing
+ for this trick to be successful, this is presently ensured in
+ x_draw_glyph_string.
+ KLUDGE cross-ref: xft fontbackend xftfont_get_colors() was too clever
+ - caches colors based on comparisons of GC and face info, so disabled
+ caching in that fontbackend for this to work.
+
+ Really, underline (and overline and strikethrough) drawing itself
+ (though maybe not positioning computation) might belong in the
+ fontbackends. */
+
+ /* Only try to break for CHAR_GLYPH and COMPOSITE_GLYPH. */
+ if (x_underline_break_at_descenders &&
+ ((s->first_glyph->type == CHAR_GLYPH) ||
+ (s->first_glyph->type == COMPOSITE_GLYPH)))
+ {
+ int x_save, y_save, ybase_save;
+ int x_jitter, y_jitter;
+ /* maybe shouldn't merrily modify fields of s, but currently works. */
+ XGCValues xgcv;
+ XRectangle r;
+
+ XGetGCValues (s->display, s->gc, GCForeground | GCBackground, &xgcv);
+ x_save = s->x;
+ y_save = s->y;
+ ybase_save = s->ybase;
+
+ /* temprorarily clip to underline area */
+ r.x = s->x;
+ r.y = y;
+ r.width = s->width;
+ r.height = thickness;
+ XSetClipRectangles (s->display, s->gc, 0, 0, &r, 1, Unsorted);
+
+ XSetForeground (s->display, s->gc, xgcv.background);
+ if (s->face->underline_defaulted_p)
+ XSetBackground (s->display, s->gc, xgcv.foreground);
+ else
+ XSetBackground (s->display, s->gc, s->face->underline_color);
+
+ for (y_jitter = -1; y_jitter <= 1; y_jitter++) {
+ s->y = y_save + y_jitter;
+ s->ybase = ybase_save + y_jitter;
+ for (x_jitter = -1 ; x_jitter <= 1; x_jitter++) {
+ s->x = x_save + x_jitter;
+ switch (s->first_glyph->type)
+ {
+ case CHAR_GLYPH:
+ x_draw_glyph_string_foreground (s);
+ break;
+ case COMPOSITE_GLYPH:
+ x_draw_composite_glyph_string_foreground (s);
+ break;
+ }
+ }
+ }
+
+ s->ybase = ybase_save;
+ s->y = y_save;
+ s->x = x_save;
+ XSetBackground (s->display, s->gc, xgcv.background);
+ XSetForeground (s->display, s->gc, xgcv.foreground);
+ XSetClipRectangles (s->display, s->gc, 0, 0, s->clip, s->num_clips,
Unsorted);
+ }
+
+ s->underline_drawn_p = 1;
+ }
+}
+
+
/* Draw glyph string S. */
static void
x_draw_glyph_string (s)
struct glyph_string *s;
{
int relief_drawn_p = 0;
@@ -2682,94 +2836,51 @@
case STRETCH_GLYPH:
x_draw_stretch_glyph_string (s);
break;
case CHAR_GLYPH:
if (s->for_overlaps)
s->background_filled_p = 1;
else
- x_draw_glyph_string_background (s, 0);
+ {
+ /* Draw fancy broken underline before foreground */
+ if (s->face->underline_p && x_underline_break_at_descenders)
+ {
+ x_draw_glyph_string_background (s, 1); /* must redraw bg */
+ x_draw_glyph_string_underline (s);
+ }
+ else
+ x_draw_glyph_string_background (s, 0);
+ }
x_draw_glyph_string_foreground (s);
break;
case COMPOSITE_GLYPH:
if (s->for_overlaps || s->gidx > 0)
s->background_filled_p = 1;
else
+ {
x_draw_glyph_string_background (s, 1);
+ /* Draw fancy broken underline before foreground */
+ if (s->face->underline_p && x_underline_break_at_descenders)
+ x_draw_glyph_string_underline (s);
+ }
x_draw_composite_glyph_string_foreground (s);
break;
default:
abort ();
}
if (!s->for_overlaps)
{
- /* Draw underline. */
- if (s->face->underline_p)
- {
- unsigned long thickness, position;
- int y;
-
- if (s->prev && s->prev->face->underline_p)
- {
- /* We use the same underline style as the previous one. */
- thickness = s->prev->underline_thickness;
- position = s->prev->underline_position;
- }
- else
- {
- /* Get the underline thickness. Default is 1 pixel. */
- if (s->font && s->font->underline_thickness > 0)
- thickness = s->font->underline_thickness;
- else
- thickness = 1;
- if (x_underline_at_descent_line)
- position = (s->height - thickness) - (s->ybase - s->y);
- else
- {
- /* Get the underline position. This is the recommended
- vertical offset in pixels from the baseline to the top of
- the underline. This is a signed value according to the
- specs, and its default is
-
- ROUND ((maximum descent) / 2), with
- ROUND(x) = floor (x + 0.5) */
-
- if (x_use_underline_position_properties
- && s->font && s->font->underline_position >= 0)
- position = s->font->underline_position;
- else if (s->font)
- position = (s->font->descent + 1) / 2;
- }
- }
- /* Check the sanity of thickness and position. We should
- avoid drawing underline out of the current line area. */
- if (s->y + s->height <= s->ybase + position)
- position = (s->height - 1) - (s->ybase - s->y);
- if (s->y + s->height < s->ybase + position + thickness)
- thickness = (s->y + s->height) - (s->ybase + position);
- s->underline_thickness = thickness;
- s->underline_position = position;
- y = s->ybase + position;
- if (s->face->underline_defaulted_p)
- XFillRectangle (s->display, s->window, s->gc,
- s->x, y, s->background_width, thickness);
- else
- {
- XGCValues xgcv;
- XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
- XSetForeground (s->display, s->gc, s->face->underline_color);
- XFillRectangle (s->display, s->window, s->gc,
- s->x, y, s->background_width, thickness);
- XSetForeground (s->display, s->gc, xgcv.foreground);
- }
- }
+ /* Draw underline if not yet drawn */
+ if (s->face->underline_p && !s->underline_drawn_p)
+ x_draw_glyph_string_underline (s);
/* Draw overline. */
if (s->face->overline_p)
{
unsigned long dy = 0, h = 1;
if (s->face->overline_color_defaulted_p)
XFillRectangle (s->display, s->window, s->gc, s->x, s->y + dy,
@@ -10778,27 +10889,45 @@
staticpro (&last_mouse_press_frame);
last_mouse_press_frame = Qnil;
DEFVAR_BOOL ("x-use-underline-position-properties",
&x_use_underline_position_properties,
doc: /* *Non-nil means make use of UNDERLINE_POSITION font properties.
A value of nil means ignore them. If you encounter fonts with bogus
UNDERLINE_POSITION font properties, for example 7x13 on XFree prior
-to 4.1, set this to nil. */);
+to 4.1, set this to nil. Variable `x-underline-minimum-display-offset' may
+be used to override the font's UNDERLINE_POSITION for small font display
+sizes. */);
x_use_underline_position_properties = 1;
DEFVAR_BOOL ("x-underline-at-descent-line",
&x_underline_at_descent_line,
doc: /* *Non-nil means to draw the underline at the same place as the
descent line.
A value of nil means to draw the underline according to the value of the
variable `x-use-underline-position-properties', which is usually at the
baseline level. The default value is nil. */);
x_underline_at_descent_line = 0;
+ DEFVAR_INT ("x-underline-minimum-display-offset",
+ &x_underline_minimum_display_offset,
+ doc: /* *When > 0, underline is drawn at least that many screen pixels
below baseline.
+This can improve legibility of underlined text at small font sizes,
+particularly when using variable `x-use-underline-position-properties'
+with fonts that specify an UNDERLINE_POSITION relatively close to the
+baseline. The default value is 0. */);
+ x_underline_minimum_display_offset = 0;
+
+ DEFVAR_BOOL ("x-underline-break-at-descenders",
+ &x_underline_break_at_descenders,
+ doc: /* *Non-nil means to break underline where it crosses text
descenders e.g. gjpqy.
+This improves legibility. Note that the current implementation may be slow,
+particularly on remote displays. The default value is nil. */);
+ x_underline_break_at_descenders = 0;
+
DEFVAR_BOOL ("x-mouse-click-focus-ignore-position",
&x_mouse_click_focus_ignore_position,
doc: /* Non-nil means that a mouse click to focus a frame does not move
point.
This variable is only used when the window manager requires that you
click on a frame to select it (give it focus). In that case, a value
of nil, means that the selected window and cursor position changes to
reflect the mouse click position, while a non-nil value means that the
selected window or cursor position is preserved. */);
Index: src/dispextern.h
===================================================================
RCS file: /sources/emacs/emacs/src/dispextern.h,v
retrieving revision 1.245
diff -U 8 -r1.245 dispextern.h
--- src/dispextern.h 22 May 2008 14:52:03 -0000 1.245
+++ src/dispextern.h 27 May 2008 03:21:35 -0000
@@ -1172,16 +1172,19 @@
/* 1 means this glyph strings face has to be drawn to the right end
of the window's drawing area. */
unsigned extends_to_end_of_line_p : 1;
/* 1 means the background of this string has been drawn. */
unsigned background_filled_p : 1;
+ /* 1 means the underline of this string has been drawn. */
+ unsigned underline_drawn_p : 1;
+
/* 1 means glyph string must be drawn with 16-bit functions. */
unsigned two_byte_p : 1;
/* 1 means that the original font determined for drawing this glyph
string could not be loaded. The member `font' has been set to
the frame's default font in this case. */
unsigned font_not_found_p : 1;
Index: src/xftfont.c
===================================================================
RCS file: /sources/emacs/emacs/src/xftfont.c,v
retrieving revision 1.9
diff -U 8 -r1.9 xftfont.c
--- src/xftfont.c 25 May 2008 11:04:53 -0000 1.9
+++ src/xftfont.c 27 May 2008 03:21:36 -0000
@@ -74,17 +74,17 @@
static void
xftfont_get_colors (f, face, gc, xftface_info, fg, bg)
FRAME_PTR f;
struct face *face;
GC gc;
struct xftface_info *xftface_info;
XftColor *fg, *bg;
{
- if (xftface_info && face->gc == gc)
+ if ( 0 && xftface_info && face->gc == gc) /* 0: Disabled for underline
breaking kludge */
{
*fg = xftface_info->xft_fg;
if (bg)
*bg = xftface_info->xft_bg;
}
else
{
XGCValues xgcv;
Index: lisp/cus-start.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/cus-start.el,v
retrieving revision 1.122
diff -U 8 -r1.122 cus-start.el
--- lisp/cus-start.el 6 May 2008 07:57:29 -0000 1.122
+++ lisp/cus-start.el 27 May 2008 03:21:36 -0000
@@ -388,16 +388,18 @@
(repeat (directory :format "%v")))
(x-gtk-use-old-file-dialog menu boolean "22.1")
(x-gtk-show-hidden-files menu boolean "22.1")
(x-gtk-file-dialog-help-text menu boolean "22.1")
(x-gtk-whole-detached-tool-bar x boolean "22.1")
;; xterm.c
(x-use-underline-position-properties display boolean "22.1")
(x-underline-at-descent-line display boolean "22.1")
+ (x-underline-minimum-display-offset display integer "23.1")
+ (x-underline-break-at-descenders display boolean "23.1")
(x-stretch-cursor display boolean "21.1")))
this symbol group type standard version native-p
;; This function turns a value
;; into an expression which produces that value.
(quoter (lambda (sexp)
(if (or (memq sexp '(t nil))
(keywordp sexp)
(and (listp sexp)
--- End Message ---