all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Alex <agrambot@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: emacs-devel@gnu.org
Subject: Re: Native line numbers, final testing
Date: Sat, 01 Jul 2017 15:00:50 -0600	[thread overview]
Message-ID: <87mv8nkh31.fsf@lylat> (raw)
In-Reply-To: <83shigpoxq.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 01 Jul 2017 11:00:01 +0300")

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 30 Jun 2017 15:15:00 -0600
>> 
>> I noticed another bug. With `display-line-numbers' set to t or 'relative
>> in xdisp.c:
>> 
>> M-g c 2970 RET
>> C-n
>> 
>> Notice that C-n moved back one column. This does not happen with
>> 'visual, nor does it happen when the window is sufficiently small. This
>> is in emacs -Q, and only happens with non-nil line-move-visual.
>
> Such problems (which IMO are minor and quite rare) cannot be helped,
> unfortunately.  They happen near the point where the line numbers will
> soon need one more or one less column.
>
> The underlying issue is how this feature calculates the number of
> columns required for displaying all the numbers in the visible portion
> of a window.  The naïve method would be to perform the window display
> twice, because you only know the largest line number in view after
> actually displaying all the lines.  But that is too slow, and would
> land us right back at the slow operation of linum-mode and friends.
>
> So instead we guess the maximum line number without actually counting
> the lines in the window.  That guess must be conservative, otherwise
> you'll see the text shift to the right in the middle of a window.
> Therefore the guess uses the maximum number of screen lines a window
> could possibly display without resizing, by using the number of rows
> in the underlying display structure called the "glyph matrix".  This
> assumes that (1) all of the glyph-matrix rows will be used (usually
> false, except on TTYs), and (2) that all the shown lines are
> non-continuation lines.
>
> This is why many times you see the number of columns grow before the
> additional column is actually needed by the last line number displayed
> at window's bottom.
>
> The same code, and the same guessing, is also used by various
> functions that simulate display, such as vertical-motion (which is
> called internally by C-n).  Except that the vertical position of point
> in the virtual "window" they consider is taken as zero, because in
> general it cannot be known (the function could be called in the middle
> of a Lisp program that moved point, and the updated vertical position
> will not be determined until the next redisplay cycle).  So the
> resulting number of columns these functions compute could be different
> from what actually is, or will be, on the screen.  And therefore
> column calculations under line-move-visual will err by the same
> amount.
>
> IOW, these issues are a (small, IMO) fine we need to pay for having
> such fast line-number display.
>
> I will keep this issue in mind, in case some idea emerges for how to
> fix them, thanks for pointing it out.

Thanks for explaining, but I disagree on the severity of this bug. It is
relatively rare, but navigating a buffer with C-n/C-p just before the
line 100/1000 isn't exactly uncommon. I believe consistency of C-n/C-p,
at least with the same size window, is quite important. Imagine if
C-f/C-b also went up/down a line if the column number was, e.g., "a bit"
before 10^k, for integers k > 1. That would break users' expectations of
these commands considerably, even if, with the exception of the column
"a bit" before 100, these column positions are rare. This is worsened by
the fact that "a bit" is dependent on the window size, meaning it's
harder to reason about exactly.

One could argue that if you like consistency, then you should set
line-move-visual to nil. I suppose that's true in some cases, but that
doesn't mean that a non-nil value should just ignore consistency if it
can somehow be preserved. Also, visual-line-mode is helpful for some
types of documents (e.g. LaTeX), and it's natural to have
line-move-visual be non-nil in this case. It would be a shame to have to
choose between native display line numbers and visual-line-mode in those
documents.

If no good solution can be found, then I think even a slowdown for
C-n/C-p would be acceptable if it meant a fix for this issue.

I have a couple more questions if that's alright:

It's not intuitive to me why this occurs between lines with the same
display line number width. If vertical-motion uses the same heuristic as
display of line numbers, then why is the column changing between lines
even when the width of line numbers isn't? Is it because it's using
the heuristic with different inputs? If so, can't they be modified to
achieve the same results as the display of line numbers?

Why is it necessary for line numbers to actually affect the vertical
position of characters in the buffer? I suppose it's a bit late to be
asking this question, but the approach from an outside view feels odd. I
don't know what the options were, but it's odd that line numbers aren't
in their own special area like in (n)linum. Does the display engine not
work well with margins?

>> I'm not planning to use 'visual, and I of course don't claim to know
>> anything about this, but I'm curious as to why it works like this.
>> goto-line in either style always goes to the same absolute line number,
>> right? Does the line number display calculation occur after the point is
>> already moved to this line?
>
> Yes.  Line numbers are calculated as part of redisplay, and that
> happens when point already moved.

>> If so, then doesn't the display engine just need to calculate the
>> visual line numbers around this point? If so, then why does a large
>> jump like M-g g 25000 take so much longer comparatively than M-g g
>> 1000? If these assumptions are wrong (they likely are), why are they
>> wrong?
>
> Because the display engine employs a lot of optimizations, to try and
> shortcut expensive redrawing of the entire window, and to comply with
> user expectations.  It doesn't, and cannot, know that buffer position
> 25000 is necessarily outside the window where the command was invoked.
> E.g., you could have some text-folding mode, such as Outline, active
> in the buffer; or you could have lines truncated with only a few very
> long lines between the window-start and position 25000.
>
> So redisplay first tries to reuse the same window-start point as it
> used in the previous cycle, because that's what you as a user would
> expect: if the new point is inside the same window, you don't expect
> the window to scroll.  Deciding whether the previous window-start is
> still usable needs to check whether point will be shown inside the
> window, and that requires to try reaching point from the original
> window-start, in this case position 1, and counting screen lines as we
> do, until we either hit point or the window's bottom.
>
> It turns out that the 'visual' mode of line numbers made this test
> unnecessarily slow, so a simple heuristic was enough to avoid that.
> See the latest branch, where 'visual' and 'relative' should now
> redisplay at the same speed in this case.

Thanks for the explanation and fix. The heuristic does indeed make it a
lot faster.



  reply	other threads:[~2017-07-01 21:00 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-30 14:49 Native line numbers, final testing Eli Zaretskii
2017-06-30 17:51 ` Alex
2017-06-30 18:20   ` Eli Zaretskii
2017-06-30 19:06     ` Alex
2017-06-30 19:55       ` Eli Zaretskii
2017-06-30 21:15         ` Alex
2017-07-01  8:00           ` Eli Zaretskii
2017-07-01 21:00             ` Alex [this message]
2017-07-02  2:40               ` Eli Zaretskii
2017-07-02  5:16                 ` Alex
2017-07-02 15:10                   ` Eli Zaretskii
2017-07-02 16:47                     ` Stefan Monnier
2017-07-02 16:51                       ` Eli Zaretskii
2017-07-02 17:38                         ` Stefan Monnier
2017-07-02 19:27                           ` Eli Zaretskii
2017-07-03  5:06                     ` Alex
2017-07-03 15:24                       ` Eli Zaretskii
2017-07-04 19:36                         ` Alex
2017-07-05 17:39                           ` Eli Zaretskii
2017-07-07  2:46                             ` Alex
2017-07-07  6:19                               ` Eli Zaretskii
2017-07-07  9:24                                 ` Eli Zaretskii
2017-07-08 20:51                                   ` Alex
2017-07-09 20:16                                     ` James Cloos
2017-07-09 21:45                                       ` Yuri Khan
2017-07-10  2:33                                         ` Eli Zaretskii
2017-07-10  7:09                                           ` Yuri Khan
2017-07-10 17:02                                             ` Eli Zaretskii
2017-07-10  2:31                                       ` Eli Zaretskii
2017-07-10  5:35                                         ` James Cloos
2017-07-10 17:00                                           ` Eli Zaretskii
2017-07-10 18:15                                             ` Filipe Silva
2017-07-10 18:18                                               ` Eli Zaretskii
2017-07-10 18:23                                                 ` Filipe Silva
2017-07-10 18:32                                             ` James Cloos
2017-07-11 20:58                                             ` Alex
2017-07-11 21:18                                               ` Filipe Silva
2017-07-11 21:20                                                 ` Filipe Silva
2017-07-11 21:37                                                   ` Alex
2017-07-12  2:35                                               ` Eli Zaretskii
2017-07-12  2:53                                                 ` Alex
2017-07-12 14:21                                                   ` Eli Zaretskii
2017-07-12 17:22                                                     ` Alex
2017-07-12 17:25                                                       ` Alex
2017-07-12 18:38                                                       ` Eli Zaretskii
2017-07-12 20:03                                                         ` Alex
2017-07-13  2:38                                                           ` Eli Zaretskii
2017-07-13  4:11                                                             ` Alex
2017-07-13 15:56                                                               ` Eli Zaretskii
2017-07-26  3:50                                                                 ` Alex
2017-07-26 14:42                                                                   ` Eli Zaretskii
2017-07-29  6:12                                                                     ` Alex
2017-07-29  7:01                                                                       ` Eli Zaretskii
2017-07-07  9:47                                 ` Eli Zaretskii
2017-07-07  9:49                                   ` Eli Zaretskii
2017-07-07 11:14                                     ` Filipe Silva
2017-07-07 12:21                                       ` Eli Zaretskii
2017-07-07 12:29                                   ` Eli Zaretskii
     [not found]                                     ` <CAEwkUWN8GkCyOiF4jEgKuZwJHhvMgJi9yVnvggRvu+Yddfp4qQ@mail.gmail.com>
2017-07-07 12:56                                       ` Filipe Silva
2017-07-01  1:59 ` Filipe Silva
2017-07-02 19:27 ` James Nguyen
2017-07-03  2:33   ` Eli Zaretskii
2017-07-03  3:22     ` James Nguyen
2017-07-03 15:58       ` Eli Zaretskii
2017-07-03 17:04         ` James Nguyen
2017-07-04 10:57           ` Filipe Silva
2017-07-04 11:00             ` Filipe Silva
2017-07-04 13:51               ` Kaushal Modi
2017-07-04 14:30               ` Eli Zaretskii
2017-07-04 14:32             ` Eli Zaretskii
2017-07-04 14:48               ` Filipe Silva
2017-07-04 14:50                 ` Filipe Silva
2017-07-04 15:44                   ` Eli Zaretskii
2017-07-04 16:22                     ` Filipe Silva
2017-07-04 16:34                       ` Filipe Silva
2017-07-04 16:35                       ` Richard Copley
2017-07-04 16:44                         ` Eli Zaretskii
2017-07-04 17:13                           ` Richard Copley
2017-07-04 17:35                             ` Filipe Silva
2017-07-04 17:48                               ` Eli Zaretskii
2017-07-04 17:52                               ` Stefan Monnier
2017-07-10 18:22                                 ` Filipe Silva
2017-07-10 20:28                                   ` Stefan Monnier
2017-07-04 17:47                             ` Eli Zaretskii
2017-07-04 17:50                           ` Alex
2017-07-04 18:24                             ` Eli Zaretskii
2017-07-04 18:37                               ` Richard Copley
2017-07-04 18:43                                 ` Eli Zaretskii
2017-07-05 20:24 ` Andy Moreton
2017-07-06 17:24   ` Eli Zaretskii

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=87mv8nkh31.fsf@lylat \
    --to=agrambot@gmail.com \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@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.