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>
Message #89 received at 67604 <at> debbugs.gnu.org (full text, mbox):
From: JD Smith <jdtsmith <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 67604 <at> debbugs.gnu.org Subject: Re: bug#67604: Motion problems with inline images Date: Fri, 9 May 2025 20:51:40 -0400
> On May 9, 2025, at 12:17 PM, Eli Zaretskii <eliz <at> gnu.org> wrote: > > Thanks, I think you made good progress. > > But you are looking at the horizontal coordinates too early: they have > not yet been updated at that point. They are updated inside the for > loop which starts a few lines down. Yeah it's a challenge to know when various members of IT are yet valid. >> From there it iterates until new_x >= last_visible_x, and in so doing, "falls off the line". If it falls to the final newline of the subsequent wrapped line, this results in a "line jump". > > You lost me here. I don't see "new_x >= it->last_visible_x" in the > source, so which line did you have in mind? This was referring (obliquely) back to my prior message. It happens a couple levels deeper, via move_it_by_lines (&it, min (PTRDIFF_MAX, nlines)), to move_it_to: /* If no TO_CHARPOS and no TO_X specified, stop at the start of the line TO_VPOS. */ if ((op & (MOVE_TO_X | MOVE_TO_POS)) == 0) { if (it->vpos == to_vpos) { reached = 1; break; } else skip = move_it_in_display_line_to (it, -1, -1, 0); And in move_it_in_display_line_to(it, -1, -1, 0): if (/* Lines are continued. */ it->line_wrap != TRUNCATE && (/* And glyph doesn't fit on the line. */ new_x > it->last_visible_x That new_x is getting incremented by it->pixel_width until it falls off the screen line, but I believe it starts at the wrong value, since it's pushed to the right of the stretch/image without knowing it. That's the (as yet unsubstantiated) theory at least. >> SOLUTION(S): >> ============ >> >> It would seem the simple solution is to do the same thing for images and stretches as is done for display strings: move to the position right *before* PT. >> >> That could be as simple as enabling disp_string_at_start_p when starting on a (real) IMAGE or STRETCH. That does indeed fix the bug, but unfortunately it results in a new bug: now moving vertically *upwards* across wrapped wide glyphs jumps an extra line up. Sigh. > > I'm not sure your conclusions are correct. If they were, it would > mean that vertical-motion doesn't work at all when the first glyph on > a line is an image or a stretch glyph (what you call "specified > space"). But that is not true, is it? I admit I don't have a complete picture, but yes, a wrapped stretch/image at the beginning of a screen line does appear to cause other small vertical motion issues: cursor moves a bit off the expected column, after line move, etc. I don't understand exactly why it needs this "fine tuning" to manifest as a full line skip, so I could easily be coming to an incorrect understanding (or more likely an understanding of a non-essential part of the problem). > I think this problem is specific to the situation when the following > happen all at once: > > . lines are wrapped (it->line_wrap = WORD_WRAP) > . the image or the stretch glyph don't fit on their line and are > wrapped to the next line > . vertical-motion is invoked when point is on or after that image or > stretch > Am I right? Yes, this is all correct, except you only see the issue when point is on or directly after (next character) the image or stretch. And it's a good point. The fact that it doesn't appear on normal continuation lines, only WORD_WRAP continuations must be significant. And it's suspicious that it only appears when an image/stretch is finely balanced between not yet wrapped and just wrapped. You might recall that I can only reproduce the bug when the window width shrinks just enough to wrap the green image, then one char narrower. And then I still have to adjust the red image width on line 1 to "fine tune" the balance further (by "eating up" some of that one char wrap margin). > If I'm right, then we should try understanding why in this particular > situation the code doesn't work, whereas it does work when, say, the > image or stretch are in a non-continuation line or are not the first > glyph Or indeed on a non WORD_WRAP continuation line, as far as I can tell. And yet, I do certainly see that Fvertical_motion incorrectly returns IT to the pink box in the image I provided. For example, if you introduce a normal character adjacent before the wrapped image, it does not get moved ahead. And that all sounds very similar to the issue with display strings that is already treated in Fvertical_motion. This happens via this combo: reseat_at_previous_visible_line_start (&it); .... move_it_to (&it, PT, -11, -1, -1, MOVE_TO_POS); which I gather is primarily for getting the correct x position. But your point raises the very good question of why this doesn't happen on normal continuation lines. What's special about word wrap here? >> 2. How can you tell if the position prior to the IMAGE/STRETCH is on a prior visual line? > > By looking at the Y coordinate (it.current_t or it.vpos), I'd say. But as you mentioned for horizontal positioning, it doesn't get updated with a valid value until you've performed the move. So you'd have to do two moves, one before, one "on" the image/glpyh. There's probably a better way. >> Later in `Fvertical_motion' there is this test, which looks very promising: >> >> else if (IT_CHARPOS (it) == PT - 1 >> && FETCH_BYTE (PT_BYTE - 1) == '\n' >> && nlines <= 0) > > This only works if the previous line ended in a newline, but will not > work with continuation lines. Of course. My point was that the commentary is highly suggestive of the very thing that went wrong: /* The position we started from was covered by a display property, so we moved to position before the string, and backed up one line, because the character at PT - 1 is a newline. So we need one less line to go up (or exactly one line to go down if nlines == 0). */ So we'd just need another test for "PT-1 fell back to the prior screen line" to go along with checking for a newline to fix the "counter-bug" induced by my original fix. But I agree we need a better understanding of the issue before devising a fix. >> 3. It seems what's relevant for wide glyphs is not only "Was there a newline behind you?", but "Was there a newline behind you, or was the position there at a lower vpos?". Is there a way to use vpos to make a test like this? > > I don't know yet, the answer should reveal itself if you step through > the code and see what happens there in this particular situation (but > not in other similar ones). I'll try stepping with the same configuration but without word wrapping and see what differs. > It's too early to devise solutions, because we don't understand the > root cause of the problem yet. At least I don't. Agreed. I have hints of it, but no fully understanding. Thanks for the good thoughts. I'll keep experimenting. If you have other thoughts of things to try, please do share.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.