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

From: Eli Zaretskii <[email-masked]>
Date: Mon, 10 Feb 2025 18:42:10 +0200

> From: Thuna <[email-masked]>
> Cc: [email-masked]
> Date: Mon, 10 Feb 2025 16:18:34 +0100
> 
> 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.

Ok, so you have already discovered that the box attribute can change
the dimensions.  My point was exactly that: using the display code
will make sure we don't risk such omissions.

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

It is in the right direction, yes.  But I see now that we can make it
even simpler: we don't need move_it_in_display_line, we only need to
call produce_image_glyph (after setting up the 'struct it' object as
produce_image_glyph expects).  Then the width of the image on display
will be in it->pixel_width, and all that's left is convert that to
columns.

Then we just need to call this new code from check_display_width when
we find an image display property.  The rest of the code in
scan_for_column is okay and doesn't need to be replaced with move_it_*
functions.

So if you can implement this idea, feel free to show the resulting
patch.  Alternatively, wait for a few days until I find enough free
time to sit down and code this myself.

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

Agreed, but only if the resulting fill-paragraph will not be
significantly slower.  And it could be significantly slower, since the
move_it_* functions are slower than just walking the buffer text one
character at a time.  If using the "specialized" fill-paragraph
becomes significantly slower due to support of images, it would make
sense not to use that method if the buffer is known to be devoid of
images.