bug#31716: move_it_in_display_line / horizontal scrolling / tab stretch

bug#31716: move_it_in_display_line / horizontal scrolling / tab stretch

[Keith David Bershatsky]

Step 1.  Build a new Emacs (master branch) with the function bug-christmas-ghost (below).  [E.g., place the new function somewhere appropriate inside of xdisp.c.]

Step 2.  Launch the newly built Emacs from the terminal so that we can see the STDERR output.

Step 3.  Paste the following Lisp code into a scratch buffer and evaluate the code.

    (setq display-line-numbers t)
    (setq buffer-display-table (make-display-table))
    (aset buffer-display-table
          ?\t
          (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
                  (make-glyph-code ?\t 'highlight)))
    (setq tab-width 8)

Step 4.  On a new line in the scratch buffer type:  C-q TAB Hello

Step 5.  Place the cursor somewhere on the word "Hello".

Step 6.  Evaluate:  (scroll-left 2)

Step 7.  Evaluate:  (bug-christmas-ghost)


OBSERVATIONS:

A.  When w->hscroll is >= 2 and IT is within the tab STRETCH, move_it_in_display_line_to reports an erroneous it.hpos subsequent to the line numbers.  In this example, it.hpos is reported as being either 0 or 5, even though the tab STRETCH visibly begins at an it.hpos of 4 when line numbers are 2 digits wide.

B.  When w->hscroll is >= 2 and IT is within the tab STRETCH, the latter portion contains one or more it.c ghosts of Christmas future.  I.e., the letter "H" gets returned at several locations along the tab STRETCH.


EXPECTATIONS / DESIRED BEHAVIOR.

i.  Loops 7 to 12 should report an it.hpos of 4.

ii.  Loops 7 to 12 should report an it.c of 9 (aka a tab character).


Here is an excerpt from the STDERR output after calling (scroll-left 2) and moving IT by X -- incrementing each loop by font->space_width.

[ * * * BEGIN excerpt of STDERR output]

[IT.HPOS IS 4, NOT 0.]
7.  TAB STRETCH
    it.c (9)
    char (	)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (0)
    it.first_visible_x (14)
    it.current_x (7)
    relative_x (28)
    new_x (42)
    font->space_width (7)
    it.pixel_width (49)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[IT.HPOS IS 4, NOT 0.]
8.  TAB STRETCH
    it.c (9)
    char (	)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (0)
    it.first_visible_x (14)
    it.current_x (7)
    relative_x (35)
    new_x (49)
    font->space_width (7)
    it.pixel_width (49)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
9.  TEXT
    it.c (72)
    char (H)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (5)
    it.first_visible_x (14)
    it.current_x (84)
    relative_x (42)
    new_x (56)
    font->space_width (7)
    it.pixel_width (7)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
10.  TEXT
    it.c (72)
    char (H)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (5)
    it.first_visible_x (14)
    it.current_x (84)
    relative_x (49)
    new_x (63)
    font->space_width (7)
    it.pixel_width (7)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
11.  TEXT
    it.c (72)
    char (H)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (5)
    it.first_visible_x (14)
    it.current_x (84)
    relative_x (56)
    new_x (70)
    font->space_width (7)
    it.pixel_width (7)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
12.  TEXT
    it.c (72)
    char (H)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (5)
    it.first_visible_x (14)
    it.current_x (84)
    relative_x (63)
    new_x (77)
    font->space_width (7)
    it.pixel_width (7)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[THIS IS THE REAL LETTER "H"]
13.  TEXT
    it.c (72)
    char (H)
    it.first_visible_x (14)
    w->hscroll (2)
    it.hpos (5)
    it.first_visible_x (14)
    it.current_x (84)
    relative_x (70)
    new_x (84)
    font->space_width (7)
    it.pixel_width (7)
    it.lnum_pixel_width (28)
    rc (MOVE_X_REACHED)

[ * * * END excerpt of STDERR output]


DEFUN ("bug-christmas-ghost", Fbug_christmas_ghost, Sbug_christmas_ghost, 0, 0, 0,
       doc: /* Debug the Ghost of Christmas Future. */)
  (void)
{
/* ******************************************************************************
                      PRELIMINARY STUFF
****************************************************************************** */
  struct window *w = decode_live_window (selected_window);
  struct frame *f = XFRAME (w->frame);
  struct it it;
  void *itdata = bidi_shelve_cache ();
  enum move_it_result rc = MOVE_X_REACHED;
  struct text_pos start_text_position;
  int count = 0;
  int new_x = 0;
  int relative_x = 0;
/* ******************************************************************************
                      START DISPLAY -- w->start
****************************************************************************** */
  /* Begin the journey at w->start. */
  SET_TEXT_POS_FROM_MARKER (start_text_position, w->start);
  start_display (&it, w, start_text_position);
  struct face *face = FACE_FROM_ID (it.f, it.face_id);
  struct font *font = face->font;
/* ******************************************************************************
                GO TO THE BEGINNING OF THE CURRENT LINE.
****************************************************************************** */
  /* Place the IT on the current line containing PT. */
  int voffset = (WINDOW_HEADER_LINE_HEIGHT (w) > 0
                 && w->output_cursor.vpos > 0)
                  ? w->output_cursor.vpos - 1
                  : w->output_cursor.vpos;
  if (voffset > 0)
    move_it_by_lines (&it, voffset);
  struct glyph_row *glyph_row = MATRIX_ROW (w->current_matrix, it.vpos);
/* ******************************************************************************
             MOVE IT OVER EACH CHARACTER ON THE CURRENT LINE.
****************************************************************************** */
  while (true)
    {
      count += 1;
      if (new_x > 0)
        relative_x = new_x - it.first_visible_x;
      if (ITERATOR_AT_END_OF_LINE_P (&it)
          || FETCH_BYTE (IT_BYTEPOS (it)) == '\n'
          || rc == MOVE_POS_MATCH_OR_ZV)
        break;
/* ******************************************************************************
                       DUMP RELEVANT GLYPH INFORMATION
****************************************************************************** */
      if (w->hscroll > 0)
        {
          int w_hscroll = w->hscroll;
      fprintf (stderr, "\n%d.  %s\n\
    it.c (%d)\n\
    char (%s)\n\
    it.first_visible_x (%d)\n\
    w->hscroll (%d)\n\
    it.hpos (%d)\n\
    it.first_visible_x (%d)\n\
    it.current_x (%d)\n\
    relative_x (%d)\n\
    new_x (%d)\n\
    font->space_width (%d)\n\
    it.pixel_width (%d)\n\
    it.lnum_pixel_width (%d)\n\
    rc (%s)\n",
            count,
            (it.c == 0
              ? "NOTHING"
              : it.c == 187
                ? "TAB CHARACTER"
              : it.c == '\t'
                ? "TAB STRETCH"
              : "TEXT"),
            it.c,
            SSDATA (Fchar_to_string (make_number (it.c))),
            it.first_visible_x,
            w_hscroll,
            it.hpos,
            it.first_visible_x,
            it.current_x,
            relative_x,
            new_x,
            font->space_width,
            it.pixel_width,
            it.lnum_pixel_width,
            (rc == MOVE_UNDEFINED
              ? "MOVE_UNDEFINED"
              : rc == MOVE_POS_MATCH_OR_ZV
                ? "MOVE_POS_MATCH_OR_ZV"
              : rc == MOVE_X_REACHED
                ? "MOVE_X_REACHED"
              : rc == MOVE_LINE_CONTINUED
                ? "MOVE_LINE_CONTINUED"
              : rc == MOVE_LINE_TRUNCATED
                ? "MOVE_LINE_TRUNCATED"
              : rc == MOVE_NEWLINE_OR_CR
                ? "MOVE_NEWLINE_OR_CR"
              : "OOPS"));
        }
/* ******************************************************************************
                       MOVE IT -- INCREMENT == FONT->SPACE_WIDTH
****************************************************************************** */
      new_x += font->space_width;
      rc = move_it_in_display_line_to (&it, ZV, new_x, MOVE_TO_POS | MOVE_TO_X);
      if (rc == MOVE_LINE_CONTINUED)
        break;
      if (it.current_x - it.first_visible_x + font->space_width >=
          window_box_width (w, TEXT_AREA))
        break;
    }
/* ******************************************************************************
                         REDISPLAY AND DUMP_GLPYH_ROW
****************************************************************************** */
  redisplay_internal ();
  fprintf (stderr, "\n");
  dump_glyph_row (glyph_row, it.vpos, 2);
  bidi_unshelve_cache (itdata, false);
  return Qnil;
}

bug#31716: move_it_in_display_line / horizontal scrolling / tab stretch

[Eli Zaretskii]

> Date: Mon, 04 Jun 2018 17:38:17 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> Step 3.  Paste the following Lisp code into a scratch buffer and evaluate the 
> code.
> 
>     (setq display-line-numbers t)
>     (setq buffer-display-table (make-display-table))
>     (aset buffer-display-table
>           ?\t
>           (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
>                   (make-glyph-code ?\t 'highlight)))
>     (setq tab-width 8)
> 
> Step 4.  On a new line in the scratch buffer type:  C-q TAB Hello
> 
> Step 5.  Place the cursor somewhere on the word "Hello".
> 
> Step 6.  Evaluate:  (scroll-left 2)
> 
> Step 7.  Evaluate:  (bug-christmas-ghost)
> 
> OBSERVATIONS:
> 
> A.  When w->hscroll is >= 2 and IT is within the tab STRETCH, 
> move_it_in_display_line_to reports an erroneous it.hpos subsequent to the line 
> numbers.  In this example, it.hpos is reported as being either 0 or 5, even 
> though the tab STRETCH visibly begins at an it.hpos of 4 when line numbers are 
> 2 digits wide.
> 
> B.  When w->hscroll is >= 2 and IT is within the tab STRETCH, the latter 
> portion contains one or more it.c ghosts of Christmas future.  I.e., the letter 
> "H" gets returned at several locations along the tab STRETCH.
> 
> EXPECTATIONS / DESIRED BEHAVIOR.
> 
> i.  Loops 7 to 12 should report an it.hpos of 4.
> 
> ii.  Loops 7 to 12 should report an it.c of 9 (aka a tab character).

Summary: Your expectations are incorrect, and the code works as
expected, AFAICT.

Details:

There seems to be a misunderstanding of what MOVE_TO_X means for
move_it_in_display_line_to.  It sounds like you expect it to end up
and the new_x coordinate, or report data about what's at new_x on
display.  But that's not what MOVE_TO_X actually does.  In reality,
move_it_in_display_line_to cannot move with 1-pixel granularity, it
can only move one "display element" at a time.  MOVE_TO_X then tells
it to stop at a display element whose display _includes_ the
coordinate new_x.  A display element that begins at X and has pixel
width of WX includes the coordinates in [X..X+WX), i.e. X+WX itself is
NOT included.  So if, for some display element that starts at X, new_x
satisfies the inequality

        X <= new_x < X+WX

then move_it_in_display_line_to will stop at X and return
MOVE_X_REACHED.

On a GUI frame, a TAB produces a single display element: a stretch
glyph whose width is the required TAB width.  In this case, it's 56
pixels, 14 of which are invisible due to hscroll.

With that knowledge in hand, you need to re-asses the results you
obtain.  Most importantly, it.current_x tells you where did
move_it_in_display_line_to actually stop; it doesn't matter where you
asked it to stop (which is new_x).

> [IT.HPOS IS 4, NOT 0.]
> 7.  TAB STRETCH
>     it.c (9)
>     char (      )
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (0)
>     it.first_visible_x (14)
>     it.current_x (7)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>     relative_x (28)
>     new_x (42)
>     font->space_width (7)
>     it.pixel_width (49)
>     it.lnum_pixel_width (28)
>     rc (MOVE_X_REACHED)

> [IT.HPOS IS 4, NOT 0.]
> 8.  TAB STRETCH
>     it.c (9)
>     char (      )
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (0)
>     it.first_visible_x (14)
>     it.current_x (7)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>     relative_x (35)
>     new_x (49)
>     font->space_width (7)
>     it.pixel_width (49)
>     it.lnum_pixel_width (28)
>     rc (MOVE_X_REACHED)

> [IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
> 9.  TEXT
>     it.c (72)
>     char (H)
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (5)
>     it.first_visible_x (14)
>     it.current_x (84)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>     relative_x (42)
>     new_x (56)

As you see, the first two calls ended up at the same X coordinate of
7, while the 3rd call stopped at X = 84, and first_visible_x is 14.
Since 7 < 14, the first two calls stopped on the TAB, and HPOS is zero
because the TAB is hscrolled (HPOS is only advanced for X coordinates
beyond first_visible_x).  The 3rd call stopped at X = 84, which is the
coordinate of 'H', because 84 = 56 + 28, the width needed for 56
pixels of the TAB and 28 pixels of the line number.  So the 3rd call
reports on 'H', and its HPOS is 5, because 4 columns are taken by the
line-number digits and one more is taken by the (partially visible)
stretch glyph that displays the TAB.

Then you see 4 more calls, all of which stop at X = 84, so they all
report on 'H':

> [IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
> 10.  TEXT
>     it.c (72)
>     char (H)
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (5)
>     it.first_visible_x (14)
>     it.current_x (84)
>     relative_x (49)
>     new_x (63)
>     font->space_width (7)
>     it.pixel_width (7)
>     it.lnum_pixel_width (28)
>     rc (MOVE_X_REACHED)

> [IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
> 11.  TEXT
>     it.c (72)
>     char (H)
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (5)
>     it.first_visible_x (14)
>     it.current_x (84)
>     relative_x (56)
>     new_x (70)
>     font->space_width (7)
>     it.pixel_width (7)
>     it.lnum_pixel_width (28)
>     rc (MOVE_X_REACHED)

> [IT.C IS 9, NOT 72; AND, IT.HPOS IS 4, NOT 5.]
> 12.  TEXT
>     it.c (72)
>     char (H)
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (5)
>     it.first_visible_x (14)
>     it.current_x (84)
>     relative_x (63)
>     new_x (77)
>     font->space_width (7)
>     it.pixel_width (7)
>     it.lnum_pixel_width (28)
>     rc (MOVE_X_REACHED)

> [THIS IS THE REAL LETTER "H"]
> 13.  TEXT
>     it.c (72)
>     char (H)
>     it.first_visible_x (14)
>     w->hscroll (2)
>     it.hpos (5)
>     it.first_visible_x (14)
>     it.current_x (84)
>     relative_x (70)
>     new_x (84)
>     font->space_width (7)
>     it.pixel_width (7)
>     it.lnum_pixel_width (28)
>     rc (MOVE_X_REACHED)

IOW, these 'H' are not "ghosts", they are the actual character
displayed at the same location X = 84.  You just get 4 reports on the
same character because move_it_in_display_line_to is at the same
coordinate.  So I see nothing wrong here.

Note that on TTY frames, a TAB is displayed as several separate SPC
characters, so in that case you _can_ move by individual increments
inside the TAB display.

bug#31716: move_it_in_display_line / horizontal scrolling / tab stretch

[Keith David Bershatsky]

Thank you very much, Eli, for taking the time to provide a detailed explanation regarding how MOVE_TO_X works.  I have come up with a way to programmatically deal with this particular situation when all lines are horizontally scrolled, but am still struggling with this situation when only the current line is horizontally scrolled.

When only the current line is horizontally scrolled in this particular situation, HPOS is not:

it.hpos - window_hscroll_limited (w, f);

Instead, HPOS is something like it.hpos - 1 for the fist tab and it becomes more complicated when horizontally scrolling multiple tabs.

If it okay, I would suggest/request that bug#31716 remain open for a little bit longer so that I can assemble a minimal working example dealing with horizontal scrolling of the current line when tabs exist.

Keith

bug#31716: move_it_in_display_line / horizontal scrolling / tab stretch

[Keith David Bershatsky]

I have had an opportunity to further study your write-up regarding issue 31716, and I really appreciate the help you have given.  Using the theory that the increase in current_x translates into a new display element and a corresponding increase in the HPOS once the first visible x is reached, I have come up with (what appears to be) a working method to reliably calculate RELATIVE_X and HPOS when tabs are present with horizontal scrolling.  However, I am essentially incrementing font->space_width from the far left of the display elements to the end of the line in order to make all of the relevant determinations along the current line (where I am drawing multiple fake cursors to achieve the visible horizontal line for crosshairs).  In a nutshell, the method I am using is somewhat akin to pgr
 owx in .gdbinit (in that RELATIVE_X is determined based upon the preceding element/determination).

The method I am using to calculate RELATIVE_X and HPOS (when tabs are present with horizontal scrolling) is undoubtedly less efficient than using nifty general methods such as:

A.  RELATIVE_X = it.current_x - it.first_visible_x.

B.  RELATIVE_X = it.current_x - (window_hscroll_limited (w, f) * frame_char_width).

C.  HPOS = it.hpos.

D.  HPOS = t.hpos - window_hscroll_limited (w, f).

More efficient ways to calculate RELATIVE_X and HPOS (when tabs are present with horizontal scrolling) can be dealt with in the future on the Emacs devel mailing list since they are not directly related to the reason that #31716 was launched.  Please feel free to close out #31716 at your convenience.

Thank you again for all your help.

Keith

bug#31716: move_it_in_display_line / horizontal scrolling / tab stretch

[Eli Zaretskii]

> Date:  Thu, 07 Jun 2018 22:12:34 -0700
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  31716@debbugs.gnu.org
> 
> More efficient ways to calculate RELATIVE_X and HPOS (when tabs are present with horizontal scrolling) can be dealt with in the future on the Emacs devel mailing list since they are not directly related to the reason that #31716 was launched.  Please feel free to close out #31716 at your convenience.

Done.