Subject: bug#76107: Consider image specs when scanning for a column

From: Eli Zaretskii <[email-masked]>
Date: Sun, 09 Feb 2025 09:08:37 +0200

> From: Thuna <[email-masked]>
> Cc: [email-masked]
> Date: Sat, 08 Feb 2025 23:17:02 +0100
> 
> > I understand the reason now, but my opinion is still the same: proper
> > support for columns requires simulating the display, 
> 
> I would _really_ appreciate it if you would point me to the problem
> point where calculating the image sizes (not the visible section - the
> _whole_ thing) becomes infeasible without access to the full display (or
> a simulation thereof).

Look at the way images are processed in xdisp.c (starting at
handle_single_display_spec and then in produce_image_glyph and its
subroutines).  It isn't a single place, and the processing is complex.
Because of this complex processing spread over several functions, I
have hard time believing that just calling image-size will produce a
reliable result, and I don't see how will you prove it does produce a
reliable result without a lot of testing, unless you use the same code
as the display engine does.  Which is why I suggested to use other
functions, which do use the display code for this purpose.

> > and the two functions I mentioned already do that.
> 
> I am not very familiar with events, but I don't see any way to do what
> `move-to-column' does using any of the things you mentioned.

The call

  (vertical-motion '(COLUMN . 0))

will move point to the specified COLUMN in the current screen line.
If you don't have to handle continuation lines, this is all you need;
otherwise, you need some more code using display--line-is-continued-p,
because vertical-motion moves only within the current screen line, and
will stop at line continuation.

Similarly, the call

  (car (posn-col-row (posn-at-point POS)))

will return the visual column of the buffer position POS.  (If this is
on a continuation line, you need to add the columns of the previous
continued lines.)

Both of these functions use display code to calculate the column, so
they take images into account.

I'm sorry, but please look at this from my POV: I don't want us to be
left with a semi-working implementation that we will need to keep
fixing and maintaining for the years to come.  Adding support for
images to move-to-column means we promise it to work correctly and
will be expected to fix any inaccuracy people report.  Using the
display code frees us from this problem, because that's what Emacs
uses to display the images in the first place, so it will always be as
correct as what we show on display.

So from my POV, either using posn-col-row and vertical-motion in a
specialized fill-paragraph-function, or using move_it_* functions in
check_display_width for measuring the column size of an image, are the
ways of making this improvement that I very much prefer.  The latter
is I think a simpler and better solution, because it doesn't have the
complications with continuation lines.  I don't understand why you
insist so much on going with Fimage_width and not with the method I
proposed, I don't think the code will be much more complex.

> > So if we want fill-paragraph to work in the presence of images and
> > other display features, we need to have a specialized
> > fill-paragraph-function which uses posn-col-row instead of
> > current-column and vertical-motion instead of move-to-column.
> 
> If we can have a version of fill-paragraph which can calculate image
> widths, then that should just be _the_ fill-paragraph function.

Images are not the only display complication that current-column and
move-to-column doesn't handle.  For example, they don't support
xwidgets, and also produce incorrect results when there are display
strings with embedded newlines.  Because of that, fill-paragraph
doesn't work properly in the presence of these display features.
Therefore, adding images to it will still not make it work correctly
in all the cases.

> > I don't see a good reason for adding semi-working image support to
> > current-column and move-to-column.
> 
> Yes, I also don't want to have a semi-working result, but what part is
> not working, other than `slice' support?
> 
> As far as I can see, in xdisp.c, `calc_pixel_width_or_height' is what
> determines the display width of the image, which returns the basically
> same thing Fimage_width does.  If there is some other step that I'm
> missing, could you _please_ tell me where or what it is?

See the two functions mentioned above, handle_single_display_spec and
produce_image_glyph.  If you use the move_it_* functions, they call
these automatically.