Re: The future of Follow Mode - a proposal.

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Wed, Feb 24, 2016 at 08:34:37PM +0200, Eli Zaretskii wrote:
> > Date: Tue, 23 Feb 2016 23:11:56 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > I don't think it's a boolean.  It should be the buffer position where
> > > the window should be switched.  So probably 2 parameters, for the
> > > beginning and end of the window.  Maybe also the window to switch to.

> > > And then you need to implement the handling of these new arguments.

> > An alternative approach, which would be slightly less disruptive, would
> > be to add in new functions which would work on window groups.  I
> > anticipate needing group versions of `move_it_by_lines', `move_it_to',
> > `move_it_vertically', and possibly `move_it_vertically_backward'.

> You will still face the same problem: when to call these new functions
> and when the old ones.

OK.  I'm not sure it'll be all that big a problem, but we'll see.

> > struct window would be augmented with the following three fields:

> >     /* The next and previous windows in any window group.  */
> >     Lisp_Object group_next;
> >     Lisp_Object group_prev;
> >     /* The window group this window belongs to, or nil.  */
> >     Lisp_Object window_group;

> (Not sure why these are Lisp objects.)

So as to be able to put one or more of them in a saved window
configuration, without garbage collection messing up.  I don't see these
types ever being user visible, so perhaps one of these PSEUDOVECTOR
thingies would do instead.

> >   if (dvpos <= 0)
> >     {
> >       while (!NILP (w->group_prev)
> >              && -dvpos > it->vpos - it->first_vpos)
> >         {
> >           dvpos += it->vpos - it->first_vpos;
> >           move_it_by_lines (it, it->first_vpos - it->vpos);

> This call to move_it_by_lines is a waste of cycles: you know you are
> going to the window start point, so just go there.  The move_it_*
> family of functions cannot go back, only forward, so what actually
> happens here is that the call will go back one line more than its
> argument, and then come back forward by slow iteration.  You don't
> want that.

Er, oh!  :-)

> >           w = XWINDOW (w->group_prev);
> >           /* Set the iterator to just off the bottom of the new window. */
> >           init_iterator (it, w, IT_CHARPOS (*it), IT_BYTEPOS (*it),
> >                          w->desired_matrix->rows + w->total_lines
> >                          - WINDOW_WANTS_MODELINE_P (w),
> >                          it->base_face_id);
> >           it->first_vpos = WINDOW_WANTS_HEADER_LINE_P (w);
> >           it->vpos = w->total_lines - WINDOW_WANTS_MODELINE_P (w);

> You cannot add vpos and w->total_lines: they are measured in different
> units.  The former is the (zero-based) number of a screen line, the
> latter is in units of frame's canonical character height.  Going to
> the end of the window will not in general give vpos the value of
> total_lines.

There seem to be three ways of meausuring vertical distance: by pixel,
by screen line number, and by lines as displayed.  Or something like
that.  Because lines as displayed can be taller than standard lines
(from which w->total_lines is calculated), counting screen lines isn't a
good way of judging the position on a screen.  I really want pixel
positions for that.  move_it_by_lines uses screen line numbers, and
move_it_vertically uses pixel measurements.  So when I'm wanting to find
the top of a left hand windows, having just scrolled the middle window
(a long way), I'll really be wanting to use
move_it_vertically\(_backwards\)?, I think.

> >   else
> >     {
> >       while (!NILP (w->group_next)
> >              && dvpos >= w->total_lines - WINDOW_WANTS_MODELINE_P (w)
> >              - it->vpos)
> >         {
> >           dvpos -= w->total_lines - WINDOW_WANTS_MODELINE_P (w) - it->vpos;
> >           move_it_by_lines (it, w->total_lines - WINDOW_WANTS_MODELINE_P (w)
> >                             - it->vpos);
> >           w = XWINDOW (w->group_next);
> >           /* Set the iterator to the top of the new window. */
> >           init_iterator (it, w, IT_CHARPOS (*it), IT_BYTEPOS (*it),
> >                          w->desired_matrix->rows
> >                          + WINDOW_WANTS_HEADER_LINE_P (w),
> >                          it->base_face_id);

> You want to avoid calling init_iterator here, since this loses the
> bidi context.  E.g., the paragraph direction is lost, and must be
> recomputed, which might be expensive.

There's a standard way of preserving the bidi stuff, is there not?

> You want to have a more light-weight function for recomputing the
> fields of the iterator object that depend on the window metrics.

That's easy enough once I've identified them.  :-)  But I take the
point.

> > > > There are around 150 calls to move_it_*.  I'm guessing that most of these
> > > > would set `physical' to false, perhaps more of the ones in window.c would
> > > > use true.

> > 40 move_it_by_lines + 46 move_it_to + 20 move_it_vertically\(_backward\)? = 106.

> I think you forgot move_it_in_display_line and
> move_it_in_display_line_to.

These two are restricted to the current line, hence can't move out of
the current window.  So their callers would have set the iterator
correctly for the current window.

> > > > > Yes, and the result will be non-trivial changes in the overall logic,
> > > > > because redisplaying a window will no longer be independent of other
> > > > > windows.

> > > > Yes.  This is what is currently implemented in Follow Mode.

> > > No, I mean that redisplay of all the windows in a group will have to
> > > be done in one go, not one window at a time.

> > Indeed.  But often (say, a single character insertion), only one window
> > of a group will need redisplay.

> You won't know that until you actually redisplay that one window and
> then check some conditions.

I anticipate redisplay_window will cope with this, either triggering
redisplay of the other windows in the group immediately, or somehow
marking them for this.

-- 
Alan Mackenzie (Nuremberg, Germany).