bug#67533: SVG images confound position pixel measurements

[JD Smith]

Progress.  The latest patch had two small hunks fail (see below), but they were 
simple enough that I was able to insert them by hand and recompile.

The good news is that this patch does on my end fix all the problems with the 
generated SVGs buffer from my recent test, at all window widths.  
Unfortunately, checking my original file where this issue was revealed, there 
are still quite a few “false zero height above” reports.  Happily in that file 
there are no longer any of the “overly large height” variety, so this 
particular issue has apparently has been fixed by your patch.

I’ve placed a new test that generates a minimal buffer with a single 
:file-based SVG overlay at the same gist, and copied/attached them below, for 
posterity.  I’ve also fixed the bug in my/check-buffer-pixel-values which 
caused the report buffer not to appear (due to end-of-buffer being signaled), 
so please update that.  And please note the new svg_test.svg file, to be placed 
in the same directory.

I don’t think there should be anything special about this particular SVG file; 
it was automatically generated from the underlying latex fragment.  But it 
seems to be confusing the display engine somehow (as do many others).

One thing to note about these SVG files is that they have transparent 
backgrounds, but I don’t think that’s anything unusual.

++++++++
New test:
++++++++

(let ((buf "svg-file-pixel-zero-demo"))
  (with-current-buffer (get-buffer-create buf)
    (erase-buffer)
    (visual-line-mode 1)
    (insert "\nProin quam nisl, tincidunt et, mattis eget, convallis nec, 
purus.  $\\chi(y) = \\exp\\left(\\sqrt{\\tan(y)}\\right)$  Nullam tempus.")
    (goto-char (point-min))
    (when  (re-search-forward (rx ?$ (* (not ?$)) ?$))
      (let ((ov (make-overlay (match-beginning 0) (match-end 0))))
(overlay-put ov 'display
    `(image :type svg
    :file ,(expand-file-name "svg_test.svg")
    :ascent center)))))
  (pop-to-buffer buf)
  (my/check-buffer-pixel-values))

++++++++++++++
Buffer check code
++++++++++++++

(eval-when-compile (require 'cl-lib))

(defun my/check-buffer-pixel-values ()
  (interactive)
  (goto-char (point-min))
  (let ((line-heights (vconcat (save-excursion
(cl-loop while (< (point) (point-max))
 collect (line-pixel-height)
 do (vertical-motion 1)))))
(line 0)
vmax)
    (with-output-to-temp-buffer "svg-pixel-demo-report"
      (princ (format "SVG Position analysis for %s (width %d, %s)\n\n"
    (current-buffer) (window-width)
    (if (next-single-property-change (point-min) 'display)
"text-properties" "overlays")))
      (while (not (eobp))
(beginning-of-visual-line)
(vertical-motion 1)
(setq vmax (save-excursion (end-of-visual-line) (point)))
(save-excursion
 (while (and (<= (point) vmax) (not (eobp)))
   (let* ((ps (window-text-pixel-size nil (cons (point) -1)
      (point) nil nil nil t))
  (h (nth 1 ps)))
     (unless (= h (aref line-heights line))
(princ
(format "Incorrect at point=%3d: line %2d at %12S (%5s): expected %2d got %2d\n"
(point) (+ line 2) (posn-x-y (posn-at-point))
(if (or (overlays-at (point))
(get-text-property (point) 'display))
    "image"
  (char-to-string (char-after (point))))
(aref line-heights line) h))))
   (forward-char)))
(cl-incf line)))))

++++++++++
Failed hunks:
++++++++++

@ -11436,7 +11436,7 @@
      /* Start at the beginning of the line containing FROM.  Otherwise
    IT.current_x will be incorrectly set to zero at some arbitrary
    non-zero X coordinate.  */
-      reseat_at_previous_visible_line_start (&it);
+      move_it_by_lines (&it, 0);
      it.current_x = it.hpos = 0;
      if (IT_CHARPOS (it) != start)
   {
@@ -11513,6 +11513,8 @@
     the width of the last buffer position manually.  */
  if (IT_CHARPOS (it) > end)
    {
+      int end_y = it.current_y;
+
      end--;
      RESTORE_IT (&it, &it2, it2data);
      x = move_it_to (&it, end, to_x, max_y, -1, move_op);

> On Dec 2, 2023, at 4:44 PM, JD Smith <jdtsmith@gmail.com> wrote:
> 
> Thanks for your work.   The port I use has fallen a few weeks behind master 
> and the patches to xdisp.c unfortunately don’t apply cleanly.  I’ll see about 
> building an NS port and report back. 
> 
>> On Dec 2, 2023, at 2:39 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>>> Cc: 67533@debbugs.gnu.org
>>> Date: Sat, 02 Dec 2023 16:18:28 +0200
>>> From: Eli Zaretskii <eliz@gnu.org>
>>> 
>>>> From: JD Smith <jdtsmith@gmail.com>
>>>> Date: Sat, 2 Dec 2023 08:36:07 -0500
>>>> Cc: 67533@debbugs.gnu.org
>>>> 
>>>> Strange.  Both of my ports here (NS and Mac) show the same issue.  Did you 
>>>> check the
>>>> svg-pixel-demo-report buffer the header mentions, where the mismatch 
>>>> measurement information
>>>> gets reported?  The specific problems encountered are listed in the report 
>>>> buffer, updated with
>>>> my/check-buffer-pixel-values.  For example, at a standard width of 80 
>>>> characters, I get this report:
>>>> 
>>>> SVG Position analysis for svg-pixel-demo (width 80, text-properties)
>>>> 
>>>> Incorrect at point= 34: line  2 at   (224 . 14) (image): expected 14 got 28
>>>> Incorrect at point= 99: line  3 at   (244 . 43) (image): expected 29 got 46
>>>> Incorrect at point=126: line  4 at    (42 . 75) (image): expected 32 got 46
>>>> Incorrect at point=162: line  4 at   (345 . 75) (image): expected 32 got 52
>>>> Incorrect at point=210: line  5 at  (133 . 110) (image): expected 35 got 49
>>> 
>>> I never understood I need to look in another buffer.  I will look into
>>> these.
>> 
>> Please try the patch below and see if any problems remain.
>> 
>> diff --git a/src/xdisp.c b/src/xdisp.c
>> index 0b2508c..cc95ab3 100644
>> --- a/src/xdisp.c
>> +++ b/src/xdisp.c
>> @@ -11436,7 +11436,7 @@ window_text_pixel_size (Lisp_Object window, 
>> Lisp_Object from, Lisp_Object to,
>>      /* Start at the beginning of the line containing FROM.  Otherwise
>> IT.current_x will be incorrectly set to zero at some arbitrary
>> non-zero X coordinate.  */
>> -      reseat_at_previous_visible_line_start (&it);
>> +      move_it_by_lines (&it, 0);
>>      it.current_x = it.hpos = 0;
>>      if (IT_CHARPOS (it) != start)
>> {
>> @@ -11513,6 +11513,8 @@ window_text_pixel_size (Lisp_Object window, 
>> Lisp_Object from, Lisp_Object to,
>>     the width of the last buffer position manually.  */
>>  if (IT_CHARPOS (it) > end)
>>    {
>> +      int end_y = it.current_y;
>> +
>>      end--;
>>      RESTORE_IT (&it, &it2, it2data);
>>      x = move_it_to (&it, end, to_x, max_y, -1, move_op);
>> @@ -11525,8 +11527,13 @@ window_text_pixel_size (Lisp_Object window, 
>> Lisp_Object from, Lisp_Object to,
>> 
>>  /* DTRT if ignore_line_at_end is t.  */
>>  if (!NILP (ignore_line_at_end))
>> -    doff = (max (it.max_ascent, it.ascent)
>> -    + max (it.max_descent, it.descent));
>> +    {
>> +      /* If END-1 is on the previous screen line, we need to
>> +                 account for the vertical dimensions of previous line.  */
>> +      if (it.current_y < end_y)
>> + doff = (max (it.max_ascent, it.ascent)
>> + + max (it.max_descent, it.descent));
>> +    }
>>  else
>>    {
>>      it.max_ascent = max (it.max_ascent, it.ascent);
>