* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
@ 2018-01-23 7:32 Keith David Bershatsky
2018-01-23 7:52 ` Keith David Bershatsky
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Keith David Bershatsky @ 2018-01-23 7:32 UTC (permalink / raw)
To: 30226
[-- Attachment #1: Type: text/plain, Size: 2119 bytes --]
The following snippet contains the ingredients that can be used to ultimately fix the problem described on the emacs-devel mailing list beginning at:
https://lists.gnu.org/archive/html/emacs-devel/2018-01/msg00466.html
In a nutshell, it->pixel_width and it->current_x are both incorrect in that situation. Because the X is wrong, all subsequent references to it->current_x on the same line are also wrong. In this snippet, we create a new gizmo in dispextern.h called my_pixel_width so as not to break anything while working on this issue. Someone more knowledgeable than myself (e.g., Eli) will need to figure out what other adjustments in x_produce_glyphs are necessary so that it->pixel_width == it->my_pixel_width in this particular situation. It is a little confusing, but here is what happens in this snippet:
it->pixel_width "should be" equal to it->my_pixel_width. If we do that, however, then it->current_x will be wrong.
it->current_x "should be" equal to it->current_x less one (1) font->space_width. Setting it->pixel_width to be one (1) font->space_width less than what it was fixes the value of it->current_x and does not break anything else in the process (as far as I can see).
dispextern.h:2590
int my_pixel_width;
xdisp.c:28298
if (it->char_to_display == '\t'
&& !NILP (Vdisplay_line_numbers)
&& it->w->hscroll > 0
&& it->current_x < it->lnum_pixel_width)
{
int my_tab_width = it->tab_width * font->space_width;
int my_x = it->current_x + it->continuation_lines_width;
int my_next_tab_x = ((1 + my_x + my_tab_width - 1) / my_tab_width)
* my_tab_width;
if (my_next_tab_x - my_x < font->space_width)
my_next_tab_x += my_tab_width;
if (!NILP (Vdisplay_line_numbers))
my_next_tab_x += it->lnum_pixel_width
- ((it->w->hscroll * font->space_width)
% my_tab_width);
it->my_pixel_width = my_next_tab_x - it->lnum_pixel_width - font->space_width;
it->pixel_width -= font->space_width;
}
else
it->my_pixel_width = 0;
[-- Attachment #2: patch.diff --]
[-- Type: application/diff, Size: 9234 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
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
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Keith David Bershatsky @ 2018-01-23 7:52 UTC (permalink / raw)
To: 30226
[-- Attachment #1: Type: text/plain, Size: 82 bytes --]
Here is a revised patch that removes an erroneous section that was experimental.
[-- Attachment #2: patch.diff --]
[-- Type: application/diff, Size: 8729 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-23 7:52 ` Keith David Bershatsky
@ 2018-01-27 14:56 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2018-01-27 14:56 UTC (permalink / raw)
To: Keith David Bershatsky; +Cc: 30226
> Date: Mon, 22 Jan 2018 23:52:20 -0800
> From: Keith David Bershatsky <esq@lawlist.com>
>
> Here is a revised patch that removes an erroneous section that was experimental.
Thanks, but I'm unable to reproduce the original problem, so I cannot
yet discuss the reasons, let alone the solution.
Here's what I did in Emacs built from the current emacs-26 branch:
1) Inserted the following snippet into *scratch*:
(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)
2) Evaluated it ("M-x eval-region RET").
3) Went to the end of the buffer, inserted a few empty lines, then
went up 2 lines from EOB, and typed "C-q TAB Hello-world"
4) Typed "M-x dump-glyph-row RET"
The result is this data shown on stderr:
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
11 409 422 18 010000100000 0 176 192 16 16 12 12
-1 -1 0
-1 -1
-1 -1
Glyph# Type Pos O W Code C Face LR
0 C -1 0 8 0x000020 20 00
1 C -1 0 8 0x000031 1 20 00
2 C -1 0 8 0x000032 2 20 00
3 C -1 0 8 0x000020 20 00
4 C 409 B 8 0x0000bb . 23 00
5 S 409 B 56 0x000000 22 00 <<<<<<<<<<<<<<<<<<
6 C 410 B 8 0x000048 H 0 00
7 C 411 B 8 0x000065 e 0 00
8 C 412 B 8 0x00006c l 0 00
9 C 413 B 8 0x00006c l 0 00
10 C 414 B 8 0x00006f o 0 00
11 C 415 B 8 0x00002d - 0 00
12 C 416 B 8 0x000077 w 0 00
13 C 417 B 8 0x00006f o 0 00
14 C 418 B 8 0x000072 r 0 00
15 C 419 B 8 0x00006c l 0 00
16 C 420 B 8 0x000064 d 0 00
17 C 0 0 8 0x000020 0 00
This is as expected: the width of the default font is 8, and the size
of the stretch glyph #5 is 8x8 = 56, as expected.
Next, I typed "M-: (scroll-left 1) RET" and again invoked
dump-glyph-row. The result:
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
11 409 422 17 011000100000 0 176 176 16 16 12 12
-1 -1 0
-1 -1
-1 -1
Glyph# Type Pos O W Code C Face LR
0 C -1 0 8 0x000020 20 00
1 C -1 0 8 0x000031 1 20 00
2 C -1 0 8 0x000032 2 20 00
3 C -1 0 8 0x000020 20 00
4 S 409 B 48 0x000000 22 00 <<<<<<<<<<<<<<
5 C 410 B 8 0x000048 H 0 00
6 C 411 B 8 0x000065 e 0 00
7 C 412 B 8 0x00006c l 0 00
8 C 413 B 8 0x00006c l 0 00
9 C 414 B 8 0x00006f o 0 00
10 C 415 B 8 0x00002d - 0 00
11 C 416 B 8 0x000077 w 0 00
12 C 417 B 8 0x00006f o 0 00
13 C 418 B 8 0x000072 r 0 00
14 C 419 B 8 0x00006c l 0 00
15 C 420 B 8 0x000064 d 0 00
16 C 0 0 8 0x000020 0 00
Note that the glyph corresponding to the u+00BB character now
disappeared from display, and therefore the stretch glyph is #4, its
width is 48 -- again, as expected.
I then typed "M-: (scroll-left 1) RET" once more, followed by
dump-glyph-row, and the results are:
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
11 409 422 17 011000100000 0 176 168 16 16 12 12
-1 -1 0
-1 -1
-1 -1
Glyph# Type Pos O W Code C Face LR
0 C -1 0 8 0x000020 20 00
1 C -1 0 8 0x000031 1 20 00
2 C -1 0 8 0x000032 2 20 00
3 C -1 0 8 0x000020 20 00
4 S 409 B 40 0x000000 22 00 <<<<<<<<<<<<<
5 C 410 B 8 0x000048 H 0 00
6 C 411 B 8 0x000065 e 0 00
7 C 412 B 8 0x00006c l 0 00
8 C 413 B 8 0x00006c l 0 00
9 C 414 B 8 0x00006f o 0 00
10 C 415 B 8 0x00002d - 0 00
11 C 416 B 8 0x000077 w 0 00
12 C 417 B 8 0x00006f o 0 00
13 C 418 B 8 0x000072 r 0 00
14 C 419 B 8 0x00006c l 0 00
15 C 420 B 8 0x000064 d 0 00
16 C 0 0 8 0x000020 0 00
Now the stretch glyph's width is 40 pixels, again as expected.
So the width of the stretch glyph is computed correctly. And your
screenshots also show similar results, where the width of the stretch
glyph is decremented for each call to scroll-left.
Your problem description talks about it->pixel_width, not about
glyph->pixel_width, but the latter should be equal to the former,
because x_produce_glyphs does this when it finishes computing the
width of the stretch glyph:
if (it->glyph_row)
{
append_stretch_glyph (it, it->object, it->pixel_width,
it->ascent + it->descent, it->ascent);
and the function append_stretch_glyph then assigns the value of its
3rd argument to glyph->pixel_width:
static void
append_stretch_glyph (struct it *it, Lisp_Object object,
int width, int height, int ascent)
{
...
glyph->pixel_width = clip_to_bounds (-1, width, SHRT_MAX);
So I'm not sure why you are saying there's a bug in the display code
regarding it->pixel_width of the stretch glyphs that represent TAB
characters.
You say:
> In a nutshell, it->pixel_width and it->current_x are both incorrect in that situation. Because the X is wrong, all subsequent references to it->current_x on the same line are also wrong.
I don't understand why you are saying that it->current_x is wrong.
What is that assertion based on? Maybe there's some misunderstanding
of what current_x is and relative to what position does it measure the
X coordinate. Can you tell what you expected current_x to be in some
specific situation (i.e., specific horizontal scroll of the display
in the above scenario), and what you really found?
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
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 21:20 ` Keith David Bershatsky
2018-01-28 18:15 ` Eli Zaretskii
2018-01-28 19:52 ` Keith David Bershatsky
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Keith David Bershatsky @ 2018-01-27 21:20 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30226
Thank you, Eli, for looking into #30226.
> Your problem description talks about it->pixel_width, not about
> glyph->pixel_width, but the latter should be equal to the former . . .,
>
* * *
>
> I don't understand why you are saying that it->current_x is wrong.
> What is that assertion based on? Maybe there's some misunderstanding
> of what current_x is and relative to what position does it measure the
> X coordinate. Can you tell what you expected current_x to be in some
> specific situation (i.e., specific horizontal scroll of the display
> in the above scenario), and what you really found?
dump-glyph-row is _correct_, assuming that a redisplay has occurred prior to calling that function.
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)
5. TEXT
it.c (72)
w->hscroll (1)
it.current_x (77)
it.pixel_width (7)
* * *
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
12 455 467 17 111000110000 0 192 154 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 29 00
1 C -1 0 7 0x000031 1 29 00
2 C -1 0 7 0x000033 3 29 00
3 C -1 0 7 0x000020 29 00
4 S 455 B 42 0x000000 31 00
5 C 456 B 7 0x000048 H 0 00
6 C 457 B 7 0x000065 e 0 00
7 C 458 B 7 0x00006c l 0 00
8 C 459 B 7 0x00006c l 0 00
9 C 460 B 7 0x00006f o 0 00
10 C 461 B 7 0x00002d - 0 00
11 C 462 B 7 0x000077 w 0 00
12 C 463 B 7 0x00006f o 0 00
13 C 464 B 7 0x000072 r 0 00
14 C 465 B 7 0x00006c l 0 00
15 C 466 B 7 0x000064 d 0 00
16 C 0 0 7 0x000020 0 00
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.
1. NOTHING
it.c (0)
w->hscroll (2)
it.current_x (0)
it.pixel_width (0)
2. TAB CHARACTER
it.c (187)
w->hscroll (2)
it.current_x (0)
it.pixel_width (7)
3. TAB STRETCH
it.c (9)
w->hscroll (2)
it.current_x (7)
it.pixel_width (49)
4. TEXT
it.c (72)
w->hscroll (2)
it.current_x (84)
it.pixel_width (7)
* * *
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
12 455 467 17 111000110000 0 192 147 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 29 00
1 C -1 0 7 0x000031 1 29 00
2 C -1 0 7 0x000033 3 29 00
3 C -1 0 7 0x000020 29 00
4 S 455 B 35 0x000000 31 00
5 C 456 B 7 0x000048 H 0 00
6 C 457 B 7 0x000065 e 0 00
7 C 458 B 7 0x00006c l 0 00
8 C 459 B 7 0x00006c l 0 00
9 C 460 B 7 0x00006f o 0 00
10 C 461 B 7 0x00002d - 0 00
11 C 462 B 7 0x000077 w 0 00
12 C 463 B 7 0x00006f o 0 00
13 C 464 B 7 0x000072 r 0 00
14 C 465 B 7 0x00006c l 0 00
15 C 466 B 7 0x000064 d 0 00
16 C 0 0 7 0x000020 0 00
I have further reduced/simplified our test function (below) that uses move_it_in_display_line_to to iterate over each character in the current line, and print the relevant values to STDERR as the iteration is occurring. At the very end of the test function, dump_glyph_row is called (following a forced redisplay) to help us compare its values to the previous values returned subsequent to each call of move_it_in_display_line_to.
DEFUN ("bug-30226", Fbug_30226, Sbug_30226, 0, 0, 0,
doc: /* Debug the pixel-width of a stretch tab. */)
(void)
{
Fscroll_left (make_number (1), Qnil);
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 = 1;
/* ******************************************************************************
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);
/* ******************************************************************************
MOVE IT OVER EACH CHARACTER ON THE CURRENT LINE.
****************************************************************************** */
while (true)
{
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\
w->hscroll (%d)\n\
it.current_x (%d)\n\
it.pixel_width (%d)\n",
count,
(it.c == 0
? "NOTHING"
: it.c == 187
? "TAB CHARACTER"
: it.c == '\t'
? "TAB STRETCH"
: "TEXT"),
it.c,
w_hscroll,
it.current_x,
it.pixel_width);
}
/* ******************************************************************************
MOVE IT -- INCREMENT == IT.PIXEL_WIDTH
****************************************************************************** */
rc = move_it_in_display_line_to (&it, ZV, it.current_x + it.pixel_width,
MOVE_TO_POS | MOVE_TO_X);
count = count + 1;
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");
struct glyph_row *glyph_row = MATRIX_ROW (w->current_matrix, it.vpos);
dump_glyph_row (glyph_row, it.vpos, 2);
bidi_unshelve_cache (itdata, false);
return Qnil;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-27 21:20 ` Keith David Bershatsky
@ 2018-01-28 18:15 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2018-01-28 18:15 UTC (permalink / raw)
To: Keith David Bershatsky; +Cc: 30226
> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
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 21:20 ` Keith David Bershatsky
@ 2018-01-28 19:52 ` Keith David Bershatsky
2018-01-29 16:14 ` Eli Zaretskii
2018-01-31 8:03 ` Keith David Bershatsky
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Keith David Bershatsky @ 2018-01-28 19:52 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30226
> 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.
Thank you, Eli, for the detailed explanation regarding what is happening underneath the Emacs hood. The "bottom line" method does not seem to be working in this example to calculate the tab STRETCH because it.current_x of the character that _follows_ the tab STRETCH is wrong when w->hscroll >= 2. In the revised bug-30226 function below: When w->hscroll >= 2, we take the it.current_x of the letter "H" in "Hello-world" (which is ostensibly 84); and, we subtract (it.first_visible_x + it.lnum_pixel_width). This gives us a purported tab STRETCH of 42; however, it should really be 35.
A. This is the first (1st) time we call the revised bug-30226 [w->hscroll == 1]:
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)
"Bottom Line" method -- pixel-width of tab STRETCH: 0
4. TAB STRETCH
it.c (9)
w->hscroll (1)
it.current_x (35)
it.pixel_width (42)
"Bottom Line" method -- pixel-width of tab STRETCH: 42
5. TEXT
it.c (72)
w->hscroll (1)
it.current_x (77)
it.pixel_width (7)
* * *
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
12 455 467 17 111000110000 0 192 154 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 29 00
1 C -1 0 7 0x000031 1 29 00
2 C -1 0 7 0x000033 3 29 00
3 C -1 0 7 0x000020 29 00
4 S 455 B 42 0x000000 31 00
5 C 456 B 7 0x000048 H 0 00
6 C 457 B 7 0x000065 e 0 00
7 C 458 B 7 0x00006c l 0 00
8 C 459 B 7 0x00006c l 0 00
9 C 460 B 7 0x00006f o 0 00
10 C 461 B 7 0x00002d - 0 00
11 C 462 B 7 0x000077 w 0 00
12 C 463 B 7 0x00006f o 0 00
13 C 464 B 7 0x000072 r 0 00
14 C 465 B 7 0x00006c l 0 00
15 C 466 B 7 0x000064 d 0 00
16 C 0 0 7 0x000020 0 00
B. This is the second (2nd) time we call the revised bug-30226 [w->hscroll == 2]:
1. NOTHING
it.c (0)
w->hscroll (2)
it.current_x (0)
it.pixel_width (0)
2. TAB CHARACTER
it.c (187)
w->hscroll (2)
it.current_x (0)
it.pixel_width (7)
3. TAB STRETCH
it.c (9)
w->hscroll (2)
it.current_x (7)
it.pixel_width (49)
"Bottom Line" method -- pixel-width of tab STRETCH: 42
4. TEXT
it.c (72)
w->hscroll (2)
it.current_x (84)
it.pixel_width (7)
* * *
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
12 455 467 17 111000110000 0 192 147 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 29 00
1 C -1 0 7 0x000031 1 29 00
2 C -1 0 7 0x000033 3 29 00
3 C -1 0 7 0x000020 29 00
4 S 455 B 35 0x000000 31 00
5 C 456 B 7 0x000048 H 0 00
6 C 457 B 7 0x000065 e 0 00
7 C 458 B 7 0x00006c l 0 00
8 C 459 B 7 0x00006c l 0 00
9 C 460 B 7 0x00006f o 0 00
10 C 461 B 7 0x00002d - 0 00
11 C 462 B 7 0x000077 w 0 00
12 C 463 B 7 0x00006f o 0 00
13 C 464 B 7 0x000072 r 0 00
14 C 465 B 7 0x00006c l 0 00
15 C 466 B 7 0x000064 d 0 00
16 C 0 0 7 0x000020 0 00
DEFUN ("bug-30226", Fbug_30226, Sbug_30226, 0, 0, 0,
doc: /* Debug the pixel-width of a stretch tab. */)
(void)
{
Fscroll_left (make_number (1), Qnil);
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 = 1;
int previous_character = 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);
/* ******************************************************************************
MOVE IT OVER EACH CHARACTER ON THE CURRENT LINE.
****************************************************************************** */
while (true)
{
if (ITERATOR_AT_END_OF_LINE_P (&it)
|| FETCH_BYTE (IT_BYTEPOS (it)) == '\n'
|| rc == MOVE_POS_MATCH_OR_ZV)
break;
/* ******************************************************************************
HYPOTHETICAL CALCULATION OF PIXEL-WIDTH
****************************************************************************** */
if (w->hscroll > 0
&& previous_character == '\t')
fprintf (stderr, "\n\"Bottom Line\" method -- pixel-width of tab STRETCH: %d\n",
it.current_x - (it.first_visible_x + it.lnum_pixel_width));
/* ******************************************************************************
DUMP RELEVANT GLYPH INFORMATION
****************************************************************************** */
if (w->hscroll > 0)
{
int w_hscroll = w->hscroll;
fprintf (stderr, "\n%d. %s\n\
it.c (%d)\n\
w->hscroll (%d)\n\
it.current_x (%d)\n\
it.pixel_width (%d)\n",
count,
(it.c == 0
? "NOTHING"
: it.c == 187
? "TAB CHARACTER"
: it.c == '\t'
? "TAB STRETCH"
: "TEXT"),
it.c,
w_hscroll,
it.current_x,
it.pixel_width);
}
/* ******************************************************************************
MOVE IT -- INCREMENT == IT.PIXEL_WIDTH
****************************************************************************** */
previous_character = it.c;
rc = move_it_in_display_line_to (&it, ZV, it.current_x + it.pixel_width,
MOVE_TO_POS | MOVE_TO_X);
count = count + 1;
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");
struct glyph_row *glyph_row = MATRIX_ROW (w->current_matrix, it.vpos);
dump_glyph_row (glyph_row, it.vpos, 2);
bidi_unshelve_cache (itdata, false);
return Qnil;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-28 19:52 ` Keith David Bershatsky
@ 2018-01-29 16:14 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2018-01-29 16:14 UTC (permalink / raw)
To: Keith David Bershatsky; +Cc: 30226
> Date: Sun, 28 Jan 2018 11:52:48 -0800
> From: Keith David Bershatsky <esq@lawlist.com>
> Cc: 30226@debbugs.gnu.org
>
> > 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.
>
> Thank you, Eli, for the detailed explanation regarding what is happening underneath the Emacs hood. The "bottom line" method does not seem to be working in this example to calculate the tab STRETCH because it.current_x of the character that _follows_ the tab STRETCH is wrong when w->hscroll >= 2. In the revised bug-30226 function below: When w->hscroll >= 2, we take the it.current_x of the letter "H" in "Hello-world" (which is ostensibly 84); and, we subtract (it.first_visible_x + it.lnum_pixel_width). This gives us a purported tab STRETCH of 42; however, it should really be 35.
No, the method does work, you just used the results of the call to
move_it_in_display_line_to not as I intended.
>
> A. This is the first (1st) time we call the revised bug-30226 [w->hscroll == 1]:
>
> 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)
>
> "Bottom Line" method -- pixel-width of tab STRETCH: 0
>
> 4. TAB STRETCH
> it.c (9)
> w->hscroll (1)
> it.current_x (35)
> it.pixel_width (42)
>
> "Bottom Line" method -- pixel-width of tab STRETCH: 42
>
> 5. TEXT
> it.c (72)
> w->hscroll (1)
> it.current_x (77)
> it.pixel_width (7)
Observer how here, 77 - 35 (the previous value of current_x) gives you
42, which is what you want.
> B. This is the second (2nd) time we call the revised bug-30226 [w->hscroll == 2]:
>
> 1. NOTHING
> it.c (0)
> w->hscroll (2)
> it.current_x (0)
> it.pixel_width (0)
>
> 2. TAB CHARACTER
> it.c (187)
> w->hscroll (2)
> it.current_x (0)
> it.pixel_width (7)
>
> 3. TAB STRETCH
> it.c (9)
> w->hscroll (2)
> it.current_x (7)
> it.pixel_width (49)
>
> "Bottom Line" method -- pixel-width of tab STRETCH: 42
>
> 4. TEXT
> it.c (72)
> w->hscroll (2)
> it.current_x (84)
> it.pixel_width (7)
And here, 84 - 7 gives you 77. But since 84 is greater than
first_visible_x + lnum_pixel_width = 14 + 28 = 42, you must subtract
42 from the result, which gives 77 - 42 = 35, as you want. All the
following results for w->hscroll > 2 will work the same as this one.
> fprintf (stderr, "\n\"Bottom Line\" method -- pixel-width of tab STRETCH: %d\n",
> it.current_x - (it.first_visible_x + it.lnum_pixel_width));
This is not how I meant for you to calculate the width. You need to
subtract consecutive values of current_x, and then correct the result
as explained above.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-23 7:32 bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers Keith David Bershatsky
` (2 preceding siblings ...)
2018-01-28 19:52 ` Keith David Bershatsky
@ 2018-01-31 8:03 ` Keith David Bershatsky
2018-02-04 19:21 ` Keith David Bershatsky
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Keith David Bershatsky @ 2018-01-31 8:03 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30226
Thank you, Eli, for teaching me how to mathematically calculate the pixel-width of the tab STRETCH in this example ["C-q TAB Hello-world" and w->hscroll >= 2]. Greatly appreciated! :)
A couple of additional questions whenever you have the time . . . .
I. In the context of this particular example ["C-q TAB Hello-world" and w->hscroll >= 2], we cannot calculate the _relative_ X by subtracting it.first_visible_x from it.current_x (as would generally be the case). Is there a better way to calculate the _relative_ X instead of keeping track of _every_ visual pixel-width to the left of the current _relative_ X that we are seeking?
(For example, pgrowx and dump_glyph_row both just keep track of the prior _relative_ X and they add the prior pixel-width to derive the current _relative_ X.)
II. In a slightly different fact pattern with two (2) consecutive tabs ["C-q TAB C-q TAB Hello-world" and w->hscroll >= 2], the tab STRETCH of the second tab _visually_ disappears and dump_glyph_row reports that its width is -1. Is there a way to programmatically know whether the tab STRETCH is -1 when we are using move_it_in_display_line_to?
The following are the results of two calls to the revised function bug-30226 using this new hypothetical with two (2) consecutive tabs. And, I added a printout of pgrowx when w->hscroll == 2.
A. This is the first (1st) time we call the revised bug-30226 [w->hscroll == 1]:
1. NOTHING
it.c (0)
w->hscroll (1)
it.hpos (0)
it.current_x (0)
it.pixel_width (0)
2. TAB CHARACTER
it.c (187)
w->hscroll (1)
it.hpos (0)
it.current_x (0)
it.pixel_width (7)
3. TAB STRETCH
it.c (9)
w->hscroll (1)
it.hpos (0)
it.current_x (7)
it.pixel_width (49)
4. TAB STRETCH
it.c (9)
w->hscroll (1)
it.hpos (4)
it.current_x (35)
it.pixel_width (42)
5. TAB CHARACTER
it.c (187)
w->hscroll (1)
it.hpos (5)
it.current_x (77)
it.pixel_width (7)
6. TAB STRETCH
it.c (9)
w->hscroll (1)
it.hpos (6)
it.current_x (84)
it.pixel_width (49)
7. TEXT
it.c (72)
w->hscroll (1)
it.hpos (7)
it.current_x (133)
it.pixel_width (7)
* * *
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
12 455 468 19 111000110000 0 192 210 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 29 00
1 C -1 0 7 0x000031 1 29 00
2 C -1 0 7 0x000033 3 29 00
3 C -1 0 7 0x000020 29 00
4 S 455 B 42 0x000000 31 00
5 C 456 B 7 0x0000bb . 32 00
6 S 456 B 49 0x000000 31 00
7 C 457 B 7 0x000048 H 0 00
8 C 458 B 7 0x000065 e 0 00
9 C 459 B 7 0x00006c l 0 00
10 C 460 B 7 0x00006c l 0 00
11 C 461 B 7 0x00006f o 0 00
12 C 462 B 7 0x00002d - 0 00
13 C 463 B 7 0x000077 w 0 00
14 C 464 B 7 0x00006f o 0 00
15 C 465 B 7 0x000072 r 0 00
16 C 466 B 7 0x00006c l 0 00
17 C 467 B 7 0x000064 d 0 00
18 C 0 0 7 0x000020 0 00
B. This is the second (2nd) time we call the revised bug-30226 [w->hscroll == 2]:
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)
3. TAB STRETCH
it.c (9)
w->hscroll (2)
it.hpos (0)
it.current_x (7)
it.pixel_width (49)
4. TAB CHARACTER
it.c (187)
w->hscroll (2)
it.hpos (5)
it.current_x (84)
it.pixel_width (7)
5. TAB STRETCH
it.c (9)
w->hscroll (2)
it.hpos (6)
it.current_x (91)
it.pixel_width (35)
6. TEXT
it.c (72)
w->hscroll (2)
it.hpos (7)
it.current_x (126)
it.pixel_width (7)
* * *
Row Start End Used oE><\CTZFesm X Y W H V A P
==============================================================================
12 455 468 19 111000110000 0 192 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 29 00
1 C -1 0 7 0x000031 1 29 00
2 C -1 0 7 0x000033 3 29 00
3 C -1 0 7 0x000020 29 00
4 S 455 B 35 0x000000 31 00
5 C 456 B 7 0x0000bb . 32 00
6 S 456 B -1 0x000000 31 00
7 C 457 B 7 0x000048 H 0 00
8 C 458 B 7 0x000065 e 0 00
9 C 459 B 7 0x00006c l 0 00
10 C 460 B 7 0x00006c l 0 00
11 C 461 B 7 0x00006f o 0 00
12 C 462 B 7 0x00002d - 0 00
13 C 463 B 7 0x000077 w 0 00
14 C 464 B 7 0x00006f o 0 00
15 C 465 B 7 0x000072 r 0 00
16 C 466 B 7 0x00006c l 0 00
17 C 467 B 7 0x000064 d 0 00
18 C 0 0 7 0x000020 0 00
PGROWX: pgrowx glyph_row
TEXT: 19 glyphs
0 0: CHAR[ ] pos=-1 blev=2,btyp=EN w=7 a+d=12+4 face=31 MB AVOID
1 7: CHAR[1] pos=-1 blev=2,btyp=EN w=7 a+d=12+4 face=31 MB AVOID
2 14: CHAR[4] pos=-1 blev=2,btyp=EN w=7 a+d=12+4 face=31 MB AVOID
3 21: CHAR[ ] pos=-1 blev=2,btyp=EN w=7 a+d=12+4 face=31 MB AVOID
4 28: STRETCH[16+12] pos=456 w=35 a+d=12+4 face=33 MB
5 63: CHAR[0xbb] pos=457 blev=0,btyp=L w=7 a+d=12+4 face=34 MB
6 70: STRETCH[16+12] pos=457 w=-1 a+d=12+4 face=33 MB
7 69: CHAR[H] pos=458 blev=0,btyp=L w=7 a+d=12+4 MB
8 76: CHAR[e] pos=459 blev=0,btyp=L w=7 a+d=12+4 MB
9 83: CHAR[l] pos=460 blev=0,btyp=L w=7 a+d=12+4 MB
10 90: CHAR[l] pos=461 blev=0,btyp=L w=7 a+d=12+4 MB
11 97: CHAR[o] pos=462 blev=0,btyp=L w=7 a+d=12+4 MB
12 104: CHAR[-] pos=463 blev=0,btyp=L w=7 a+d=12+4 MB
13 111: CHAR[w] pos=464 blev=0,btyp=L w=7 a+d=12+4 MB
14 118: CHAR[o] pos=465 blev=0,btyp=L w=7 a+d=12+4 MB
15 125: CHAR[r] pos=466 blev=0,btyp=L w=7 a+d=12+4 MB
16 132: CHAR[l] pos=467 blev=0,btyp=L w=7 a+d=12+4 MB
17 139: CHAR[d] pos=468 blev=0,btyp=L w=7 a+d=12+4 MB
18 146: CHAR[ ] pos=0 blev=0,btyp=B w=7 a+d=12+4 MB
DEFUN ("bug-30226", Fbug_30226, Sbug_30226, 0, 0, 0,
doc: /* Debug the pixel-width of a stretch tab. */)
(void)
{
Fscroll_left (make_number (1), Qnil);
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 = 1;
int previous_char = 0;
int previous_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)
{
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\
w->hscroll (%d)\n\
it.hpos (%d) \n\
it.current_x (%d)\n\
it.pixel_width (%d)\n",
count,
(it.c == 0
? "NOTHING"
: it.c == 187
? "TAB CHARACTER"
: it.c == '\t'
? "TAB STRETCH"
: "TEXT"),
it.c,
w_hscroll,
it.hpos,
it.current_x,
it.pixel_width);
}
/* ******************************************************************************
MOVE IT -- INCREMENT == IT.PIXEL_WIDTH
****************************************************************************** */
previous_char = it.c;
previous_x = it.current_x;
rc = move_it_in_display_line_to (&it, ZV, it.current_x + it.pixel_width,
MOVE_TO_POS | MOVE_TO_X);
count = count + 1;
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;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-23 7:32 bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers Keith David Bershatsky
` (3 preceding siblings ...)
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 18:52 ` Keith David Bershatsky
6 siblings, 0 replies; 12+ messages in thread
From: Keith David Bershatsky @ 2018-02-04 19:21 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30226
I was able to locate the section of code responsible (at least in part) for the width of a tab STRETCH taking on a negative value and visually disappearing as stated in paragraph II of the preceding email; i.e., thread message #26:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30226#26
xdisp:28250 to xdisp:28252
/* Adjust for line numbers, if needed. */
if (!NILP (Vdisplay_line_numbers) && x0 >= it->lnum_pixel_width)
x -= it->lnum_pixel_width;
In that section of code (lines 28250 to 28252), x is adjusted for line numbers; however, x0 is _not_ adjusted. This leads to the second (and other subsequent) tab STRETCH visually disappearing and take on a negative width value when w->hscroll >= 2. Here is a link to a screenshot of what this looks like:
https://www.lawlist.com/images/30226_a.png
I have not yet been able to figure out why the values for it->current_x are different when glyphs are actually produced, versus when we only use move_it_in_display_line_to without producing glyphs. The difference in behavior in the value for X and all values derived therefrom (e.g., pixel_width) can be seen by setting up a few stderr messages in x_produce_glyphs and also move_it_in_display_line_to. When append_stretch_glyph is called (because glyphs are actually being produced), the value for it->pixel_width is completely different than when we only call move_it_in_display_line_to without producing glyphs.
In the event that I am able to make any additional headway, I'll post a follow-up to #30226.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-23 7:32 bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers Keith David Bershatsky
` (4 preceding siblings ...)
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
6 siblings, 1 reply; 12+ messages in thread
From: Keith David Bershatsky @ 2018-02-19 2:17 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30226
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.
In the revised fact pattern described in Message #26 of this thread (i.e., TAB TAB Hello-world, and w->hscroll >= 2):
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30226#26
it->current_x stops at 7 when w->hscroll == 2.
In the simulation (move_it_in_display_line_to that does _not_ produce glyphs), it is impossible to reach an X of 35 because that would be in the middle of the STRETCH. maybe_produce_line_number does _not_ advance it->current_x in the simulation.
In the real thing (display_line actually produces glyphs), maybe_produce_line_number advances it->current_x from 7 to 35.
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-02-19 2:17 ` Keith David Bershatsky
@ 2018-02-19 16:28 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2018-02-19 16:28 UTC (permalink / raw)
To: Keith David Bershatsky; +Cc: 30226
> 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...
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
2018-01-23 7:32 bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers Keith David Bershatsky
` (5 preceding siblings ...)
2018-02-19 2:17 ` Keith David Bershatsky
@ 2018-02-19 18:52 ` Keith David Bershatsky
6 siblings, 0 replies; 12+ messages in thread
From: Keith David Bershatsky @ 2018-02-19 18:52 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 30226
[-- 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 --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-02-19 18:52 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).