Subject: Re: bug#67604: Motion problems with inline images

From: JD Smith <[email-masked]>
Date: Thu, 29 May 2025 09:56:55 -0400

> On May 29, 2025, at 3:17 AM, Eli Zaretskii <[email-masked]> wrote:
> 
>> 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.
> 
> We never return MOVE_POS_MATCH_OR_ZV unless we are iterating on buffer
> text, yes.  That's because display properties and other similar
> features can "cover" any number of buffer-text characters, and we
> cannot know where it ends until we are done moving over it in its
> entirety.  

Makes sense (though see below).

> Maybe in your case the image only covered one buffer
> position, but that is not always the case.  

My image does cover 30 or so buffer positions.

> Or maybe you thought that
> we return MOVE_POS_MATCH_OR_ZV when we are _at_ the position, whereas
> in fact we return that when we got past the position (look at the
> conditions in BUFFER_POS_REACHED_P that compare buffer positions).

Ahh yes, I see that, hiding under the bidi_p branch of that test.  So we return when IT >= TO_CHARPOS.  When re-seating "to" the buffer position at the image where you started via start_display, it is too late to return _past_ the image; that's what causes this pixel overflow and line skip.

Question: if you are simply "moving back to the current position", as start_display does (so as to correctly calculate the x pixel position), why does it matter whether the image covers 1 or more buffer positions?  E.g., my green box covers pos=89-125 in my test setup.  If I'm moving "back" to pos=89 from BOL, should reaching the position at the precise start of the image span not constitute BUFFER_POS_REACHED_P?  

In other words, it seems we don't need to know where an image or stretch _ends_ to know that we've reached its _beginning_.  I.e. could that portion of the test be crafted as (see alt2 patch, attached):

   && (it->method == GET_FROM_BUFFER                            \
       || ((it->method == GET_FROM_IMAGE			\
	    || it->method == GET_FROM_STRETCH)			\
	   && IT_CHARPOS (*it) == to_charpos)                   \

??

> So in this case, to return MOVE_POS_MATCH_OR_ZV, you need to move past
> the image to the newline.  If that's what you did, and it->method was
> still GET_FROM_IMAGE, please see why we don't update it->method,
> because that is unexpected, I think.

In the case of a newline inserted right after a "just fitting on the line" green image (testing newline overflow into the fringe), this is indeed the current default scenario: we move past the image to the newline.  But, with my first patch, since it avoids the "just fits" branch, the one whose body starts:

		      ++it->hpos;
		      it->current_x = new_x;

		      /* The character's last glyph just barely fits
			 in this row.  */
		      if (i == it->nglyphs - 1)

we do not get a chance to notice ITERATOR_AT_END_OF_LINE_P and return early with MOVE_POS_MATCH_OR_ZV, instead returning MOVE_LINE_CONTINUED a bit further down, here:

		  move_trace ("move_it_in: continued at %td\n",
			      IT_CHARPOS (*it));
		  result = MOVE_LINE_CONTINUED;
		  break;

This would also be true for a normal (non-WS) just-fitting character followed by a newline.  Again, this could be a distinction without a difference, but I don't know what depends on the precise return value of move_it_in_display_line_to.