question about frame local variable

question about frame local variable

[Kenichi Handa]

It seems that we need the attached patch to make a code like
this (allow scalable fonts only for a specific frame) work
well.

(defun make-frame-allowing-scalable-fonts ()
  (make-variable-frame-local 'scalable-fonts-allowed)
  (let ((frame (make-frame '((name . "SCALABLE")))))
    (modify-frame-parameters frame '((scalable-fonts-allowed . t)))
    frame))

The patch accesses the value of `scalable-fonts-allowed' not
directly by Vscalable_fonts_allowed but via:
   find_symbol_value (Qscalable_fonts_allowed)

Is this the correct way?  Shall I install it?

---
Ken'ichi HANDA
handa@m17n.org

*** xfaces.c.~1.284.~	Tue Sep  2 08:26:41 2003
--- xfaces.c	Fri Oct 24 16:48:04 2003
***************
*** 6162,6174 ****
  may_use_scalable_font_p (font)
       const char *font;
  {
!   if (EQ (Vscalable_fonts_allowed, Qt))
      return 1;
!   else if (CONSP (Vscalable_fonts_allowed))
      {
        Lisp_Object tail, regexp;
  
!       for (tail = Vscalable_fonts_allowed; CONSP (tail); tail = XCDR (tail))
  	{
  	  regexp = XCAR (tail);
  	  if (STRINGP (regexp)
--- 6162,6177 ----
  may_use_scalable_font_p (font)
       const char *font;
  {
!   Lisp_Object val;
! 
!   val = find_symbol_value (Qscalable_fonts_allowed);
!   if (EQ (val, Qt))
      return 1;
!   else if (CONSP (val))
      {
        Lisp_Object tail, regexp;
  
!       for (tail = val; CONSP (tail); tail = XCDR (tail))
  	{
  	  regexp = XCAR (tail);
  	  if (STRINGP (regexp)
***************
*** 6364,6370 ****
  	}
  
        /* Try all scalable fonts before giving up.  */
!       if (nfonts == 0 && ! EQ (Vscalable_fonts_allowed, Qt))
  	{
  	  int count = SPECPDL_INDEX ();
  	  specbind (Qscalable_fonts_allowed, Qt);
--- 6367,6374 ----
  	}
  
        /* Try all scalable fonts before giving up.  */
!       if (nfonts == 0
! 	  && ! EQ (find_symbol_value (Qscalable_fonts_allowed), Qt))
  	{
  	  int count = SPECPDL_INDEX ();
  	  specbind (Qscalable_fonts_allowed, Qt);

Re: question about frame local variable

[Richard Stallman]

    It seems that we need the attached patch to make a code like
    this (allow scalable fonts only for a specific frame) work
    well.

    (defun make-frame-allowing-scalable-fonts ()
      (make-variable-frame-local 'scalable-fonts-allowed)
      (let ((frame (make-frame '((name . "SCALABLE")))))
	(modify-frame-parameters frame '((scalable-fonts-allowed . t)))
	frame))

    The patch accesses the value of `scalable-fonts-allowed' not
    directly by Vscalable_fonts_allowed but via:

is there a bug in handling frame-local bindings for variables
forwarded to C vars?  if so, can we fix that bug?

Re: question about frame local variable

[Kenichi Handa]

In article <E1ADjMF-0005OQ-HM@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:
>     It seems that we need the attached patch to make a code like
>     this (allow scalable fonts only for a specific frame) work
>     well.

>     (defun make-frame-allowing-scalable-fonts ()
>       (make-variable-frame-local 'scalable-fonts-allowed)
>       (let ((frame (make-frame '((name . "SCALABLE")))))
> 	(modify-frame-parameters frame '((scalable-fonts-allowed . t)))
> 	frame))

>     The patch accesses the value of `scalable-fonts-allowed' not
>     directly by Vscalable_fonts_allowed but via:

> is there a bug in handling frame-local bindings for variables
> forwarded to C vars?  if so, can we fix that bug?

I don't know how "frame-local bindings for variables" should
be treated in C code.  So I can't tell if there's a bug or
not.

Ideally, I think selecte-frame should change the value of
Vscalable_fonts_allowed when that has frame local binding.
Then, C code can simply refer to this variable to get the
current value.

But, apparently, Fselect_frame and do_switch_frame called
from it does nothing about such variables.  Only via
find_symbol_value, C code gets the current value.

On the other hand, set_buffer_internal_1 surely pays
attention to buffer local variables so that C variables get
the current value.

---
Ken'ichi HANDA
handa@m17n.org

Re: question about frame local variable

[Kenichi Handa]

CCed to Gerd.

In article <200310280803.RAA05883@etlken.m17n.org>, Kenichi Handa <handa@m17n.org> writes:
> Ideally, I think selecte-frame should change the value of
> Vscalable_fonts_allowed when that has frame local binding.
> Then, C code can simply refer to this variable to get the
> current value.

I found that the above is not enough.  It seems that the
display engine doesn't issue select-frame; i.e. provided
that there are two frames A and B, there is a case that the
display engine redisplays A while the selected frame is B.

It means that even if we make such variables frame local
that affect the redisplaying (font-selection, etc), any
frame local bindings are just ignored.

Gerd, do you remember this matter?  Is my analysis correct?

---
Ken'ichi HANDA
handa@m17n.org

Re: question about frame local variable

[Gerd Moellmann]

Kenichi Handa <handa@m17n.org> writes:

> CCed to Gerd.
> 
> In article <200310280803.RAA05883@etlken.m17n.org>, Kenichi Handa <handa@m17n.org> writes:
> > Ideally, I think selecte-frame should change the value of
> > Vscalable_fonts_allowed when that has frame local binding.
> > Then, C code can simply refer to this variable to get the
> > current value.
> 
> I found that the above is not enough.  It seems that the
> display engine doesn't issue select-frame; i.e. provided
> that there are two frames A and B, there is a case that the
> display engine redisplays A while the selected frame is B.
> 
> It means that even if we make such variables frame local
> that affect the redisplaying (font-selection, etc), any
> frame local bindings are just ignored.
> 
> Gerd, do you remember this matter?  Is my analysis correct?

I think so; it matches what I remember.  Local bindings are not
supported for all redisplay variables.  AFAIR, that has always 
been the case and isn't new in 21.  Where it is supported,
I think redisplay looks at frame parameters.

(BTW, calling select-frame in redisplay would almost certainly not be
the right thing to do, which probably gets obvious when taking a look
at what that function does.)

Re: question about frame local variable

[Kenichi Handa]

In article <86k76njcj4.fsf@gerd.free-bsd.org>, gerd.moellmann@t-online.de (Gerd Moellmann) writes:
>>  I found that the above is not enough.  It seems that the
>>  display engine doesn't issue select-frame; i.e. provided
>>  that there are two frames A and B, there is a case that the
>>  display engine redisplays A while the selected frame is B.
>>  
>>  It means that even if we make such variables frame local
>>  that affect the redisplaying (font-selection, etc), any
>>  frame local bindings are just ignored.
>>  
>>  Gerd, do you remember this matter?  Is my analysis correct?

> I think so; it matches what I remember.  Local bindings are not
> supported for all redisplay variables.  AFAIR, that has always 
> been the case and isn't new in 21.  

I see, thank you.

> Where it is supported, I think redisplay looks at frame
> parameters.

Yes, I noticed several Fassq (Qxxx, f->param_alist) in
xfaces.c and xdisp.c.  I think we must do the similar thing for
variables that affect font selection.

> (BTW, calling select-frame in redisplay would almost certainly not be
> the right thing to do, which probably gets obvious when taking a look
> at what that function does.)

It seems so.

---
Ken'ichi HANDA
handa@m17n.org

Re: question about frame local variable

[Stefan Monnier]

> (BTW, calling select-frame in redisplay would almost certainly not be
> the right thing to do, which probably gets obvious when taking a look
> at what that function does.)

How about calling a select_frame_internal_for_variables_only ?


        Stefan

Re: question about frame local variable

[Gerd Moellmann]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> > (BTW, calling select-frame in redisplay would almost certainly not be
> > the right thing to do, which probably gets obvious when taking a look
> > at what that function does.)
> 
> How about calling a select_frame_internal_for_variables_only ?

To swap frame-local bindings into C variables, I suppose?  That would
be the alternative to searching in frame parameters, yes.  I guess
it's even better than assq, because it's more general, although it
might do a little bit more work than strictly necessary.

Re: question about frame local variable

[Kenichi Handa]

In article <86oevy1pxp.fsf@gerd.free-bsd.org>, gerd.moellmann@t-online.de (Gerd Moellmann) writes:
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>  > (BTW, calling select-frame in redisplay would almost certainly not be
>>  > the right thing to do, which probably gets obvious when taking a look
>>  > at what that function does.)
>>  
>>  How about calling a select_frame_internal_for_variables_only ?

That seems to be a good idea.  But, as I'm quite unfamiliar
with how frame-local variables are implemented, I don't know
how to write such a funciton.  I would very much appreciate
if someone else implements it.

I found this code in redisplay_window (xdisp.c).

  /* Really select the buffer, for the sake of buffer-local
     variables.  */
  set_buffer_internal_1 (XBUFFER (w->buffer));

Perhaps, we should call
select_frame_internal_for_variables_only around there.

> To swap frame-local bindings into C variables, I suppose?  That would
> be the alternative to searching in frame parameters, yes.  I guess
> it's even better than assq, because it's more general, although it
> might do a little bit more work than strictly necessary.

I think that "a little bit more work" is negligible because
the display engine already does "set-buffer" as above.

---
Ken'ichi HANDA
handa@m17n.org

Re: question about frame local variable

[Gerd Moellmann]

Kenichi Handa <handa@m17n.org> writes:

> I found this code in redisplay_window (xdisp.c).
> 
>   /* Really select the buffer, for the sake of buffer-local
>      variables.  */
>   set_buffer_internal_1 (XBUFFER (w->buffer));
> 
> Perhaps, we should call
> select_frame_internal_for_variables_only around there.

I think the best place is in redisplay_internal, which redisplays
frame by frame.  Otherwise, we'd end up doing this for each window
on a frame again and again.

I can't work on this, but maybe this is helpful:

void
select_frame_for_redisplay (Lisp_Object frame)
{
  Lisp_Object tail, sym, val;
  
  selected_frame = frame;
  
  for (tail = XFRAME (frame)->param_alist; CONSP (tail); tail = XCDR (tail))
    if (CONSP (XCAR (tail))
	&& (sym = XCAR (XCAR (tail)),
	    SYMBOLP (sym))
	&& (sym = indirect_variable (sym),
	    val = SYMBOL_VALUE (sym),
	    (BUFFER_LOCAL_VALUEP (val)
	     || SOME_BUFFER_LOCAL_VALUEP (val)))
	&& XBUFFER_LOCAL_VALUE (val)->check_frame)
      Fsymbol_value (sym);
}

This works analogous to what set_buffer does for buffer-local
variables (hopefully; I haven't tried anything): For all frame
parameters P, check if a frame-local variable P exists.  If so, swap
P's value in by calling Fsymbol_value.  That works because the innards
of Fsymbol_value compare the frame recorded in the local value with
the currently selected frame.

In redisplay_internal, call the function with the frame being
displayed, and in an unwind-protect handler (I think there is already
one), call it to restore the original selected frame and its values.

HTH

Re: question about frame local variable

[Kenichi Handa]

In article <863ccwjysw.fsf@gerd.free-bsd.org>, gerd.moellmann@t-online.de (Gerd Moellmann) writes:
>>  Perhaps, we should call
>>  select_frame_internal_for_variables_only around there.

> I think the best place is in redisplay_internal, which redisplays
> frame by frame.  Otherwise, we'd end up doing this for each window
> on a frame again and again.

> I can't work on this, but maybe this is helpful:

> void
> select_frame_for_redisplay (Lisp_Object frame)
> {
>   Lisp_Object tail, sym, val;
[...]
  
Thank you very much for the help.  I'm now using the
attached patch.  It seems that frame-local variables are
handled correctly.

Richard, what do you think about this change.  Shall I
install it?

---
Ken'ichi HANDA
handa@m17n.org

*** xdisp.c.~1.849.~	Mon Oct 13 11:25:49 2003
--- xdisp.c	Tue Nov 11 17:24:48 2003
***************
*** 814,819 ****
--- 814,820 ----
  static void push_it P_ ((struct it *));
  static void pop_it P_ ((struct it *));
  static void sync_frame_with_window_matrix_rows P_ ((struct window *));
+ static void select_frame_for_redisplay P_ ((Lisp_Object));
  static void redisplay_internal P_ ((int));
  static int echo_area_display P_ ((int));
  static void redisplay_windows P_ ((Lisp_Object));
***************
*** 9542,9547 ****
--- 9543,9586 ----
      }
  }
  \f
+ 
+ /* Select FRAME to forward the values of frame-local variables into C
+    variables so that the redisplay routines can access those values
+    directly.  */
+ 
+ static void
+ select_frame_for_redisplay (frame)
+      Lisp_Object frame;
+ {
+   Lisp_Object tail, sym, val;
+   Lisp_Object old = selected_frame;
+   
+   selected_frame = frame;
+ 
+   for (tail = XFRAME (frame)->param_alist; CONSP (tail); tail = XCDR (tail))
+     if (CONSP (XCAR (tail))
+ 	&& (sym = XCAR (XCAR (tail)),
+ 	    SYMBOLP (sym))
+ 	&& (sym = indirect_variable (sym),
+ 	    val = SYMBOL_VALUE (sym),
+ 	    (BUFFER_LOCAL_VALUEP (val)
+ 	     || SOME_BUFFER_LOCAL_VALUEP (val)))
+ 	&& XBUFFER_LOCAL_VALUE (val)->check_frame)
+       Fsymbol_value (sym);
+ 
+   for (tail = XFRAME (old)->param_alist; CONSP (tail); tail = XCDR (tail))
+     if (CONSP (XCAR (tail))
+ 	&& (sym = XCAR (XCAR (tail)),
+ 	    SYMBOLP (sym))
+ 	&& (sym = indirect_variable (sym),
+ 	    val = SYMBOL_VALUE (sym),
+ 	    (BUFFER_LOCAL_VALUEP (val)
+ 	     || SOME_BUFFER_LOCAL_VALUEP (val)))
+ 	&& XBUFFER_LOCAL_VALUE (val)->check_frame)
+       Fsymbol_value (sym);
+ }
+ 
+ 
  #define STOP_POLLING					\
  do { if (! polling_stopped_here) stop_polling ();	\
         polling_stopped_here = 1; } while (0)
***************
*** 9607,9613 ****
    /* Record a function that resets redisplaying_p to its old value
       when we leave this function.  */
    count = SPECPDL_INDEX ();
!   record_unwind_protect (unwind_redisplay, make_number (redisplaying_p));
    ++redisplaying_p;
    specbind (Qinhibit_free_realized_faces, Qnil);
  
--- 9646,9653 ----
    /* Record a function that resets redisplaying_p to its old value
       when we leave this function.  */
    count = SPECPDL_INDEX ();
!   record_unwind_protect (unwind_redisplay,
! 			 Fcons (make_number (redisplaying_p), selected_frame));
    ++redisplaying_p;
    specbind (Qinhibit_free_realized_faces, Qnil);
  
***************
*** 10021,10026 ****
--- 10061,10071 ----
  
  	  if (FRAME_WINDOW_P (f) || f == sf)
  	    {
+ 	      if (! EQ (frame, selected_frame))
+ 		/* Select the frame, for the sake of frame-local
+ 		   variables.  */
+ 		select_frame_for_redisplay (frame);
+ 
  #ifdef HAVE_WINDOW_SYSTEM
  	      if (clear_face_cache_count % 50 == 0
  		  && FRAME_WINDOW_P (f))
***************
*** 10273,10285 ****
  /* Function registered with record_unwind_protect in
     redisplay_internal.  Reset redisplaying_p to the value it had
     before redisplay_internal was called, and clear
!    prevent_freeing_realized_faces_p.  */
  
  static Lisp_Object
! unwind_redisplay (old_redisplaying_p)
!      Lisp_Object old_redisplaying_p;
  {
    redisplaying_p = XFASTINT (old_redisplaying_p);
    return Qnil;
  }
  
--- 10318,10337 ----
  /* Function registered with record_unwind_protect in
     redisplay_internal.  Reset redisplaying_p to the value it had
     before redisplay_internal was called, and clear
!    prevent_freeing_realized_faces_p.  It also selects the previously
!    selected frame.  */
  
  static Lisp_Object
! unwind_redisplay (val)
!      Lisp_Object val;
  {
+   Lisp_Object old_redisplaying_p, old_frame;
+ 
+   old_redisplaying_p = XCAR (val);
    redisplaying_p = XFASTINT (old_redisplaying_p);
+   old_frame = XCDR (val);
+   if (! EQ (old_frame, selected_frame))
+     select_frame_for_redisplay (old_frame);
    return Qnil;
  }

Re: question about frame local variable

[Richard Stallman]

    Thank you very much for the help.  I'm now using the
    attached patch.  It seems that frame-local variables are
    handled correctly.

    Richard, what do you think about this change.  Shall I
    install it?

It looks good to me.  Please install it.

Re: question about frame local variable

[Kenichi Handa]

In article <E1AK1Bu-0005CG-J0@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:
>     Thank you very much for the help.  I'm now using the
>     attached patch.  It seems that frame-local variables are
>     handled correctly.

>     Richard, what do you think about this change.  Shall I
>     install it?

> It looks good to me.  Please install it.

I'll install it as soon as savannah is recovered.  savannah
doesn't respond now.  Does someone have information about
that?

---
Ken'ichi HANDA
handa@m17n.org

Re: question about frame local variable

[Richard Stallman]

If there is a problem with Savannah, please write to savannah-hackers@gnu.org.

Re: question about frame local variable

[Kenichi Handa]

In article <E1AK8s2-0002fg-Je@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:

> If there is a problem with Savannah, please write to savannah-hackers@gnu.org.

I was actually going to write to them, but before I hit C-c
C-c, it started to work again.  So, the patch for
frame-local variables was already installed.

---
Ken'ichi HANDA
handa@m17n.org