GNU bug report logs - #76107
Consider image specs when scanning for a column

Previous Next

Full log

View this message in rfc822 format

From: Thuna <thuna.cing <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76107 <at> debbugs.gnu.org
Subject: bug#76107: Consider image specs when scanning for a column
Date: Mon, 10 Feb 2025 16:18:34 +0100

[Message part 1 (text/plain, inline)]

> 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.

I've attached the code for handle_single_display_spec and
produce_image_glyph, with (what I found to be) irrelevant code elided
and with my comments of what's happening where.  If there's nothing else
that's happening between these two steps (and I don't think there is - I
at least can't find any), then the display width (of unsliced images) is
exactly what Fimage_size returns, with the exception of width added due
to `box' face attributes.  The calculation is simple even when the image
is sliced:

  width = min(<slice width>, <image width> - <slice x>)
          [+ <image hmargin>, if slice is leftmost]
          [+ <image hmargin>, if slice is rightmost]

It is still meaningful to not duplicate the calculations being done, but
I don't think that the way to go is for the width calculators to use the
display code but for the display code to use the width calculators.  The
important question for me is this: Does the _actual_ width of a glyph
depend on where it is located in the display?  If not (and I imagine
not), then we should not need to simulate the display.

> 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. [...] 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.

I understand where you're coming from, I also do not want to leave
behind an unmaintainable mess.  But I do not know xdisp.c enough to
actually be able to make use of it, and I cannot imagine that anyone
else has the motivation.

Still, here's my best shot.  I have attached a version of
`scan_for_column' which uses `move_it_in_display_line', which I've
written with my very limited understanding of how this is all supposed
to work.  I can't even test it, since I don't know how I would go about
recovering all the `prev*' values, but is it at least _somewhat_ in the
right direction?

> 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.
>
>> > 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 think you misunderstood me; what I mean here is that if there is a
"specialized" fill-paragraph that handles display features, then that
should be _the_ fill-paragraph, and not be its own separate thing.

[commented xdisp.c code.c (text/plain, attachment)]

[scan_for_column alternative.c (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.