bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.

[Eli Zaretskii]

> Date:  Sat, 27 Jan 2018 13:20:55 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  30226@debbugs.gnu.org
> 
> dump-glyph-row is _correct_, assuming that a redisplay has occurred prior to 
> calling that function.

Glyph matrices are produced by redisplay, so of course one needs
redisplay to happen before the glyph-rows can be meaningfully dumped.

> Feature request 17684 (i.e., crosshairs) uses move_it_in_display_line_to -- 
> by increments of it.pixel_width -- to dump various values for each character 
> in the current line.  I reproduce issue #30226 by placing IT at the beginning 
> of the current line (i.e., it.current_x == 0) and iterating over each 
> character in that line:
> 
>   move_it_in_display_line_to (&it, ZV, it.current_x + it.pixel_width,
>                               MOVE_TO_POS | MOVE_TO_X);
> 
> Your outline of the steps needed to reproduce this issue requires a few 
> additional steps:
> 
> Step 0.5) Build a new Emacs 26 with the reduced/simplified function named 
> bug-30226 (written below); and, add `defsubr (&Sbug_30226);` to syms_of_xdisp.
> 
> Step 1) No changes.
> 
> Step 2) No changes.
> 
> Step 3) No changes.
> 
> Step 4) Instead of typing "M-: (scroll-left 1) RET", type "M-: (bug-30226) 
> RET".  w->hscroll should now equal 1.  In step 4, there is a _hiccup_ with 
> the initial value of it->pixel_width being wrong as to the tab STRETCH, and 
> the subsequent value of it->pixel_width is correct as to the tab STRETCH.
> 
> 1.  NOTHING
>     it.c (0)
>     w->hscroll (1)
>     it.current_x (0)
>     it.pixel_width (0)
> 
> 2.  TAB CHARACTER
>     it.c (187)
>     w->hscroll (1)
>     it.current_x (0)
>     it.pixel_width (7)
> 
> 3.  TAB STRETCH
>     it.c (9)
>     w->hscroll (1)
>     it.current_x (7)
>     it.pixel_width (49)
> 
> 4.  TAB STRETCH
>     it.c (9)
>     w->hscroll (1)
>     it.current_x (35)
>     it.pixel_width (42)

I don't see any such "hiccups" on my system.  But that's an aside.

> Step 5) Type "M-: (bug-30226) RET".  w->hscroll should now equal 2.  In step 
> 5, the value for it->pixel_width is wrong as to the tab STRETCH and 
> it->current_x is wrong as to the characters that follow the tab STRETCH -- 
> probably because it->pixel_width of the tab STRETCH was wrong.

Well, I looked into the relevant code, and I don't find any bugs
there, only subtle (mis)features.

The main problems in your design are twofold:

  . move_it_in_display_line_to is "tricky" when called with MOVE_TO_X
    flag in its last argument.  It is tricky because when it returns
    with it->current_x set to some X, it actually already processed
    the glyph following that coordinate.  In your case, that glyph is
    the stretch glyph used to display the TAB.  In effect,
    move_it_in_display_line_to processes the stretch glyph, and then
    backs up, but it leaves the value of it->pixel_width as it was
    computed during that (abandoned) processing.  The value of
    pixel_width is indeed not adjusted in this case, but since it
    "belongs" to the "next" glyph, it shouldn't be expected to be
    accurate.
  . the value of it->pixel_width was never intended to be used by code
    which calls move_it_* functions, it is a temporary store used by
    the display engine for its internal purposes.  The above "tricky"
    aspect of move_it_in_display_line_to is just one manifestation of
    that general fact.  In a nutshell, it->pixel_width is not
    guaranteed to describe accurately the glyph at the location where
    these functions return: it can be a glyph before or after that
    place, and as you see in your example, it can be inaccurately
    calculated.

Bottom line: to get accurate values of pixel width, you need to
subtract it->current_x value at some position from current_x at the
next glyph position.  This is the only reliable way to obtain accurate
pixel width values when using the move_it_* functions.

As an aside, the value of it->pixel_width you see in the scenario
described in this thread cannot even be accused of inaccuracy: it
describes accurately the pixel width of the stretch glyph, except that
part of that glyph is not visible because it precedes
it->first_visible_x, and so is outside of the window "viewport".  This
happens even when line numbers are not displayed (your debug code
prints the same "incorrect" width values for me in that case, contrary
to what you told originally).  When line numbers is displayed, they
muddy the waters even more, because the part of the stretch glyph that
is left invisible is followed by the glyphs produced to show the line
number, and "the rest" of the stretch glyph follows that.  Which is
probably why you expected the width of the stretch glyph, as reflected
in it->pixel_width, to change with the hscroll, but in fact it doesn't
change at all, the hscroll just makes part of the stretch invisible.