bug#315: Underline minimum display offset

bug#315: Underline minimum display offset

[David De La Harpe Golden]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

Package: emacs
Version: 23.0.60
Severity: wishlist
Tags: patch

Attached (trivial) patch adds a customization variable
x-underline-minimum-display-offset to allow specifying that underlining
should always be _at least_ N display pixels from baseline. N=0 is
current emacs behaviour and the default in the patch, though maybe 1
should be the default.  Setting it to nonzero value improves legibility
at small font sizes while keeping the font-specified underline at large
font sizes.

e.g. On my system (~100DPI), xft backend, the DejaVu Sans Mono underline
position "hits the baseline" for sizes below 11.5 point, decreasing
legibility, especially when the underline is the same color as the text.
For sizes above that, the font-specified underline position works okay
and in keeping with general style of the font, so I do not want to set
x-underline-at-descent-line to t or x-use-underline-position-properties
to nil, this is a more subtle override effect for small sizes.

See attached with_underline_offset.png and without_underline_offset.png
(file to generate them included as underline_sample.txt).  Both with
x-use-underline-position-properties non-nil, "with" with 1 pixel
mininum, "without" with 0 pixel minimum.




[-- Attachment #2: x-underline-minimum-display-offset.diff --]
[-- Type: text/x-diff, Size: 3608 bytes --]

Index: src/xterm.c
===================================================================
RCS file: /sources/emacs/emacs/src/xterm.c,v
retrieving revision 1.993
diff -U 4 -r1.993 xterm.c
--- src/xterm.c	25 May 2008 11:14:31 -0000	1.993
+++ src/xterm.c	26 May 2008 00:09:27 -0000
@@ -182,8 +182,14 @@
 /* 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;
+
 /* This is a chain of structures for all the X displays currently in
    use.  */
 
 struct x_display_info *x_display_list;
@@ -2741,8 +2747,10 @@
 		    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)
@@ -10782,9 +10790,11 @@
 	       &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,
@@ -10793,8 +10803,18 @@
 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-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
Index: lisp/cus-start.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/cus-start.el,v
retrieving revision 1.122
diff -U 4 -r1.122 cus-start.el
--- lisp/cus-start.el	6 May 2008 07:57:29 -0000	1.122
+++ lisp/cus-start.el	26 May 2008 00:09:27 -0000
@@ -392,8 +392,9 @@
 	     (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-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.

[-- Attachment #3: without_underline_offset.png --]
[-- Type: image/png, Size: 51566 bytes --]

[-- Attachment #4: with_underline_offset.png --]
[-- Type: image/png, Size: 51691 bytes --]

[-- Attachment #5: underline_sample.txt --]
[-- Type: text/plain, Size: 1531 bytes --]

;; Sample text.

;;size5:  The quick brown fox jumps over the lazy dog. 0123456789
;;size6:  The quick brown fox jumps over the lazy dog. 0123456789
;;size7:  The quick brown fox jumps over the lazy dog. 0123456789
;;size8:  The quick brown fox jumps over the lazy dog. 0123456789
;;size9:  The quick brown fox jumps over the lazy dog. 0123456789
;;size10: The quick brown fox jumps over the lazy dog. 0123456789
;;size11: The quick brown fox jumps over the lazy dog. 0123456789
;;size12: The quick brown fox jumps over the lazy dog. 0123456789
;;size13: The quick brown fox jumps over the lazy dog. 0123456789
;;size14: The quick brown fox jumps over the lazy dog. 0123456789

;; Eval me to munge above.
(prog1 nil
  (setq size-test-overlays (list))
  (let ((sizes '(5 6 7 8 9 10 11 12 13 14)))
    (mapcar (lambda (size)
	      (let ((overlay (make-overlay 0 0)))
		(overlay-put overlay 'face
			     `(:height ,(* 10 size) :underline t))
		(setq size-test-overlays
		      (cons (cons size overlay) size-test-overlays))))
	    sizes))
  (save-match-data
    (save-excursion
      (goto-char 1)
      (while (re-search-forward "^;;size\\([0-9]*\\):.*$" nil t)
	(let* ((size (car (read-from-string (match-string 1))))
	       (overlay (cdr (assoc size size-test-overlays))))
	  (move-overlay overlay (match-beginning 0) (match-end 0)))))))

;; Eval me to clean up.
(prog1 nil
  (mapcar (lambda (pair)
	    (move-overlay (cdr pair) 0 0)
	    (delete-overlay (cdr pair)))
	  size-test-overlays)
  (setq size-test-overlays (list)))

bug#315: marked as done (Underline minimum display offset)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]


Your message dated Tue, 10 Jun 2008 16:26:56 -0400
with message-id <jwvprqpox69.fsf-monnier+emacsbugreports@gnu.org>
and subject line Re: Underline minimum display offset
has caused the Emacs bug report #315,
regarding Underline minimum display offset
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
315: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=315
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 148354 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1226 bytes --]

Package: emacs
Version: 23.0.60
Severity: wishlist
Tags: patch

Attached (trivial) patch adds a customization variable
x-underline-minimum-display-offset to allow specifying that underlining
should always be _at least_ N display pixels from baseline. N=0 is
current emacs behaviour and the default in the patch, though maybe 1
should be the default.  Setting it to nonzero value improves legibility
at small font sizes while keeping the font-specified underline at large
font sizes.

e.g. On my system (~100DPI), xft backend, the DejaVu Sans Mono underline
position "hits the baseline" for sizes below 11.5 point, decreasing
legibility, especially when the underline is the same color as the text.
For sizes above that, the font-specified underline position works okay
and in keeping with general style of the font, so I do not want to set
x-underline-at-descent-line to t or x-use-underline-position-properties
to nil, this is a more subtle override effect for small sizes.

See attached with_underline_offset.png and without_underline_offset.png
(file to generate them included as underline_sample.txt).  Both with
x-use-underline-position-properties non-nil, "with" with 1 pixel
mininum, "without" with 0 pixel minimum.




[-- Attachment #2.1.2: x-underline-minimum-display-offset.diff --]
[-- Type: text/x-diff, Size: 3608 bytes --]

Index: src/xterm.c
===================================================================
RCS file: /sources/emacs/emacs/src/xterm.c,v
retrieving revision 1.993
diff -U 4 -r1.993 xterm.c
--- src/xterm.c	25 May 2008 11:14:31 -0000	1.993
+++ src/xterm.c	26 May 2008 00:09:27 -0000
@@ -182,8 +182,14 @@
 /* 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;
+
 /* This is a chain of structures for all the X displays currently in
    use.  */
 
 struct x_display_info *x_display_list;
@@ -2741,8 +2747,10 @@
 		    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)
@@ -10782,9 +10790,11 @@
 	       &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,
@@ -10793,8 +10803,18 @@
 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-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
Index: lisp/cus-start.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/cus-start.el,v
retrieving revision 1.122
diff -U 4 -r1.122 cus-start.el
--- lisp/cus-start.el	6 May 2008 07:57:29 -0000	1.122
+++ lisp/cus-start.el	26 May 2008 00:09:27 -0000
@@ -392,8 +392,9 @@
 	     (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-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.

[-- Attachment #2.1.3: without_underline_offset.png --]
[-- Type: image/png, Size: 51566 bytes --]

[-- Attachment #2.1.4: with_underline_offset.png --]
[-- Type: image/png, Size: 51691 bytes --]

[-- Attachment #2.1.5: underline_sample.txt --]
[-- Type: text/plain, Size: 1531 bytes --]

;; Sample text.

;;size5:  The quick brown fox jumps over the lazy dog. 0123456789
;;size6:  The quick brown fox jumps over the lazy dog. 0123456789
;;size7:  The quick brown fox jumps over the lazy dog. 0123456789
;;size8:  The quick brown fox jumps over the lazy dog. 0123456789
;;size9:  The quick brown fox jumps over the lazy dog. 0123456789
;;size10: The quick brown fox jumps over the lazy dog. 0123456789
;;size11: The quick brown fox jumps over the lazy dog. 0123456789
;;size12: The quick brown fox jumps over the lazy dog. 0123456789
;;size13: The quick brown fox jumps over the lazy dog. 0123456789
;;size14: The quick brown fox jumps over the lazy dog. 0123456789

;; Eval me to munge above.
(prog1 nil
  (setq size-test-overlays (list))
  (let ((sizes '(5 6 7 8 9 10 11 12 13 14)))
    (mapcar (lambda (size)
	      (let ((overlay (make-overlay 0 0)))
		(overlay-put overlay 'face
			     `(:height ,(* 10 size) :underline t))
		(setq size-test-overlays
		      (cons (cons size overlay) size-test-overlays))))
	    sizes))
  (save-match-data
    (save-excursion
      (goto-char 1)
      (while (re-search-forward "^;;size\\([0-9]*\\):.*$" nil t)
	(let* ((size (car (read-from-string (match-string 1))))
	       (overlay (cdr (assoc size size-test-overlays))))
	  (move-overlay overlay (match-beginning 0) (match-end 0)))))))

;; Eval me to clean up.
(prog1 nil
  (mapcar (lambda (pair)
	    (move-overlay (cdr pair) 0 0)
	    (delete-overlay (cdr pair)))
	  size-test-overlays)
  (setq size-test-overlays (list)))


[-- Attachment #3: Type: message/rfc822, Size: 1737 bytes --]

From: Stefan Monnier <monnier@iro.umontreal.ca>
To: 315-done@emacsbugs.donarmstrong.com
Subject: Re: Underline minimum display offset
Date: Tue, 10 Jun 2008 16:26:56 -0400
Message-ID: <jwvprqpox69.fsf-monnier+emacsbugreports@gnu.org>

I've installed your patch and then promptly changed it: it should
work on w32 as well (thoroughyl untested) and the variable name has been
changed to `underline-minimum-offset'.

Thank you,


        Stefan