unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Keith David Bershatsky <esq@lawlist.com>
Cc: 30226@debbugs.gnu.org
Subject: bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.
Date: Sat, 27 Jan 2018 16:56:05 +0200	[thread overview]
Message-ID: <83bmhfjqpm.fsf@gnu.org> (raw)
In-Reply-To: <m2607tqaff.wl%esq@lawlist.com> (message from Keith David Bershatsky on Mon, 22 Jan 2018 23:52:20 -0800)

> 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?





  reply	other threads:[~2018-01-27 14:56 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 [this message]
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

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

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83bmhfjqpm.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=30226@debbugs.gnu.org \
    --cc=esq@lawlist.com \
    /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 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).