From: Keith David Bershatsky <esq@lawlist.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 30226@debbugs.gnu.org
Subject: bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
Date: Mon, 19 Feb 2018 10:52:28 -0800 [thread overview]
Message-ID: <m2zi44errn.wl%esq@lawlist.com> (raw)
In-Reply-To: <m2k1w9kp35.wl%esq@lawlist.com>
[-- Attachment #1: Type: text/plain, Size: 8647 bytes --]
Both the current_x _and_ the hpos do not advance in the simulation immediately prior to generating the tab STRETCH; whereas, the opposite true when it is the real deal with display_line. It may very well be the case that maybe_produce_line_number is not called from move_it_in_display_line_to in some case, whereas it is called from display_line.
I set up a couple of STDERR messages in display_line to observe the values for current_x and hpos immediately prior to calling PRODUCE_GLYPHS -- with one STDERR message before calling maybe_produce_line_number, and another STDERR message immediately after calling maybe_produce_line_number.
I set up a STDERR message in x_produce_glyphs to observe the values for current_x and hpos immediately prior to the calculations for the tab STRETCH.
When we iterate over the line containing "[TAB][TAB]Hello-world" when w->hscroll == 2, moving by increments of it.pixel_width using move_it_in_display_line_to, we observe the following behavior:
* Prior to calling PRODUCE_GLYPHS for the _first_ TAB:
** it.current_x == 7.
** it.hpos == 0.
When we observe the STDERR messages produced from display_line, we observe the following behavior:
* Prior to calling PRODUCE_GLYPHS for the _first_ TAB:
** it.current_x == 35.
** it.hpos == 3.
The difference between the simulation and the real deal is directly related to a call in display_line to maybe_produce_line_number. I have not yet figured out exactly where in the simulation we need to advance it.current_x and it.hpos so that they are in sync with the real deal (display_line). All of the calculations beginning at xdisp.c:28247 that relate to the tab STRETCH are dependent upon it.current_x -- when that value is wrong, everything to the right also gets skewed when iterating over that current line using move_it_in_display_line_to.
int tab_width = it->tab_width * font->space_width;
int x = it->current_x + it->continuation_lines_width;
int x0 = x;
/* Adjust for line numbers, if needed. */
if (!NILP (Vdisplay_line_numbers) && x0 >= it->lnum_pixel_width)
x -= it->lnum_pixel_width;
int next_tab_x = ((1 + x + tab_width - 1) / tab_width) * tab_width;
/* If the distance from the current position to the next tab
stop is less than a space character width, use the
tab stop after that. */
if (next_tab_x - x < font->space_width)
next_tab_x += tab_width;
if (!NILP (Vdisplay_line_numbers) && x0 >= it->lnum_pixel_width)
next_tab_x += (it->lnum_pixel_width
- ((it->w->hscroll * font->space_width)
% tab_width));
it->pixel_width = next_tab_x - x0;
Here is an excerpt of the STDERR printout using the function bug_30226 (when w->hscroll == 2) on the line containing "[TAB][TAB]Hello-world" with the following Elisp and the attached patch.diff:
(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)
(global-set-key [f5] (lambda () (interactive) (bug-30226)))
(insert "\n\n\t\tHello-world")
1. NOTHING
it.c (0)
w->hscroll (2)
it.hpos (0)
it.current_x (0)
it.pixel_width (0)
2. TAB CHARACTER
it.c (187)
w->hscroll (2)
it.hpos (0)
it.current_x (0)
it.pixel_width (7)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (7)
it->hpos (0)
it->lnum_pixel_width (28)
3. TAB STRETCH
it.c (9)
w->hscroll (2)
it.hpos (0)
it.current_x (7)
it.pixel_width (49)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (7)
it->hpos (0)
it->lnum_pixel_width (28)
4. TAB CHARACTER
it.c (187)
w->hscroll (2)
it.hpos (5)
it.current_x (84)
it.pixel_width (7)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (91)
it->hpos (6)
it->lnum_pixel_width (28)
5. TAB STRETCH
it.c (9)
w->hscroll (2)
it.hpos (6)
it.current_x (91)
it.pixel_width (35)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (91)
it->hpos (6)
it->lnum_pixel_width (28)
6. TEXT
it.c (72)
w->hscroll (2)
it.hpos (7)
it.current_x (126)
it.pixel_width (7)
* * *
15. TEXT
it.c (108)
w->hscroll (2)
it.hpos (16)
it.current_x (189)
it.pixel_width (7)
16. TEXT
it.c (100)
w->hscroll (2)
it.hpos (17)
it.current_x (196)
it.pixel_width (7)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (7)
it->hpos (0)
it->lnum_pixel_width (28)
display_line -- BEFORE maybe_produce_line_number:
it->current_x (7)
it->hpos (0)
it->c (9)
display_line -- AFTER maybe_produce_line_number:
it->current_x (35)
it->hpos (3)
it->c (9)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (35)
it->hpos (3)
it->lnum_pixel_width (28)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (77)
it->hpos (5)
it->lnum_pixel_width (28)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (7)
it->hpos (0)
it->lnum_pixel_width (28)
x_produce_glyphs:
it->c (9)
hscroll (2)
it->first_visible_x (14)
it->current_x (91)
it->hpos (6)
it->lnum_pixel_width (28)
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
13 486 499 19 111000110000 0 208 153 16 16 12 12
-1 -1 0
-1 -1
-1 -1
Glyph# Type Pos O W Code C Face LR
0 C -1 0 7 0x000020 27 00
1 C -1 0 7 0x000031 1 27 00
2 C -1 0 7 0x000034 4 27 00
3 C -1 0 7 0x000020 27 00
4 S 486 B 35 0x000000 29 00
5 C 487 B 7 0x0000bb . 30 00
6 S 487 B -1 0x000000 29 00
7 C 488 B 7 0x000048 H 0 00
8 C 489 B 7 0x000065 e 0 00
9 C 490 B 7 0x00006c l 0 00
10 C 491 B 7 0x00006c l 0 00
11 C 492 B 7 0x00006f o 0 00
12 C 493 B 7 0x00002d - 0 00
13 C 494 B 7 0x000077 w 0 00
14 C 495 B 7 0x00006f o 0 00
15 C 496 B 7 0x000072 r 0 00
16 C 497 B 7 0x00006c l 0 00
17 C 498 B 7 0x000064 d 0 00
18 C 0 0 7 0x000020 0 00
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
DATE: [02-19-2018 08:28:38] <19 Feb 2018 18:28:38 +0200>
FROM: Eli Zaretskii <eliz@gnu.org>
>
> > Date: Sun, 18 Feb 2018 18:17:26 -0800
> > From: Keith David Bershatsky <esq@lawlist.com>
> > Cc: 30226@debbugs.gnu.org
> >
> > A major break-through in the tracing of bug 30226 .... it->current_x differs between the real thing (when display_line calls PRODUCE_GLYPHS), versus the simulation (when move_it_in_display_line_to calls PRODUCE_GLYPHS), _because_ maybe_produce_line_number advances it->current_x in the former but _not_ the latter.
>
> How do you see that maybe_produce_line_number doesn't advance
> current_x? AFAICS, it does that unconditionally:
>
> for ( ; g < e; g++)
> {
> it->current_x += g->pixel_width; <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> /* The following is important when this function is called
> from move_it_in_display_line_to: HPOS is incremented only
> when we are in the visible portion of the glyph row. */
> if (it->current_x > it->first_visible_x)
> it->hpos++;
> if (p)
> {
> *p++ = *g;
> (*u)++;
> }
> }
>
> Did you mean hpos instead? Or did you mean that
> maybe_produce_line_number is not called from
> move_it_in_display_line_to in some case, whereas it is called from
> display_line? Or something else?
>
> > SOLUTION (suggested): Revise move_it_in_display_line_to advance X by simulating a call to maybe_produce_line_number and permitting that function to advance X.
>
> But this is already done...
[-- Attachment #2: patch.diff --]
[-- Type: application/diff, Size: 6705 bytes --]
prev parent reply other threads:[~2018-02-19 18:52 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-23 7:32 bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers Keith David Bershatsky
2018-01-23 7:52 ` Keith David Bershatsky
2018-01-27 14:56 ` Eli Zaretskii
2018-01-27 21:20 ` Keith David Bershatsky
2018-01-28 18:15 ` Eli Zaretskii
2018-01-28 19:52 ` Keith David Bershatsky
2018-01-29 16:14 ` Eli Zaretskii
2018-01-31 8:03 ` Keith David Bershatsky
2018-02-04 19:21 ` Keith David Bershatsky
2018-02-19 2:17 ` Keith David Bershatsky
2018-02-19 16:28 ` Eli Zaretskii
2018-02-19 18:52 ` Keith David Bershatsky [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m2zi44errn.wl%esq@lawlist.com \
--to=esq@lawlist.com \
--cc=30226@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.