unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: dick <dick.r.chiang@gmail.com>
Cc: 57669@debbugs.gnu.org
Subject: bug#57669: 29.0.50; C-n, C-p off under long lines
Date: Sat, 10 Sep 2022 10:45:42 +0300	[thread overview]
Message-ID: <838rmrbrkp.fsf@gnu.org> (raw)
In-Reply-To: <87wnace31g.fsf@dick> (message from dick on Fri, 09 Sep 2022 15:55:07 -0400)

> From: dick <dick.r.chiang@gmail.com>
> Cc: 57669@debbugs.gnu.org
> Date: Fri, 09 Sep 2022 15:55:07 -0400
> 
> > showing your code that deals with very long lines and explaining how
> > it solves that
> 
> I can't believe I'm dignifying this.  All line references from commit
> beb489e.
> 
> xdisp.c[9319,9369]: Calculates dy algebraically without char-by-char.

This uses state variables from 'struct it' which are _local_ and
_momentary_, i.e. they can change as result of processing any buffer
position, and reflect only the latest change, forgetting what was
before.  Specifically, it->method is such a state variable.  So making
global conclusions, such as that "all characters will have the same
metrics on display", based on it->method is fundamentally wrong, and
can only be true in very simple situations.

The buffer->text->monospace flag is likewise implemented based on
simplistic assumptions: that only text properties could violate the
"monospace-ness" attribute.  Here's why this is simplistic:

  . faces can be applied by overlays as well
  . faces can be applied directly by C code
  . faces can be applied by using glyphs from display-tables
  . even if we only have the default face, fonts used for various
    non-ASCII characters can have different metrics from the default
    face's font, and in some cases they can even be variable-pitch
    fonts
  . portions of display can be made invisible by selective-display,
    not just by 'invisible' properties

(I'm not claiming that the above is the exhaustive list of all the
reasons that the assumptions in behaved_p could be violated; there
could be more of them.)

> xdisp.c[9537,9577]: Calculates CAPPED to limit char-by-char scan for
> moving backwards (Blandy and Moellmann wrote xdisp like a singly linked
> list -- fast and precise forwards, much less so going backwards).

I cannot see how the above description is related to the actual code
in that part of xdisp.c: there's no char-by-char movement back in the
corresponding parts of our code; instead, we move back by lines, then
move forward by characters -- and that's what the code there does as
well.  So I think this part of the code is equivalent to what we do,
in back_to_previous_visible_line_start, and should have the same
performance.  But maybe I'm missing something -- was this change
profiled, and if so, what were the results?  And if this code is
significantly faster, which of its changes is responsible for the
speedups?  And in which kind of files under what major modes were
these speedups measured?

In any case, the replacement code uses behaved_p, whose problematic
assumptions I already explained above.

To comment on the above description: in general, moving the iterator
backwards needs to consider all the changes in the state variables of
'struct it' that eventually affect the metrics and the layout
calculations.  It is _not_ the same as moving back by characters, and
the reason is that the state changes of 'struct it' are defined (in
set_iterator_to_next, get_next_display_element, and their subroutines)
only for moving forward, not for moving back.  That is why all
move_it_* functions cannot move back, except by going to a preceding
newline and then coming forward.





  parent reply	other threads:[~2022-09-10  7:45 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-08  5:40 bug#57669: 29.0.50; C-n, C-p off under long lines dick.r.chiang
2022-09-08  7:38 ` Gregory Heytings
2022-09-08  8:17 ` Eli Zaretskii
2022-09-08 12:02   ` dick
2022-09-08 17:12     ` Gregory Heytings
2022-09-08 18:13       ` dick
2022-09-08 18:26         ` Gregory Heytings
2022-09-09  6:00         ` Eli Zaretskii
2022-09-09 13:08           ` dick
2022-09-09 13:38             ` Gregory Heytings
2022-09-09 14:34               ` dick
2022-09-09 14:39                 ` Gregory Heytings
2022-09-09 15:06                   ` dick
2022-09-09 15:41                     ` Gregory Heytings
2022-09-09 16:30                       ` dick
2022-09-09 16:42                         ` Gregory Heytings
2022-09-09 17:11                           ` dick
2022-09-09 18:09                             ` Gregory Heytings
2022-09-09 15:49                 ` Eli Zaretskii
2022-09-09 15:58                   ` Gregory Heytings
2022-09-09 16:04                     ` Eli Zaretskii
2022-09-09 14:12             ` Eli Zaretskii
2022-09-09 15:04               ` dick
2022-09-09 15:19                 ` Gregory Heytings
2022-09-09 15:52                   ` dick
2022-09-09 16:03                     ` Eli Zaretskii
2022-09-09 16:06                       ` Gregory Heytings
2022-09-09 16:19                         ` Eli Zaretskii
2022-09-09 16:25                           ` Gregory Heytings
2022-09-09 17:44                           ` dick
2022-09-09 18:06                             ` Eli Zaretskii
2022-09-09 18:22                               ` dick
2022-09-09 18:57                                 ` Eli Zaretskii
2022-09-09 19:28                                   ` dick
2022-09-09 19:38                                     ` Eli Zaretskii
2022-09-09 19:55                                   ` dick
2022-09-09 21:28                                     ` Gregory Heytings
2022-09-09 22:00                                       ` dick
2022-09-09 22:44                                         ` Gregory Heytings
2022-09-09 23:27                                           ` dick
2022-09-10  8:32                                             ` Eli Zaretskii
2022-09-10 12:51                                               ` dick
2022-09-10 13:09                                                 ` Eli Zaretskii
2022-09-10 13:29                                                   ` Eli Zaretskii
2022-09-10 13:22                                                 ` Eli Zaretskii
2022-09-10 14:03                                                   ` dick
2022-09-10 14:20                                                     ` Eli Zaretskii
2022-09-10 14:52                                                       ` dick
2022-09-10 10:26                                             ` Gregory Heytings
2022-09-10  7:45                                     ` Eli Zaretskii [this message]
2022-09-10 12:07                                       ` dick
2022-09-10 12:20                                         ` dick
2022-09-10 12:24                                         ` Eli Zaretskii
2022-09-10 16:55                                       ` Gregory Heytings

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=838rmrbrkp.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=57669@debbugs.gnu.org \
    --cc=dick.r.chiang@gmail.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).