GNU bug report logs - #31716
move_it_in_display_line / horizontal scrolling / tab stretch

Previous Next

Package: emacs;

Reported by: Keith David Bershatsky <esq <at> lawlist.com>

Date: Tue, 5 Jun 2018 00:39:02 UTC

Severity: normal

Tags: notabug

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31716 in the body.
You can then email your comments to 31716 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#31716; Package emacs. (Tue, 05 Jun 2018 00:39:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Keith David Bershatsky <esq <at> lawlist.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 05 Jun 2018 00:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Emacs Bug Reports <bug-gnu-emacs <at> gnu.org>
Subject: move_it_in_display_line / horizontal scrolling / tab stretch
Date: Mon, 04 Jun 2018 17:38:17 -0700
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;
}




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31716; Package emacs. (Tue, 05 Jun 2018 14:57:01 GMT) Full text and rfc822 format available.

Message #8 received at 31716 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 31716 <at> debbugs.gnu.org
Subject: Re: bug#31716: move_it_in_display_line / horizontal scrolling / tab
 stretch
Date: Tue, 05 Jun 2018 17:56:32 +0300
> Date: Mon, 04 Jun 2018 17:38:17 -0700
> From: Keith David Bershatsky <esq <at> 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.




Added tag(s) notabug. Request was from Eli Zaretskii <eliz <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 05 Jun 2018 14:58:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31716; Package emacs. (Wed, 06 Jun 2018 04:19:01 GMT) Full text and rfc822 format available.

Message #13 received at 31716 <at> debbugs.gnu.org (full text, mbox):

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 31716 <at> debbugs.gnu.org
Subject: Re: bug#31716: move_it_in_display_line / horizontal scrolling /
 tab	stretch
Date: Tue, 05 Jun 2018 21:18:27 -0700
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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31716; Package emacs. (Fri, 08 Jun 2018 05:13:02 GMT) Full text and rfc822 format available.

Message #16 received at 31716 <at> debbugs.gnu.org (full text, mbox):

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 31716 <at> debbugs.gnu.org
Subject: Re: bug#31716: move_it_in_display_line / horizontal scrolling /
 tab	stretch
Date: Thu, 07 Jun 2018 22:12:34 -0700
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 pgrowx 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




Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Fri, 08 Jun 2018 08:21:02 GMT) Full text and rfc822 format available.

Notification sent to Keith David Bershatsky <esq <at> lawlist.com>:
bug acknowledged by developer. (Fri, 08 Jun 2018 08:21:02 GMT) Full text and rfc822 format available.

Message #21 received at 31716-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 31716-done <at> debbugs.gnu.org
Subject: Re: bug#31716: move_it_in_display_line / horizontal scrolling /
 tab	stretch
Date: Fri, 08 Jun 2018 11:19:57 +0300
> Date:  Thu, 07 Jun 2018 22:12:34 -0700
> From:  Keith David Bershatsky <esq <at> lawlist.com>
> Cc:  31716 <at> 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.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 06 Jul 2018 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 353 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.