Package: emacs;
Reported by: JD Smith <jdtsmith <at> gmail.com>
Date: Sun, 3 Dec 2023 16:56:01 UTC
Severity: normal
Done: Eli Zaretskii <eliz <at> gnu.org>
View this message in rfc822 format
From: JD Smith <jdtsmith <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 67604 <at> debbugs.gnu.org Subject: bug#67604: Motion problems with inline images Date: Wed, 28 May 2025 19:42:50 -0400
[Message part 1 (text/plain, inline)]
> On May 27, 2025, at 9:23 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> From: JD Smith <jdtsmith <at> gmail.com>
>> Date: Sat, 17 May 2025 23:13:45 -0400
>> Cc: 67604 <at> debbugs.gnu.org
>>
>> I have some progress to report. To keep things simple, I compared two cases, both moving vertically down by one line, starting with point on the just-wrapped green image:
>>
>> 1. The "fine-tuned" case, which skips a line.
>> 2. A "nearby" case with the same window width that doesn't exhibit the bug (no skip).
>
> Thanks, and sorry for my long delays. These issues need some time and
> a quiet environment, and I don't easily get both together...
No problem. Indeed this stretch of code requires some serious concentration. In fact here I concentrated so much in responding to you that another yet-deeper possibility occurred to me at the very end. Apologies for the (possible) late plot twist.
>> In terms of the special case path for "last glyph just fits": this is appropriate for (non-word) window wrapping, but not, I believe word-wrapping. Why? I noticed that wrapped words carry _all_ their trailing whitespace with them when they wrap (presumably to keep WS from wrapping to the start of a display line). In other words, why care if the green image itself can "barely fit" on a line, when in fact, it will _already have wrapped_ at a larger window width, due to the trailing whitespace (of which there must be at least one).
>
> That condition, i.e.
>
> if (/* IT->hpos == 0 means the very first glyph
> doesn't fit on the line, e.g. a wide
> image. */
> it->hpos == 0
> || (new_x == it->last_visible_x //**!!! FINE-TUNING
> && FRAME_WINDOW_P (it->f)))
>
> is because GUI windows can show the cursor on the fringe. So even if
> a glyph "barely fits", we can show the cursor after it, and don't need
> an extra column of the window's text-area for that purpose. See
> overflow-newline-into-fringe.
Got it, thanks. I did notice that the overflow feature seems to work even when the branch isn't taken; see below.
> Given what you say, it might be TRT to not go there under word-wrap
> when the next character can wrap, but please verify that:
The idea is not to go there under word-wrap _unless_ you can wrap directly after (i.e. IT is on a WS). Because otherwise, the just-fitting glyph at it->current (e.g., the green box) will certainly wrap — it only "just fits" if it's followed by a newline. Technically it's true that a trailing WS that seems to "just fit" could in fact also wrap. I.e. imagine something like the first trailing space just filling the window width in this:
"XXXXX "
But apparently avoiding the branch in this case doesn't matter in practice (likely because BUFFER_POS_REACHED_P works fine for such chars; see below).
> . it->current was already moved to the character after the image,
> because AFAIR char_can_wrap_after looks at the character at
> it->current
When the branch above would have been taken — an outcome which is now disabled by my patch — it->current is at pos=89, on the green box. It seems `char_can_wrap_after' just checks if the current char is whitespace (false, for the green box). So at pos=89, the previous char (88, a WS) could wrap after, and the current char (89, green box) can wrap before, so wrap_it is stored as a suitable wrap position. After this test, at pos=89, may_wrap=false, since you can't wrap directly after a non-WS char.
I think this all checks out. In fact on review I noticed we can just use the saved variable may_wrap instead of using char_can_wrap_after(it); they are equivalent.
> . when this problem happens with the image right next to a newline
> that ends a line, we can still show the cursor on the fringe when
> you move point to the buffer position after the image, both with
> and without word-wrap
I added a newline after the green box. By adjusting the red box width until the green box "just fits" on the line (144pix), I can confirm with this patch that the cursor correctly overflows from the green box into the fringe, with and without word wrap. It would appear however that the above branch is now never taken with WORD_WRAP except when you are on a whitespace which "just fits". But in all cases the fringe-newline still appears to work.
> If the patch passes these two tests, feel free to install on the
> master branch.
It did not pass test 1, but I don't think it should have, since I believe TRT is to ignore just-fitting non-WS glyphs that cannot wrap directly after (since they themselves will wrap!).
I modified the patch to give priority to the hpos==0 (overly wide image) test, and used the saved variable may_wrap; see attached.
PLOT TWIST:
===========
This all works fine, but there was one tiny distinction that bothered me as I tested fringe newline overflow. When moving past (with forward-char) the "just fits" green box when it is followed by a newline, rather than returning MOVE_POS_MATCH_OR_ZV, move_it_in_display_line_to now returns MOVE_LINE_CONTINUED. Again, this results in no operational difference I could find, and maybe it matters not one bit, but it gave me pause.
This thought then brought me deeper, to this macro:
#define BUFFER_POS_REACHED_P() \
((op & MOVE_TO_POS) != 0 \
&& BUFFERP (it->object) \
&& (IT_CHARPOS (*it) == to_charpos \
|| ((!it->bidi_p \
|| BIDI_AT_BASE_LEVEL (it->bidi_it)) \
&& IT_CHARPOS (*it) > to_charpos) \
|| (it->what == IT_COMPOSITION \
&& ((IT_CHARPOS (*it) > to_charpos \
&& to_charpos >= it->cmp_it.charpos) \
|| (IT_CHARPOS (*it) < to_charpos \
&& to_charpos <= it->cmp_it.charpos)))) \
&& (it->method == GET_FROM_BUFFER \
|| (it->method == GET_FROM_DISPLAY_VECTOR \
&& it->dpvec + it->current.dpvec_index + 1 >= it->dpend)))
When the iterator arrives on the green box, it has reached its destination (to_charpos=89). And yet, it does not stop. Moving past the green box is what causes all the problems. Why does it move on? Everything here in this macro test is true, _except_ for it->method==GET_FROM_BUFFER and following. As we're on an image, we have it->method=GET_FROM_IMAGE. So, it fails.
Can you never actually reach a TO_CHARPOS position that is on an image (or stretch)? Are you doomed to move past it? That I don't understand. If this macro returned True for pos=89, as I think it perhaps should, this whole issue would never have occurred. The just-fits branch would notice that the destination position (namely, the green image) had been reached, save the position in atpos_it, and just prior to exiting, restore it.
And yet, I think based on the various comments I've seen in the code about "moving past" images, BUFFER_POS_REACHED_P is perhaps intentionally False for images/stretches. Is that your understanding?
I noticed this ChangeLog entry from Richard in 2004:
* xdisp.c (BUFFER_POS_REACHED_P): We haven't reached the specified
position if we're reading from something other than the buffer.
Why? Maybe that's from an earlier time and only meant for (e.g.) strings.
If I revert the original patch, and instead just redefine the macro (see alt patch) to read:
...
&& (it->method == GET_FROM_BUFFER \
|| it->method == GET_FROM_IMAGE \
|| it->method == GET_FROM_STRETCH \
|| (it->method == GET_FROM_DISPLAY_VECTOR \
...
this also fixes the original line-skip bug, has all correct behavior including fringe newline overflow, etc., all without any change to the just-fits branch test or any of the return values. Note that this macro is used 8 times in xdisp.c.
JD
[inline-image-line-skip-2.patch (application/octet-stream, attachment)]
[inline-image-line-skip-alt.patch (application/octet-stream, attachment)]
[Message part 4 (text/plain, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.