Re: Bold by moving pixels problem

Re: Bold by moving pixels problem

[Miles Bader]

On Wed, Nov 20, 2002 at 01:09:10PM +0000, Robert J. Chassell wrote:
> Did you install the patch in which bold is created by duplicating a
> glyph, but moving over a pixel?

Yes

> The change does not look too bad with the plain vanilla instance,
> however, there are three problems with the consequences of the change
> for instances of emacs started with my .emacs file:
> 
>   * the new bold creation technique fills in letters such as `m' so
>     that they become unreadable rectangles

Yes, that happens with the font I use too; however, as I said in my original
post, it actually doesn't seem to make much difference to readability.

[I'm not entirely sure _why_ it doesn't make a diffence -- I suppose that the
little blob is close enough to an `m' and there's enough context that my
brain can fill in the details.  It also helps I suppose that boldface
typically only occurs in short stretches of text, not whole buffers.  In any
case, I simply don't notice it at all, unless I explicitly look to see.]

> That is to say, when I reset the value of `mode-line-buffer-identification'
> so its face is :weight normal rather than :weight bold, that change is only
> temporary.
...
>   * I do not know how to set the value that is associated with 
>         (face (:weight bold) ...
>     in my .emacs.

As far as I know, there is no way to universally tweak face attributes.

If the only thing that concerns you is the `synthesized bold,' I suppose a
variable could be added to specifically inhibit this behavior.

However, some `real' bold fonts actually have this same problem --
characters like `m' can be only vague blobs, with the vertical strokes
undifferentiated.  Perhaps in such a case, people would like to disable
even `real' bold-face.

So maybe it would be better to add a more general feature that could also
make such a decision based on the actual font rather than universally.
[Another case might be fonts that don't have italic -- some people
might like to e.g. display an underline instead in this case]

I'm not sure how such feature should look though.

> The `m' in the word `emacs' becomes unreadable.

You might think about just trying it for a while, and seeing if you stop
noticing...

Thanks,

-Miles
-- 
P.S.  All information contained in the above letter is false,
      for reasons of military security.

Re: Bold by moving pixels problem

[Robert J. Chassell]

   Yes, that happens with the font I use too; however, as I said in my original
   post, it actually doesn't seem to make much difference to readability.

I know what you mean -- on some fonts it does not matter.  But in this
case, it is very obvious and ugly.

   If the only thing that concerns you is the `synthesized bold,' I suppose a
   variable could be added to specifically inhibit this behavior.

Nice, but not enough.

   However, some `real' bold fonts actually have this same problem --
   characters like `m' can be only vague blobs, with the vertical strokes
   undifferentiated.  Perhaps in such a case, people would like to disable
   even `real' bold-face.

Yes -- I would like to disable `real' bold-face.  

I would like everything with bold to be `normal' weight; I then use
color to indicate bold.  With this laptop screen, that works best.  (I
am using "DodgerBlue4" for background and "cyan" for foreground --
suits me well.)

   So maybe it would be better to add a more general feature that could also
   make such a decision based on the actual font rather than universally.

Yes.

   [Another case might be fonts that don't have italic -- some people
   might like to e.g. display an underline instead in this case]

Yes -- again, on this screen, my italic shape looks terrible, so I
use color for this, too.  (I have an underline face, which I could
use for italic, but color works best.)

Do please try to come up with some variable; that way I can use the
shape changers on appropriate screens and what amounts to a fixed
width sans serif font with colors for most of my other faces.

Thanks!

-- 
    Robert J. Chassell                         Rattlesnake Enterprises
    http://www.rattlesnake.com                  GnuPG Key ID: 004B4AC8
    http://www.teak.cc                             bob@rattlesnake.com

Re: Bold by moving pixels problem

[Stefan Monnier]

> I would like everything with bold to be `normal' weight; I then use
> color to indicate bold.  With this laptop screen, that works best.  (I
> am using "DodgerBlue4" for background and "cyan" for foreground --
> suits me well.)
> 
>    So maybe it would be better to add a more general feature that could also
>    make such a decision based on the actual font rather than universally.
> 
> Yes.

How about a face transformer hook called at the end of face-set-spec
as in the patch below ?
We should of course come up with useful functions to put on
that hook.  For example to turn italics into underline or bold
into something else.  Or to enforce a minimum contrast between
foreground and background color, ...


	Stefan


Index: faces.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/faces.el,v
retrieving revision 1.272
diff -u -u -b -r1.272 faces.el
--- faces.el	6 Sep 2002 07:14:29 -0000	1.272
+++ faces.el	21 Nov 2002 01:31:09 -0000
@@ -1377,7 +1379,8 @@
 	       (setq attribute nil))))
 	(when attribute
 	  (set-face-attribute face frame attribute value)))
-      (setq attrs (cdr (cdr attrs))))))
+      (setq attrs (cdr (cdr attrs)))))
+  (run-hook-with-args 'face-transformer-functions face frame))
 
 
 (defun face-attr-match-p (face attrs &optional frame)

Re: Bold by moving pixels problem

[Miles Bader]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:
> How about a face transformer hook called at the end of face-set-spec
> as in the patch below ?

The problem is that only works for customized faces, not for those
defined with lower-level functions, or for `merged' faces resulting from
text-properties, etc.  In fact I think one of the complaints Bob had was
about a face property added by the mode-line stuff.

I was envisioning a similar `face-filter,' but run somewhere just before
face-realization or thereabouts.  Of course, this approach requires more
care to avoid bjorking redisplay, or slowing things down too much.

-Miles
-- 
"1971 pickup truck; will trade for guns"

Re: Bold by moving pixels problem

[Eli Zaretskii]

On Thu, 21 Nov 2002, Robert J. Chassell wrote:

> I would like everything with bold to be `normal' weight; I then use
> color to indicate bold.  With this laptop screen, that works best.

The issue of transforming face attributes has come up before.

I think it would be a good feature to be able to tell Emacs that a certain
face attribute should be displayed as some other attribute.  An example of
bold being displayed in some distinct color is a good case in point.

Many telnet clients which run on color displays already do that.  The
MS-DOS port of Emacs before v21.1 actually implemented that by redefining
the bold and italics faces in the DOS-specific startup files. 

Re: Bold by moving pixels problem

[Miles Bader]

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

I've attached a patch below that's implements the `final face
realization filtering' function that bob wanted.  This is just a
first-cut, so I'd appreciate any comments people have.

The basic interface is a variable, `realize-face-filter':

   realize-face-filter's value is nil

   If non-nil, a function called to perturb faces before final realization.
   It is passed a lisp-vector containing all the attributes of the
   fully-specified face, and can change any that it wishes.

For instance the following will completely turn off bold fonts:

   (defun unboldify-lface (lface)
     (aset lface 4 'normal))
   (setq realize-face-filter 'unboldify-lface)
   (clear-face-cache)

It seems to work fine.

If I instrument the filter function a bit, like:

   (defvar ubf-count 0)
   (defun unboldify-lface (lface)
     (setq ubf-count (1+ ubf-count))
     (aset lface 4 'normal))
   (setq realize-face-filter 'unboldify-lface)
   (clear-face-cache)

`ubf-count' seems to continually incremented (if I keep checking in
*scratch* it goes up 2 or 3 each time, but if I display a complicated
new buffer, it can go up by 100).  It doesn't seem noticably slow to me,
but I have a reasonably fast cpu (1GHz).

I guess I'll need to see exactly what it's getting called on, but this
suggests that maybe more places should be checking the face cache
(or that the face-cache checking should be moved into realize_face).
[Gerd, are you listening...?]

Also, perhaps `realize-face-filter' should be a list of functions
instead, e.g., `realize-face-filter-functions', as there might be
circumstances when two entities want to add a filter -- but maybe this
is something that will only ever be changed by the end-user, who can
make his own filter function do everything he wants.

Any comments?

Thanks,

-Miles

Patch:



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: realize-face-filter-20021217.patch --]
[-- Type: text/x-patch, Size: 2064 bytes --]

Index: src/xfaces.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xfaces.c,v
retrieving revision 1.266
diff -u -r1.266 xfaces.c
--- src/xfaces.c	17 Nov 2002 23:51:19 -0000	1.266
+++ src/xfaces.c	17 Dec 2002 04:31:36 -0000
@@ -422,6 +422,13 @@
 
 Lisp_Object Vtty_defined_color_alist;
 
+/* If non-nil, a function called to perturb faces before final realization.
+   It is passed a lisp-vector containing all the attributes of the
+   fully-specified face, and can change any that it wishes. */
+
+Lisp_Object Vrealize_face_filter;
+
+
 /* Counter for calls to clear_face_cache.  If this counter reaches
    CLEAR_FONT_TABLE_COUNT, and a frame has more than
    CLEAR_FONT_TABLE_NFONTS load, unused fonts are freed.  */
@@ -6691,6 +6698,21 @@ realize_face (cache, attrs, c, base_face
       free_realized_face (cache->f, former_face);
     }
 
+  if (! NILP (Vrealize_face_filter))
+    {
+      /* Call a user-defined function to perturb the face attributes
+	 before realization.  */
+      Lisp_Object lface = Fmake_vector (make_number (LFACE_VECTOR_SIZE),
+					Qunspecified);
+      bcopy (attrs, XVECTOR (lface)->contents,
+	     LFACE_VECTOR_SIZE * (sizeof *attrs));
+
+      safe_call1 (Vrealize_face_filter, lface);
+
+      bcopy (XVECTOR (lface)->contents, attrs,
+	     LFACE_VECTOR_SIZE * (sizeof *attrs));
+    }      
+
   if (FRAME_WINDOW_P (cache->f))
     face = realize_x_face (cache, attrs, c, base_face);
   else if (FRAME_TERMCAP_P (cache->f) || FRAME_MSDOS_P (cache->f))
@@ -7670,6 +7692,13 @@
 Each element is a regular expression that matches names of fonts to
 ignore.  */);
   Vface_ignored_fonts = Qnil;
+
+  DEFVAR_LISP ("realize-face-filter", &Vrealize_face_filter,
+	       doc: /* If non-nil, a function called to perturb faces before final realization.
+It is passed a lisp-vector containing all the attributes of the
+fully-specified face, and can change any that it wishes. */);
+  Vrealize_face_filter = Qnil;
+
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sbitmap_spec_p);

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]



-- 
[|nurgle|]  ddt- demonic? so quake will have an evil kinda setting? one that 
            will  make every christian in the world foamm at the mouth? 
[iddt]      nurg, that's the goal 

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Bold by moving pixels problem

[Miles Bader]

I wrote:
> `ubf-count' seems to continually incremented (if I keep checking in
> *scratch* it goes up 2 or 3 each time, but if I display a complicated
> new buffer, it can go up by 100).
> 
> I guess I'll need to see exactly what it's getting called on, but this
> suggests that maybe more places should be checking the face cache
> (or that the face-cache checking should be moved into realize_face).

Duh!  I just realized why it's doing this, because with the previous
patch I sent, it _inserts_ into the cache using the post-filter-function
attributes, but _looks up_ in the cache using pre-filter-function
attributes (so cached faces never get properly found later).

I'll fix it...

-Miles
-- 
80% of success is just showing up.  --Woody Allen

Re: Bold by moving pixels problem

[Miles Bader]

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

Ok, this new patch should fix the previously mentioned problem; now the
face-caching seems to be working correctly, and realize-face-filter
doesn't get called too often (in an emacs that I've been using for a
while, it's only been called 78 times).

Thanks,

-Miles


Patch:



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: realize-face-filter-20021217-1.patch --]
[-- Type: text/x-patch, Size: 9170 bytes --]

2002-12-17  Miles Bader  <miles@gnu.org>

	* xfaces.c (Vrealize_face_filter): New variable.
	(realize_face): If Vrealize_face_filter has a non-nil value, use
	it to filter the face attributes before realization.  Use new
	calling conventsions for realize_x_face, realize_tty_face, and
	load_face_font.
	(realize_x_face, realize_tty_face): Add new argument FACE, and
	change return type to void.  Use FACE instead of creating our own.
	(load_face_font): Add a new argument ATTRS, and use it instead of
	FACE->lface.
	(syms_of_xfaces): Initialize Vrealize_face_filter.

Index: src/xfaces.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xfaces.c,v
retrieving revision 1.266
diff -u -r1.266 xfaces.c
--- src/xfaces.c	17 Nov 2002 23:51:19 -0000	1.266
+++ src/xfaces.c	17 Dec 2002 07:01:09 -0000
@@ -422,6 +422,13 @@
 
 Lisp_Object Vtty_defined_color_alist;
 
+/* If non-nil, a function called to perturb faces before final realization.
+   It is passed a lisp-vector containing all the attributes of the
+   fully-specified face, and can change any that it wishes. */
+
+Lisp_Object Vrealize_face_filter;
+
+
 /* Counter for calls to clear_face_cache.  If this counter reaches
    CLEAR_FONT_TABLE_COUNT, and a frame has more than
    CLEAR_FONT_TABLE_NFONTS load, unused fonts are freed.  */
@@ -481,7 +488,7 @@
 static unsigned char *xstrlwr P_ ((unsigned char *));
 static void signal_error P_ ((char *, Lisp_Object));
 static struct frame *frame_or_selected_frame P_ ((Lisp_Object, int));
-static void load_face_font P_ ((struct frame *, struct face *, int));
+static void load_face_font P_ ((struct frame *, struct face *, Lisp_Object *, int));
 static void load_face_colors P_ ((struct frame *, struct face *, Lisp_Object *));
 static void free_face_colors P_ ((struct frame *, struct face *));
 static int face_color_gray_p P_ ((struct frame *, char *));
@@ -502,10 +509,10 @@
 static int cmp_font_names P_ ((const void *, const void *));
 static struct face *realize_face P_ ((struct face_cache *, Lisp_Object *, int,
 				      struct face *, int));
-static struct face *realize_x_face P_ ((struct face_cache *,
-					Lisp_Object *, int, struct face *));
-static struct face *realize_tty_face P_ ((struct face_cache *,
-					  Lisp_Object *, int));
+static void realize_x_face P_ ((struct face *, struct face_cache *,
+				Lisp_Object *, int, struct face *));
+static void realize_tty_face P_ ((struct face *, struct face_cache *,
+				  Lisp_Object *, int));
 static int realize_basic_faces P_ ((struct frame *));
 static int realize_default_face P_ ((struct frame *));
 static void realize_named_face P_ ((struct frame *, Lisp_Object, int));
@@ -1245,14 +1252,15 @@
 
 #ifdef HAVE_WINDOW_SYSTEM
 
-/* Load font of face FACE which is used on frame F to display
-   character C.  The name of the font to load is determined by lface
-   and fontset of FACE.  */
+/* Load font of face FACE with attributes ATTRS which is used on frame F to
+   display character C.  The name of the font to load is determined by
+   lface and fontset of FACE.  */
 
 static void
-load_face_font (f, face, c)
+load_face_font (f, face, attrs, c)
      struct frame *f;
      struct face *face;
+     Lisp_Object *attrs;
      int c;
 {
   struct font_info *font_info = NULL;
@@ -1262,7 +1270,7 @@
   face->font_info_id = -1;
   face->font = NULL;
 
-  font_name = choose_face_font (f, face->lface, face->fontset, c,
+  font_name = choose_face_font (f, attrs, face->fontset, c,
 				&needs_overstrike);
   if (!font_name)
     return;
@@ -6678,6 +6686,11 @@
      int former_face_id;
 {
   struct face *face;
+  /* The set of attributes this face is know by to the user, as opposed to
+     the set actually used to render the face.  They're usually the same
+     as, but may be different if some attribute is changed by
+     realize-face-filter.  */
+  Lisp_Object *orig_attrs = attrs;
 
   /* LFACE must be fully specified.  */
   xassert (cache != NULL);
@@ -6691,41 +6704,59 @@
       free_realized_face (cache->f, former_face);
     }
 
+  if (! NILP (Vrealize_face_filter))
+    {
+      /* Call a user-defined function to perturb the face attributes
+	 before realization.  */
+      Lisp_Object lface = Fmake_vector (make_number (LFACE_VECTOR_SIZE),
+					Qunspecified);
+      bcopy (attrs, XVECTOR (lface)->contents,
+	     LFACE_VECTOR_SIZE * (sizeof *attrs));
+
+      safe_call1 (Vrealize_face_filter, lface);
+
+      attrs = XVECTOR (lface)->contents;
+    }      
+
+  /* Allocate a new realized face.  */
+  face = make_realized_face (orig_attrs);
+
+  /* Fill it in.  */
   if (FRAME_WINDOW_P (cache->f))
-    face = realize_x_face (cache, attrs, c, base_face);
+    realize_x_face (face, cache, attrs, c, base_face);
   else if (FRAME_TERMCAP_P (cache->f) || FRAME_MSDOS_P (cache->f))
-    face = realize_tty_face (cache, attrs, c);
+    realize_tty_face (face, cache, attrs, c);
   else
     abort ();
 
   /* Insert the new face.  */
-  cache_face (cache, face, lface_hash (attrs));
+  cache_face (cache, face, lface_hash (orig_attrs));
 #ifdef HAVE_WINDOW_SYSTEM
   if (FRAME_WINDOW_P (cache->f) && face->font == NULL)
-    load_face_font (cache->f, face, c);
+    load_face_font (cache->f, face, attrs, c);
 #endif  /* HAVE_WINDOW_SYSTEM */
   return face;
 }
 
 
-/* Realize the fully-specified face with attributes ATTRS in face
-   cache CACHE for character C.  Do it for X frame CACHE->f.  If C is
-   a multibyte character, BASE_FACE is a face that has the same
-   attributes.  Otherwise, BASE_FACE is ignored.  If the new face
-   doesn't share font with the default face, a fontname is allocated
-   from the heap and set in `font_name' of the new face, but it is not
-   yet loaded here.  Value is a pointer to the newly created realized
-   face.  */
+/* Realize into FACE the fully-specified face with attributes ATTRS in face
+   cache CACHE for character C.  Do it for X frame CACHE->f.  If C is a
+   multibyte character, BASE_FACE is a face that has the same attributes.
+   Otherwise, BASE_FACE is ignored.  If the new face doesn't share font
+   with the default face, a fontname is allocated from the heap and set in
+   `font_name' of the new face, but it is not yet loaded here.  Value is a
+   pointer to the newly created realized face.  */
 
-static struct face *
-realize_x_face (cache, attrs, c, base_face)
+static void
+realize_x_face (face, cache, attrs, c, base_face)
+     struct face *face;
      struct face_cache *cache;
      Lisp_Object *attrs;
      int c;
      struct face *base_face;
 {
 #ifdef HAVE_WINDOW_SYSTEM
-  struct face *face, *default_face;
+  struct face *default_face;
   struct frame *f;
   Lisp_Object stipple, overline, strike_through, box;
 
@@ -6733,9 +6764,6 @@
   xassert (SINGLE_BYTE_CHAR_P (c)
 	   || base_face);
 
-  /* Allocate a new realized face.  */
-  face = make_realized_face (attrs);
-
   f = cache->f;
 
   /* If C is a multibyte character, we share all face attirbutes with
@@ -6751,7 +6779,7 @@
 
       /* to force realize_face to load font */
       face->font = NULL;
-      return face;
+      return;
     }
 
   /* Now we are realizing a face for ASCII (and unibyte) characters.  */
@@ -6921,7 +6949,6 @@
     face->stipple = load_pixmap (f, stipple, &face->pixmap_w, &face->pixmap_h);
 
   xassert (FACE_SUITABLE_FOR_CHAR_P (face, c));
-  return face;
 #endif /* HAVE_WINDOW_SYSTEM */
 }
 
@@ -7012,17 +7039,16 @@
 }
 
 
-/* Realize the fully-specified face with attributes ATTRS in face
-   cache CACHE for character C.  Do it for TTY frame CACHE->f.  Value is a
-   pointer to the newly created realized face.  */
+/* Realize into FACE the fully-specified face with attributes ATTRS in face
+   cache CACHE for character C.  Do it for TTY frame CACHE->f.  */
 
-static struct face *
-realize_tty_face (cache, attrs, c)
+static void
+realize_tty_face (face, cache, attrs, c)
+     struct face *face;
      struct face_cache *cache;
      Lisp_Object *attrs;
      int c;
 {
-  struct face *face;
   int weight, slant;
   int face_colors_defaulted = 0;
   struct frame *f = cache->f;
@@ -7030,8 +7056,6 @@
   /* Frame must be a termcap frame.  */
   xassert (FRAME_TERMCAP_P (cache->f) || FRAME_MSDOS_P (cache->f));
 
-  /* Allocate a new realized face.  */
-  face = make_realized_face (attrs);
   face->font_name = FRAME_MSDOS_P (cache->f) ? "ms-dos" : "tty";
 
   /* Map face attributes to TTY appearances.  We map slant to
@@ -7068,8 +7092,6 @@
       && face->background == FACE_TTY_DEFAULT_FG_COLOR
       && face->foreground == FACE_TTY_DEFAULT_BG_COLOR)
     face->tty_bold_p = 0;
-
-  return face;
 }
 
 
@@ -7670,6 +7692,13 @@
 Each element is a regular expression that matches names of fonts to
 ignore.  */);
   Vface_ignored_fonts = Qnil;
+
+  DEFVAR_LISP ("realize-face-filter", &Vrealize_face_filter,
+	       doc: /* If non-nil, a function called to perturb faces before final realization.
+It is passed a lisp-vector containing all the attributes of the
+fully-specified face, and can change any that it wishes. */);
+  Vrealize_face_filter = Qnil;
+
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sbitmap_spec_p);

[-- Attachment #3: Type: text/plain, Size: 49 bytes --]



-- 
Quidquid latine dictum sit, altum viditur.

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Bold by moving pixels problem

[Kim F. Storm]

Miles Bader <miles@lsi.nec.co.jp> writes:

> 
> The basic interface is a variable, `realize-face-filter':

Shouldn't it be called `realize-face-filter-function' ?

> Also, perhaps `realize-face-filter' should be a list of functions
> instead, e.g., `realize-face-filter-functions', as there might be
> circumstances when two entities want to add a filter -- but maybe this
> is something that will only ever be changed by the end-user, who can
> make his own filter function do everything he wants.

You may be right, but once the realize-face-filter-function feature
has been added, some elisp package is bound to emerge which uses it to
solve some unforeseen problem... and want to add its own filter
function.  So I definitely think it should be a list (and as you said
in a subsequent mail, it really doesn't get called very often, so it
shouldn't really matter).

[Sorry, I haven't had time to look at the patch].

Re: Bold by moving pixels problem

[Robert J. Chassell]

Many thanks!

The second version of the patch that implements the `final face
realization filtering' function works fine.  I can finally read the
`m' in my *mail* buffer mode line!

I put the following in my .emacs file

      (defun unboldify-lface (lface)
        (aset lface 4 'normal))

      (setq realize-face-filter 'unboldify-lface)

      (clear-face-cache)

and all is well, as far as I have tested it.

My only issue is semantic:  I continue to have `bold' faces, but I
set `bold' to be a color, not a weight.  What it seems you are doing
is not so much `unboldifying' the face as removing a

    :weight bold

attribute.  Should we not call the defun  `face-normal-weight'?

      (defun face-normal-weight (lface)
        (aset lface 4 'normal))
      (setq realize-face-filter 'face-normal-weight)
      (clear-face-cache)

-- 
    Robert J. Chassell                         Rattlesnake Enterprises
    http://www.rattlesnake.com                  GnuPG Key ID: 004B4AC8
    http://www.teak.cc                             bob@rattlesnake.com

Re: Bold by moving pixels problem

[Miles Bader]

On Tue, Dec 17, 2002 at 04:38:57PM +0000, Robert J. Chassell wrote:
> My only issue is semantic:  I continue to have `bold' faces, but I
> set `bold' to be a color, not a weight.  What it seems you are doing
> is not so much `unboldifying' the face as removing a
> 
>     :weight bold
> 
> attribute.  Should we not call the defun  `face-normal-weight'?

Well, I didn't think very much about the name of that function.  It probably
shouldn't really use the work `face', because it doesn't operate on normal
faces, but rather the low-level internal representation of them (for which
the term `lface' is sometimes used).

You can make it translate bold into a color too, by similarly

  (defun lface-change-bold-to-yellow (lface)
    (if (memq (aref lface 4) '(bold heavy ...)) ; whatever weights you need
        (progn
	  (aset lface 4 'normal)  ; unboldify
	  (aset lface 8 "yellow"))))

WARNING: When I was trying a more complicated function like this yesterday,
my emacs aborted (in UNBLOCK_INPUT), so something funny is going on, I'm not
sure exactly what.  However it might still be too dangerous for casual use
(luckily emacs is great at not losing data, but it's still no fun when it
crashes).

-Miles
-- 
[|nurgle|]  ddt- demonic? so quake will have an evil kinda setting? one that 
            will  make every christian in the world foamm at the mouth? 
[iddt]      nurg, that's the goal 

Re: Bold by moving pixels problem

[Miles Bader]

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

Here's an updated version of my patch, that uses a list of functions:



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: realize-face-filter-20021218-0.patch --]
[-- Type: text/x-patch, Size: 9543 bytes --]

2002-12-17  Miles Bader  <miles@gnu.org>

	* xfaces.c (Vrealize_face_filter_functions): New variable.
	(realize_face): If Vrealize_face_filter_functions has a non-nil
	value, use it to filter the face attributes before realization.
	Use new calling conventions for realize_x_face, realize_tty_face,
	and load_face_font.
	(realize_x_face, realize_tty_face): Add new argument FACE, and
	change return type to void.  Use FACE instead of creating our own.
	(load_face_font): Add a new argument ATTRS, and use it instead of
	FACE->lface.
	(syms_of_xfaces): Initialize Vrealize_face_filter_functions.

Index: src/xfaces.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xfaces.c,v
retrieving revision 1.266
diff -u -r1.266 xfaces.c
--- src/xfaces.c	17 Nov 2002 23:51:19 -0000	1.266
+++ src/xfaces.c	18 Dec 2002 05:26:34 -0000
@@ -422,6 +422,13 @@
 
 Lisp_Object Vtty_defined_color_alist;
 
+/* A list of functions to perturb faces before final realization.
+   They are passed a lisp-vector containing all the attributes of the
+   fully-specified face, and can change any that they wish. */
+
+Lisp_Object Vrealize_face_filter_functions;
+
+
 /* Counter for calls to clear_face_cache.  If this counter reaches
    CLEAR_FONT_TABLE_COUNT, and a frame has more than
    CLEAR_FONT_TABLE_NFONTS load, unused fonts are freed.  */
@@ -481,7 +488,7 @@
 static unsigned char *xstrlwr P_ ((unsigned char *));
 static void signal_error P_ ((char *, Lisp_Object));
 static struct frame *frame_or_selected_frame P_ ((Lisp_Object, int));
-static void load_face_font P_ ((struct frame *, struct face *, int));
+static void load_face_font P_ ((struct frame *, struct face *, Lisp_Object *, int));
 static void load_face_colors P_ ((struct frame *, struct face *, Lisp_Object *));
 static void free_face_colors P_ ((struct frame *, struct face *));
 static int face_color_gray_p P_ ((struct frame *, char *));
@@ -502,10 +509,10 @@
 static int cmp_font_names P_ ((const void *, const void *));
 static struct face *realize_face P_ ((struct face_cache *, Lisp_Object *, int,
 				      struct face *, int));
-static struct face *realize_x_face P_ ((struct face_cache *,
-					Lisp_Object *, int, struct face *));
-static struct face *realize_tty_face P_ ((struct face_cache *,
-					  Lisp_Object *, int));
+static void realize_x_face P_ ((struct face *, struct face_cache *,
+				Lisp_Object *, int, struct face *));
+static void realize_tty_face P_ ((struct face *, struct face_cache *,
+				  Lisp_Object *, int));
 static int realize_basic_faces P_ ((struct frame *));
 static int realize_default_face P_ ((struct frame *));
 static void realize_named_face P_ ((struct frame *, Lisp_Object, int));
@@ -1245,14 +1252,15 @@
 
 #ifdef HAVE_WINDOW_SYSTEM
 
-/* Load font of face FACE which is used on frame F to display
-   character C.  The name of the font to load is determined by lface
-   and fontset of FACE.  */
+/* Load font of face FACE with attributes ATTRS which is used on frame F to
+   display character C.  The name of the font to load is determined by
+   lface and fontset of FACE.  */
 
 static void
-load_face_font (f, face, c)
+load_face_font (f, face, attrs, c)
      struct frame *f;
      struct face *face;
+     Lisp_Object *attrs;
      int c;
 {
   struct font_info *font_info = NULL;
@@ -1262,7 +1270,7 @@
   face->font_info_id = -1;
   face->font = NULL;
 
-  font_name = choose_face_font (f, face->lface, face->fontset, c,
+  font_name = choose_face_font (f, attrs, face->fontset, c,
 				&needs_overstrike);
   if (!font_name)
     return;
@@ -6678,6 +6686,11 @@
      int former_face_id;
 {
   struct face *face;
+  /* The set of attributes this face is know by to the user, as opposed to
+     the set actually used to render the face.  They're usually the same
+     as, but may be different if some attribute is changed by
+     realize-face-filter.  */
+  Lisp_Object *orig_attrs = attrs;
 
   /* LFACE must be fully specified.  */
   xassert (cache != NULL);
@@ -6691,41 +6704,70 @@
       free_realized_face (cache->f, former_face);
     }
 
+  if (CONSP (Vrealize_face_filter_functions))
+    {
+      /* Call these user-defined functions to perturb the face attributes
+	 before realization.  */
+      Lisp_Object filters, cycle_check;
+      Lisp_Object lface = Fmake_vector (make_number (LFACE_VECTOR_SIZE),
+					Qunspecified);
+
+      bcopy (attrs, XVECTOR (lface)->contents,
+	     LFACE_VECTOR_SIZE * (sizeof *attrs));
+
+      cycle_check = Qnil;
+      for (filters = Vrealize_face_filter_functions;
+	   CONSP (filters);
+	   filters = XCDR (filters))
+	{
+	  safe_call1 (XCAR (filters), lface);
+	  cycle_check = CYCLE_CHECK (cycle_check, filters, 50);
+	  if (NILP (cycle_check))
+	    break;		/* cycle detected */
+	}
+
+      attrs = XVECTOR (lface)->contents;
+    }      
+
+  /* Allocate a new realized face.  */
+  face = make_realized_face (orig_attrs);
+
+  /* Fill it in.  */
   if (FRAME_WINDOW_P (cache->f))
-    face = realize_x_face (cache, attrs, c, base_face);
+    realize_x_face (face, cache, attrs, c, base_face);
   else if (FRAME_TERMCAP_P (cache->f) || FRAME_MSDOS_P (cache->f))
-    face = realize_tty_face (cache, attrs, c);
+    realize_tty_face (face, cache, attrs, c);
   else
     abort ();
 
   /* Insert the new face.  */
-  cache_face (cache, face, lface_hash (attrs));
+  cache_face (cache, face, lface_hash (orig_attrs));
 #ifdef HAVE_WINDOW_SYSTEM
   if (FRAME_WINDOW_P (cache->f) && face->font == NULL)
-    load_face_font (cache->f, face, c);
+    load_face_font (cache->f, face, attrs, c);
 #endif  /* HAVE_WINDOW_SYSTEM */
   return face;
 }
 
 
-/* Realize the fully-specified face with attributes ATTRS in face
-   cache CACHE for character C.  Do it for X frame CACHE->f.  If C is
-   a multibyte character, BASE_FACE is a face that has the same
-   attributes.  Otherwise, BASE_FACE is ignored.  If the new face
-   doesn't share font with the default face, a fontname is allocated
-   from the heap and set in `font_name' of the new face, but it is not
-   yet loaded here.  Value is a pointer to the newly created realized
-   face.  */
+/* Realize into FACE the fully-specified face with attributes ATTRS in face
+   cache CACHE for character C.  Do it for X frame CACHE->f.  If C is a
+   multibyte character, BASE_FACE is a face that has the same attributes.
+   Otherwise, BASE_FACE is ignored.  If the new face doesn't share font
+   with the default face, a fontname is allocated from the heap and set in
+   `font_name' of the new face, but it is not yet loaded here.  Value is a
+   pointer to the newly created realized face.  */
 
-static struct face *
-realize_x_face (cache, attrs, c, base_face)
+static void
+realize_x_face (face, cache, attrs, c, base_face)
+     struct face *face;
      struct face_cache *cache;
      Lisp_Object *attrs;
      int c;
      struct face *base_face;
 {
 #ifdef HAVE_WINDOW_SYSTEM
-  struct face *face, *default_face;
+  struct face *default_face;
   struct frame *f;
   Lisp_Object stipple, overline, strike_through, box;
 
@@ -6733,9 +6775,6 @@
   xassert (SINGLE_BYTE_CHAR_P (c)
 	   || base_face);
 
-  /* Allocate a new realized face.  */
-  face = make_realized_face (attrs);
-
   f = cache->f;
 
   /* If C is a multibyte character, we share all face attirbutes with
@@ -6751,7 +6790,7 @@
 
       /* to force realize_face to load font */
       face->font = NULL;
-      return face;
+      return;
     }
 
   /* Now we are realizing a face for ASCII (and unibyte) characters.  */
@@ -6921,7 +6960,6 @@
     face->stipple = load_pixmap (f, stipple, &face->pixmap_w, &face->pixmap_h);
 
   xassert (FACE_SUITABLE_FOR_CHAR_P (face, c));
-  return face;
 #endif /* HAVE_WINDOW_SYSTEM */
 }
 
@@ -7012,17 +7050,16 @@
 }
 
 
-/* Realize the fully-specified face with attributes ATTRS in face
-   cache CACHE for character C.  Do it for TTY frame CACHE->f.  Value is a
-   pointer to the newly created realized face.  */
+/* Realize into FACE the fully-specified face with attributes ATTRS in face
+   cache CACHE for character C.  Do it for TTY frame CACHE->f.  */
 
-static struct face *
-realize_tty_face (cache, attrs, c)
+static void
+realize_tty_face (face, cache, attrs, c)
+     struct face *face;
      struct face_cache *cache;
      Lisp_Object *attrs;
      int c;
 {
-  struct face *face;
   int weight, slant;
   int face_colors_defaulted = 0;
   struct frame *f = cache->f;
@@ -7030,8 +7067,6 @@
   /* Frame must be a termcap frame.  */
   xassert (FRAME_TERMCAP_P (cache->f) || FRAME_MSDOS_P (cache->f));
 
-  /* Allocate a new realized face.  */
-  face = make_realized_face (attrs);
   face->font_name = FRAME_MSDOS_P (cache->f) ? "ms-dos" : "tty";
 
   /* Map face attributes to TTY appearances.  We map slant to
@@ -7068,8 +7103,6 @@
       && face->background == FACE_TTY_DEFAULT_FG_COLOR
       && face->foreground == FACE_TTY_DEFAULT_BG_COLOR)
     face->tty_bold_p = 0;
-
-  return face;
 }
 
 
@@ -7670,6 +7703,14 @@
 Each element is a regular expression that matches names of fonts to
 ignore.  */);
   Vface_ignored_fonts = Qnil;
+
+  DEFVAR_LISP ("realize-face-filter-functions",
+	       &Vrealize_face_filter_functions,
+	       doc:/* A list of functions to perturb faces before final realization.
+They are passed a lisp-vector containing all the attributes of the
+fully-specified face, and can change any that they wish. */);
+  Vrealize_face_filter_functions = Qnil;
+
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sbitmap_spec_p);

[-- Attachment #3: Type: text/plain, Size: 59 bytes --]



Also, here's a lisp file that implements some filters:



[-- Attachment #4: Some filters for face-realization --]
[-- Type: application/emacs-lisp, Size: 2270 bytes --]

[-- Attachment #5: Type: text/plain, Size: 165 bytes --]



-Miles
-- 
I'm beginning to think that life is just one long Yoko Ono album; no rhyme
or reason, just a lot of incoherent shrieks and then it's over.  --Ian Wolff

[-- Attachment #6: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Bold by moving pixels problem

[Kim F. Storm]

Miles Bader <miles@lsi.nec.co.jp> writes:

> Here's an updated version of my patch, that uses a list of functions:
> 
> 
> 
> 2002-12-17  Miles Bader  <miles@gnu.org>
> 
> 	* xfaces.c (Vrealize_face_filter_functions): New variable.
> 	(realize_face): If Vrealize_face_filter_functions has a non-nil
> 	value, use it to filter the face attributes before realization.
> 	Use new calling conventions for realize_x_face, realize_tty_face,
> 	and load_face_font.


I've looked at your patch, and it seems to be very low-level to give
the filter functions direct access to the actual lface vector elements.

I'd suggest a different approach using the internal-lisp-face attribute
names that are already defined in xfaces.c such as :family, :height, etc.

Instead of the raw lface vector, the filter function should receive
a plist with all of the current lface attributes (:family X :height Y ...),
and it should return a (possibly) modified plist ... which is then
passed on to the next filter function in the list.

When all filter functions have been called, the plist is used to
build the final lface vector.

It might be a little less efficient than using the raw vector, but
this doesn't get called very often anyway (according to your own
statistics).  And not at all if Vrealize_face_filter_functions is nil.

Example:
 
Your code:
> 
> (defun lface-emulate-bold-with-color (lface)
>   (if (memq (aref lface 4)
> 	    '(bold heavy extra-bold semi-bold ultra-bold))
>       (let ((fg-vals (color-values (aref lface 8))))
> 	;; unboldify
> 	(aset lface 4 'normal)
> 	;; Tweak the fg color to indicate bold.
> 	(cond ((equal fg-vals '(65535 65535 65535))
> 	       (aset lface 8 "khaki"))
> 	      ((equal fg-vals '(0 0 0))
> 	       (aset lface 8 "orange4"))
> 	      (t
> 	       ;; intensify the color
> 	       (aset lface 8 (highlight-color (aref lface 8) 2 5000)))))))

becomes something like this: [not tested]

(defun lface-emulate-bold-with-color (lface)
  (let ((weight (plist-get lface :weight))
        (fg (plist-get lface :foreground)))
    (if (memq weight '(bold heavy extra-bold semi-bold ultra-bold))
        (let ((fg-vals (color-values fg)))
	  ;; unboldify
	  (setq lface (plist-put lface :weight 'normal))
	  ;; Tweak the fg color to indicate bold.
	  (cond ((equal fg-vals '(65535 65535 65535))
		 (setq lface (plist-put lface :foreground "khaki")))
		((equal fg-vals '(0 0 0))
		 (setq lface (plist-put lface :foreground "orange4")))
		(t
		 ;; intensify the color
		 (setq lface (plist-put lface :foreground
					(highlight-color fg 2 5000))))))))
  lface)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Bold by moving pixels problem

[Robert J. Chassell]

On 18 Dec 2002 19:01:01 +0900, Miles Bader <miles@lsi.nec.co.jp> wrote:

   Here's an updated version of my patch, that uses a list of functions:

Success report.

This works so far, using an even more updated
CVS snapshot from today, Wed, 2002 Dec 18  13:25 UTC,
GNU Emacs 21.3.50.31 (i686-pc-linux-gnu, X toolkit)
on a concurrently updated
Debian GNU/Linux, `sarge' (`testing'), kernel 2.4.18 system
under GNOME/sawfish, with my usual .emacs file.

Thanks!

-- 
    Robert J. Chassell                         Rattlesnake Enterprises
    http://www.rattlesnake.com                  GnuPG Key ID: 004B4AC8
    http://www.teak.cc                             bob@rattlesnake.com

Re: Bold by moving pixels problem

[Miles Bader]

storm@cua.dk (Kim F. Storm) writes:
> I've looked at your patch, and it seems to be very low-level to give
> the filter functions direct access to the actual lface vector elements.

Well, it's a low-level thing... :-)

> I'd suggest a different approach using the internal-lisp-face attribute
> names that are already defined in xfaces.c such as :family, :height, etc.

I agree, code filled with `(aref .. 3)' is not good.  Going all the
way to a plist seems a bit gratuitous though.

What about just providing accessor/setter macros for `lface' vectors,
e.g., (lface-weight LFACE) and (set-lface-weight LFACE VAL)?

After all the `lface' representation _is_ exposed to lisp already, so
such macros might help other code as well.

[I'm not sure about the term `lface' though -- it's used fairly
 pervasively in the C code, but of course that's from the perspective of
 the C code...]

-Miles
-- 
The secret to creativity is knowing how to hide your sources.
  --Albert Einstein

Re: Bold by moving pixels problem

[Miles Bader]

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

BTW, here's a version of the face-filter lisp example code I sent
before, rewritten to use some macros instead; it's quite a bit more
readable!  I used the term `facevec' instead of `lface', since it's a
bit more obvious, but that's just a suggestion...

-Miles



[-- Attachment #2: Example realize-face-filter-functions filters --]
[-- Type: application/emacs-lisp, Size: 3594 bytes --]

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]



-- 
`Cars give people wonderful freedom and increase their opportunities.
 But they also destroy the environment, to an extent so drastic that
 they kill all social life' (from _A Pattern Language_)

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Bold by moving pixels problem

[Miles Bader]

storm@cua.dk (Kim F. Storm) writes:
> > What about just providing accessor/setter macros for `lface' vectors,
> > e.g., (lface-weight LFACE) and (set-lface-weight LFACE VAL)?
> 
> IMO, That's just adding stuff to remedy the wrong approach; the plist
> approach doesn't need adding _any_ new stuff at the lisp level.

But it adds a fair amount of hair at the C level -- and a few lisp
macros is not something to worry about.

> > After all the `lface' representation _is_ exposed to lisp already, so
> > such macros might help other code as well.
> 
> Excuse my ignorance, but where is that exposed already?

See `face-attributes-as-vector'.

> > [I'm not sure about the term `lface' though -- it's used fairly
> >  pervasively in the C code, but of course that's from the perspective of
> >  the C code...]
> 
> It seems to be called (internal-)lisp-face in stuff like
> internal-make-lisp-face (try apropos on "internal lisp face").
> lisp-face-vector may be a good name?

No, that's too long, and really, the term `lisp-' is rather silly in
this context.

-Miles
-- 
Yo mama's so fat when she gets on an elevator it HAS to go down.

Re: Bold by moving pixels problem

[Kim F. Storm]

Miles Bader <miles@lsi.nec.co.jp> writes:

> storm@cua.dk (Kim F. Storm) writes:
> > I've looked at your patch, and it seems to be very low-level to give
> > the filter functions direct access to the actual lface vector elements.
> 
> Well, it's a low-level thing... :-)
> 
> > I'd suggest a different approach using the internal-lisp-face attribute
> > names that are already defined in xfaces.c such as :family, :height, etc.
> 
> I agree, code filled with `(aref .. 3)' is not good.  Going all the
> way to a plist seems a bit gratuitous though.

IMHO, the only argument for the current implementation vs. a plist would be
performance.

> 
> What about just providing accessor/setter macros for `lface' vectors,
> e.g., (lface-weight LFACE) and (set-lface-weight LFACE VAL)?

IMO, That's just adding stuff to remedy the wrong approach; the plist approach
doesn't need adding _any_ new stuff at the lisp level.

> 
> After all the `lface' representation _is_ exposed to lisp already, so
> such macros might help other code as well.

Excuse my ignorance, but where is that exposed already?

> 
> [I'm not sure about the term `lface' though -- it's used fairly
>  pervasively in the C code, but of course that's from the perspective of
>  the C code...]

It seems to be called (internal-)lisp-face in stuff like internal-make-lisp-face
(try apropos on "internal lisp face").  lisp-face-vector may be a good name?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Bold by moving pixels problem

[Miles Bader]

BTW, there's another possibility, which might be better than either the
`macro' or `plist' approaches --

A while back I toyed with the idea of using face-vectors as `anonymous'
faces, since it's often a pain to have to name a face.

On reason I didn't really do anything was that I figured there are
probably places, in the redisplay code especially, which wouldn't work
well without a named face (though at the time I wanted to make
anonymous faces to inherit from, which should work fine).

However, in many places, it's trival -- in particular
`internal-get-lisp-face-attribute' and `internal-set-lisp-face-attribute',
since they use vectors internally and just translate the face-symbol into a
vector at their start (the latter function would require a bit more tweaking,
but as far as I could see, it's still fair to call it `trivial').

Now if those two functions were changed to allow `anonymous' faces (face
vectors), then such functions as `face-attribute', `set-face-attribute',
`make-face-bold', etc., would all start working on face-vectors too!

That way, functions in realize-face-filter-functions could still accept face-
vectors, avoiding the plist translation step, but also use the same familiar
face functions that users already know about; this seems like a huge win to
me... 

[p.s. I'd still like to also allow anonymous faces in more places, but that's
a separate issue]

-Miles
-- 
80% of success is just showing up.  --Woody Allen

Re: Bold by moving pixels problem

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> Now if those two functions were changed to allow `anonymous' faces (face
> vectors), then such functions as `face-attribute', `set-face-attribute',
> `make-face-bold', etc., would all start working on face-vectors too!

Bright idea!

> 
> That way, functions in realize-face-filter-functions could still accept face-
> vectors, avoiding the plist translation step, but also use the same familiar
> face functions that users already know about; this seems like a huge win to
> me... 

I like that!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Bold by moving pixels problem

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> A while back I toyed with the idea of using face-vectors as `anonymous'
> faces, since it's often a pain to have to name a face.

> However, in many places, it's trival -- in particular
> `internal-get-lisp-face-attribute' and `internal-set-lisp-face-attribute',
> since they use vectors internally and just translate the face-symbol into a
> vector at their start (the latter function would require a bit more tweaking,
> but as far as I could see, it's still fair to call it `trivial').
> 
> Now if those two functions were changed to allow `anonymous' faces (face
> vectors), then such functions as `face-attribute', `set-face-attribute',
> `make-face-bold', etc., would all start working on face-vectors too!
> 
> That way, functions in realize-face-filter-functions could still accept face-
> vectors, avoiding the plist translation step, but also use the same familiar
> face functions that users already know about; this seems like a huge win to
> me... 

Me too.

Did you try to implement this?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Bold by moving pixels problem

[Miles Bader]

On Tue, Jan 07, 2003 at 12:02:19PM +0100, Kim F. Storm wrote:
> > A while back I toyed with the idea of using face-vectors as `anonymous'
> > faces, since it's often a pain to have to name a face.
...
> > That way, functions in realize-face-filter-functions could still accept
> > face- vectors, avoiding the plist translation step, but also use the same
> > familiar face functions that users already know about; this seems like a
> > huge win to me...
> 
> Did you try to implement this?

I actually did so a long time ago but never went further, so maybe I can dig
up the patch I made.

The problem is that the signal/mouse-face problem I ran into make it a bit
dangerous to add the face-filtering feature in general, until that's solved
somehow.

-Miles
-- 
80% of success is just showing up.  --Woody Allen

Re: Bold by moving pixels problem

[Richard Stallman]

It might also make sense to use gensyms or other uninterned symbols as
anonymous faces.  That way, they could have informal names that would
help humans understand what's going on when they're in use.

Re: Bold by moving pixels problem

[Miles Bader]

Richard Stallman <rms@gnu.org> writes:
> It might also make sense to use gensyms or other uninterned symbols as
> anonymous faces.  That way, they could have informal names that would
> help humans understand what's going on when they're in use.

Doing that would make them a lot more expensive, I think -- right now in
places such as the face-realization-filter code that was being talked
about, you've already got the face in the form of a vector, so all
you've got to do is box it and pass it on.

Requiring them to be named would require you to crank up all the rest of
the face creation machinery; currently it would also prevent the face
from ever being GCed (since emacs keeps a permanent list of all named
faces).

My idea, at least in this case, is something that can be used as a
temporary `in between' face, e.g., to represent the result of face
manipulation in the same way that the C code currently uses face
vectors.  Such faces are almost always going to be very short-lived so
it's arguable whether such anymous names would even be useful in
debugging.

-Miles
-- 
Fast, small, soon; pick any 2.

Re: Bold by moving pixels problem

[Richard Stallman]

This patch makes it possible to GC inside a lot of places
that formerly could not.  A list of them is below.
I would expect that some of them don't GCPRO what they need to,
but I have not checked them for that.

It also looks like eval can in principle be called from a signal
handler.  We could solve that problem if we move all X event
processing outside of the signal handler, as someone suggested.  That
would mean that mouse highlighting doesn't update if you move the
mouse while a command is running, and the Emacs frame would not
rewrite itself if you move another window across it while a command is
running.  I think that would be a very noticeable step backwards.

Is there a way to get the job done by having the user
specify something other than Lisp code to run?



realize_face
lookup_face
realize_default_face
realize_named_face
lookup_named_face
smaller_face
face_with_height
lookup_derived_face
compute_char_face
face_at_buffer_position
face_at_string_position
face_for_char
realize_basic_faces
ascii_face_of_lisp_face
highlight_trailing_whitespace
get_overlay_arrow_glyph_row
handle_face_prop
note_mouse_highlight
note_mouse_movement
(I did not search for the callers of the ones below.)
display_string
init_frame_faces
recompute_basic_faces
update_face_from_frame_parameter
next_element_from_display_vector
direct_output_for_insert
display_line
XTframe_up_to_date
redo_mouse_highlight
expose_frame
handle_one_xevent
x_dispatch_event
XTread_socket

Re: Bold by moving pixels problem

[Stefan Monnier]

> It also looks like eval can in principle be called from a signal
> handler.  We could solve that problem if we move all X event
> processing outside of the signal handler, as someone suggested.  That
> would mean that mouse highlighting doesn't update if you move the
> mouse while a command is running, and the Emacs frame would not
> rewrite itself if you move another window across it while a command is
> running.  I think that would be a very noticeable step backwards.

That's not quite accurate.  The updating is about as immediate with
my synchronous signal handler as it is with the current async
signal handler, because it can take place any time QUIT is used,
which means "also in the middle of elisp code".

Of course, this means that it's still unsafe to `eval' code
from a synchronous signal handler, so it wouldn't help in
the case of Miles's code.

I've been running with synchronous signal handlers since I mentioned
it on this list and I'm pretty happy with it.


	Stefan

Re: Bold by moving pixels problem

[Kim F. Storm]

Before we accept Miles' patch, I would like to remind you that we
talked about a much better aproach the last time the issue was
raised (it probably has the same problems with signal handlers
and GC):

> Date: 19 Dec 2002 21:25:49 +0900
> From: Miles Bader <miles@gnu.org>
> 
> A while back I toyed with the idea of using face-vectors as `anonymous'
> faces, since it's often a pain to have to name a face.
> 
> On reason I didn't really do anything was that I figured there are
> probably places, in the redisplay code especially, which wouldn't work
> well without a named face (though at the time I wanted to make
> anonymous faces to inherit from, which should work fine).
> 
> However, in many places, it's trival -- in particular
> `internal-get-lisp-face-attribute' and `internal-set-lisp-face-attribute',
> since they use vectors internally and just translate the face-symbol into a
> vector at their start (the latter function would require a bit more tweaking,
> but as far as I could see, it's still fair to call it `trivial').
> 
> Now if those two functions were changed to allow `anonymous' faces (face
> vectors), then such functions as `face-attribute', `set-face-attribute',
> `make-face-bold', etc., would all start working on face-vectors too!
> 
> That way, functions in realize-face-filter-functions could still accept face-
> vectors, avoiding the plist translation step, but also use the same familiar
> face functions that users already know about; this seems like a huge win to
> me... 
> 
> [p.s. I'd still like to also allow anonymous faces in more places, but that's
> a separate issue]
> 



Richard Stallman <rms@gnu.org> writes:

> This patch makes it possible to GC inside a lot of places
> that formerly could not.  A list of them is below.
> I would expect that some of them don't GCPRO what they need to,
> but I have not checked them for that.

> 
> It also looks like eval can in principle be called from a signal
> handler.  We could solve that problem if we move all X event
> processing outside of the signal handler, as someone suggested.  That
> would mean that mouse highlighting doesn't update if you move the
> mouse while a command is running, and the Emacs frame would not
> rewrite itself if you move another window across it while a command is
> running.  I think that would be a very noticeable step backwards.
> 

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Bold by moving pixels problem

[Richard Stallman]

    That's not quite accurate.  The updating is about as immediate with
    my synchronous signal handler as it is with the current async
    signal handler, because it can take place any time QUIT is used,
    which means "also in the middle of elisp code".

Please forgive my confusion.

    I've been running with synchronous signal handlers since I mentioned
    it on this list and I'm pretty happy with it.

I think it would be good for more people to try it.

Re: Bold by moving pixels problem

[Miles Bader]

On Fri, Jun 06, 2003 at 03:45:19AM +0200, Kim F. Storm wrote:
> > I don't see how use of face vectors would address Bob's issue,
> > which is about customizing the way face attributes are merged.
> 
> The issue is whether we can use "standard" functions to manipulate the
> face vectors (which corresponds to face attributes), or we need to use
> (aref ...) to access the face attributes as in Miles' original patch
> to xfaces.c.

Yeah the dicussed implementation was nicer than what Bob's currently using,
but still it involves running lisp code during font selection, which is the
main problem Richard's concerned about...

-Miles
-- 
Would you like fries with that?

Re: Bold by moving pixels problem

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

> I don't see how use of face vectors would address Bob's issue,
> which is about customizing the way face attributes are merged.

The issue is whether we can use "standard" functions to manipulate the
face vectors (which corresponds to face attributes), or we need to use
(aref ...) to access the face attributes as in Miles' original patch
to xfaces.c.

I think Miles can explain this better that I can...


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Bold by moving pixels problem

[Robert J. Chassell]

    .... That could be different colors, different font or whatever. ....

Speaking of fonts, has anyone permanently fixed the font problem I
reported last November?  I am still using the second patch from Miles
on 18 Dec 2002, which solves the problem for me personally.

I just checked. The font problem still exists with 

    Today's CVS snapshot, Mon, 2003 Aug 11  18:39 UTC
    GNU Emacs 21.3.50.29 (i686-pc-linux-gnu, X toolkit)

and is fixed when I patch emacs/src/xfaces.c with what Miles sent.


Miles said his second patch should not be widely used since it could
cause a race condition.  While I suffered initially, I have not
suffered any problems for several months and wonder if other changes
to the emacs/src/xfaces.c code have taken care of the potential
problem.  Or have I just been lucky?

To remind you, this is the font issue:

    Date: Wed, 20 Nov 2002 13:09:10 +0000 (UTC)
    Subject: Bold by moving pixels problem
    From: "Robert J. Chassell" <bob@rattlesnake.com>

    ... on 19 Nov 2002, mode-line-buffer-identification
    suddenly started showing buffer names in bold.

    This applies both to a plain vanilla instance of Emacs started
    with:

         /usr/local/bin/emacs -q --no-site-file --eval '(blink-cursor-mode 0)'

    and to the instance I start with a .emacs file.

    The change does not look too bad with the plain vanilla instance.

    However, there are three problems with the consequences of the
    change for instances of emacs started with my .emacs file:

      * the new bold creation technique fills in letters such as `m'
        so that they become unreadable rectangles

        I am using a `10x20' font,
            -Misc-Fixed-Medium-R-Normal--20-200-75-75-C-100-ISO8859-1
        which has been very clear for the screen I am using.

      * I cannot permanently change
            :weight bold
        to
            :weight normal
        in the variable `mode-line-buffer-identification', which is
        buffer-local.

        That is to say, when I reset the value of
        `mode-line-buffer-identification' so its face is :weight
        normal rather than :weight bold, that change is only
        temporary.  I don't know what to write in my .emacs file that
        can make a permanent global change to a variable that is
        buffer local.  If there is a method please tell me!

      * I do not know how to set the value that is associated with

            (face (:weight bold) ...

        in my .emacs.

        Put another way, evaluating the following works temporarily
        but not permanently:

            (setq
             mode-line-buffer-identification
             (quote
              (#("%14b" 0 4
                 (face
                  ;; (:weight bold)
                  (:weight normal)
                  help-echo
                  "mouse-1: other buffer, mouse-2: prev, M-mouse-2: next, mouse-3: buffer menu"
                  local-map
                  (keymap
                   (header-line keymap
                                (down-mouse-3 . mouse-buffer-menu)
                                (mouse-2 . bury-buffer)
                                (M-mouse-2 . mode-line-unbury-buffer)
                                (mouse-1 . mode-line-other-buffer))
                   (mode-line keymap
                              (down-mouse-3 . mouse-buffer-menu)
                              (mouse-2 . bury-buffer)
                              (M-mouse-2 . mode-line-unbury-buffer))))))
              ))


        On the other hand, the following produces the face that I
        specify when I evaluate it:

            (custom-set-faces
            ...
             '(bold ((t (:background "DodgerBlue4" :foreground "cyan"))))
            ...)

        How do I reset the bold characteristic of the face in
        `mode-line-buffer-identification' when it is a local variable?

We had another discussion about this in May 2003.

Miles' second patch is in this message:

    Subject: Re: Bold by moving pixels problem
    Date: 18 Dec 2002 19:01:01 +0900
    From: Miles Bader <miles@lsi.nec.co.jp>
    To: bob@rattlesnake.com
    Cc: emacs-devel@gnu.org
    Reply-To: Miles Bader <miles@gnu.org>
    In-Reply-To: <buoisxtazqu.fsf@mcspd15.ucom.lsi.nec.co.jp>

Thanks!

--
    Robert J. Chassell                         Rattlesnake Enterprises
    http://www.rattlesnake.com                  GnuPG Key ID: 004B4AC8
    http://www.teak.cc                             bob@rattlesnake.com

Re: Bold by moving pixels problem

[Stefan Monnier]

>     That's not quite accurate.  The updating is about as immediate with
>     my synchronous signal handler as it is with the current async
>     signal handler, because it can take place any time QUIT is used,
>     which means "also in the middle of elisp code".
> 
> Please forgive my confusion.
> 
>     I've been running with synchronous signal handlers since I mentioned
>     it on this list and I'm pretty happy with it.
> 
> I think it would be good for more people to try it.

I have installed patches to that effect.
By default, nothing's changed.  But if you define SYNC_INPUT, then signal
handlers will only set a variable and the actual processing will be done
from UNBLOCK_INPUT or from QUIT, whichever occurs sooner.

Please try it out.  Normally, the only problem you might encounter with the
patch is that Emacs gets unresponsive.  If that happens, please try to
figure out how to reproduce it, and also try to get a (bunch of) C backtrace
of when the unresponsiveness happens.

I've been using a very similar patch for a while now with no problems.

If SYNC_INPUT has no ill effect, it would be good to make it the default
unconditionally.  This would allow us to get rid of the ugly BLOCK_INPUT
wrappers in alloc.c, for one thing.  It would also make Emacs more stable by
removing a whole class of hard to reproduce bugs (we've squashed some of
those over time, but as the recent mallopt problem showed, there are still
several lurking).


        Stefan