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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 15 Feb 2025 10:34:53 +0200

> Cc: [email-masked]
> Date: Fri, 14 Feb 2025 09:54:48 +0200
> From: Eli Zaretskii <[email-masked]>
> 
> > From: Thuna <[email-masked]>
> > Cc: [email-masked]
> > Date: Thu, 13 Feb 2025 22:45:54 +0100
> > 
> > > Please tell me if you are or will be working on this, or you are
> > > waiting for me to do it?  I'd like to avoid duplicate work.
> > I am trying stuff, but the code looks roughly like what I sent before.
> > It would probably be faster for you to do it.
> 
> OK, I will work on this.

The proposed patch is below.

You were right: the idea of using produce_image_glyph was not a good
one, as the code would be too tedious.  So I've decided to go with
higher-level functions instead, as you see below.

I did very little testing of the patch, so please test is as well as
you can, both with images and slices.  The problem here is that image
and slice specs can have different forms, so the code in
check_display_width should detect at least the important and popular
ones, and I'm not sure what's below is sufficient.

If you have any trouble with the patched Emacs, like crashes or
incorrect results from current-column or move-to-column, please tell
and show a recipe for reproducing the problems.

Thanks.

diff --git a/src/indent.c b/src/indent.c
index 01cea22..c90d2f0 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -466,13 +466,21 @@ current_column (void)
 }
 
 
+static void restore_window_buffer (Lisp_Object);
+
 /* Check the presence of a display property and compute its width.
+   POS and POS_BYTE are the character and byte positions of the
+   display property and COL is the column number at POS.
    If a property was found and its width was found as well, return
    its width (>= 0) and set the position of the end of the property
    in ENDPOS.
-   Otherwise just return -1.  */
+   Otherwise just return -1.
+   WINDOW is the window to use when it is important; nil stands for
+   the selected window.  */
 static int
-check_display_width (ptrdiff_t pos, ptrdiff_t col, ptrdiff_t *endpos)
+check_display_width (Lisp_Object window,
+		     ptrdiff_t pos, ptrdiff_t pos_byte, ptrdiff_t col,
+		     ptrdiff_t *endpos)
 {
   Lisp_Object val, overlay;
 
@@ -482,30 +490,76 @@ check_display_width (ptrdiff_t pos, ptrdiff_t col, ptrdiff_t *endpos)
       int width = -1;
       Lisp_Object plist = Qnil;
 
-      /* Handle '(space ...)' display specs.  */
-      if (CONSP (val) && EQ (Qspace, XCAR (val)))
-	{ /* FIXME: Use calc_pixel_width_or_height.  */
-	  Lisp_Object prop;
-	  EMACS_INT align_to_max =
-	    (col < MOST_POSITIVE_FIXNUM - INT_MAX
-	     ? (EMACS_INT) INT_MAX + col
-	     : MOST_POSITIVE_FIXNUM);
-
-	  plist = XCDR (val);
-	  if ((prop = plist_get (plist, QCwidth),
-	       RANGED_FIXNUMP (0, prop, INT_MAX))
-	      || (prop = plist_get (plist, QCrelative_width),
-		  RANGED_FIXNUMP (0, prop, INT_MAX)))
-	    width = XFIXNUM (prop);
-	  else if (FLOATP (prop) && 0 <= XFLOAT_DATA (prop)
-		   && XFLOAT_DATA (prop) <= INT_MAX)
-	    width = (int)(XFLOAT_DATA (prop) + 0.5);
-	  else if ((prop = plist_get (plist, QCalign_to),
-		    RANGED_FIXNUMP (col, prop, align_to_max)))
-	    width = XFIXNUM (prop) - col;
-	  else if (FLOATP (prop) && col <= XFLOAT_DATA (prop)
-		   && (XFLOAT_DATA (prop) <= align_to_max))
-	    width = (int)(XFLOAT_DATA (prop) + 0.5) - col;
+      if (CONSP (val))
+	{
+	  Lisp_Object xcar = XCAR (val);
+
+	  /* Handle '(space ...)' display specs.  */
+	  if (EQ (Qspace, xcar))
+	    { /* FIXME: Use calc_pixel_width_or_height.  */
+	      Lisp_Object prop;
+	      EMACS_INT align_to_max =
+		(col < MOST_POSITIVE_FIXNUM - INT_MAX
+		 ? (EMACS_INT) INT_MAX + col
+		 : MOST_POSITIVE_FIXNUM);
+
+	      plist = XCDR (val);
+	      if ((prop = plist_get (plist, QCwidth),
+		   RANGED_FIXNUMP (0, prop, INT_MAX))
+		  || (prop = plist_get (plist, QCrelative_width),
+		      RANGED_FIXNUMP (0, prop, INT_MAX)))
+		width = XFIXNUM (prop);
+	      else if (FLOATP (prop) && 0 <= XFLOAT_DATA (prop)
+		       && XFLOAT_DATA (prop) <= INT_MAX)
+		width = (int)(XFLOAT_DATA (prop) + 0.5);
+	      else if ((prop = plist_get (plist, QCalign_to),
+			RANGED_FIXNUMP (col, prop, align_to_max)))
+		width = XFIXNUM (prop) - col;
+	      else if (FLOATP (prop) && col <= XFLOAT_DATA (prop)
+		       && (XFLOAT_DATA (prop) <= align_to_max))
+		width = (int)(XFLOAT_DATA (prop) + 0.5) - col;
+	    }
+	    /* Handle images.  */
+	  else if (EQ (Qimage, xcar)
+		   || EQ (Qslice, xcar)
+		   /* 'insert-sliced-image' creates property of the form
+                      ((slice ...) image ...) */
+		   || (CONSP (xcar) && EQ (Qslice, XCAR (xcar))))
+	    {
+	      specpdl_ref count = SPECPDL_INDEX ();
+	      struct window *w = decode_live_window (window);
+
+	      /* If needed, set the window's buffer temporarily to the
+                 current buffer and its window-point to POS.  */
+	      if (XBUFFER (w->contents) != current_buffer)
+		{
+		  Lisp_Object oldbuf
+		    = list4 (window, w->contents,
+			     make_fixnum (marker_position (w->pointm)),
+			     make_fixnum (marker_byte_position (w->pointm)));
+		  record_unwind_protect (restore_window_buffer, oldbuf);
+		  wset_buffer (w, Fcurrent_buffer ());
+		  set_marker_both (w->pointm, w->contents, pos, pos_byte);
+		}
+
+	      struct text_pos startpos;
+	      struct it it;
+	      SET_TEXT_POS (startpos, pos, pos_byte);
+	      void *itdata = bidi_shelve_cache ();
+	      record_unwind_protect_void (unwind_display_working_on_window);
+	      display_working_on_window_p = true;
+	      start_display (&it, w, startpos);
+	      it.last_visible_x = 1000000; /* prevent image clipping */
+	      /* The POS+1 value is a trick: move_it_in_display_line
+                 will not return until it finished processing the entire
+                 image, even if it covers more than one buffer position.  */
+	      move_it_in_display_line (&it, pos + 1, -1, MOVE_TO_POS);
+	      /* The caller wants the width in units of the frame's
+                 canonical character width.  */
+	      width = ((double)it.current_x / FRAME_COLUMN_WIDTH (it.f)) + 0.5;
+	      bidi_unshelve_cache (itdata, 0);
+	      unbind_to (count, Qnil);
+	    }
 	}
       /* Handle 'display' strings.   */
       else if (STRINGP (val))
@@ -652,7 +706,7 @@ scan_for_column (ptrdiff_t *endpos, EMACS_INT *goalcol,
 
       { /* Check display property.  */
 	ptrdiff_t endp;
-	int width = check_display_width (scan, col, &endp);
+	int width = check_display_width (window, scan, scan_byte, col, &endp);
 	if (width >= 0)
 	  {
 	    col += width;