bug#70093: *** SPAM *** bug#70093: Remove unnecessary `image-tests--map-equal'

[Joseph Turner via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 2698 bytes --]


David Ponce <da_vid@orange.fr> writes:

> On 30/03/2024 20:44, Joseph Turner wrote:
>> With the resolution of bug#69992, it may be possible to remove the
>> `image-tests--map-equal' equality predicate from lisp/image-tests.el.
>> Eli and David, would you please apply the attached patch and then
>> run
>> the tests to see if we still need to allow for tolerances in
>> `image--compute-map-and-original-map'?
>> The tests pass for me, but then again, they never failed on my
>> machine...
>> Thank you!! I'm grateful for this fun collaboration. :)
>> Joseph
>>
>
> Hi Joseph,
>
> Did you forget to attach the patch? I cannot find it.

Apologies.  Here it is.  (This is the same patch as in my response to Eli)

> Anyway, I don't think the resolution of bug#69992 changed anything
> regarding the test `image--compute-map-and-original-map'.  And to
> be honest I don't understand why it gives a different result on our
> configurations.

I thought perhaps calculating scale with

(/ (float (car size))
                     (car (image-size
                           (image--image-without-parameters image) t)))

instead of

(or (image-property image :scale) 1)

would fix the variations on our machines.

> To test, I also eval the following simple code in *scratch* buffer,
> which gives me consistent results on my laptop:
>
> (let* ((svg (svg-create 200 100))
>        (omap `(((rect . ((0 . 0) . (50 . 50)))
>                 rect-test (help-echo "rect-test"))
>                ((circle . ((85 . 55) . 25))
>                 circle-test (help-echo "circle-test"))
>                ((poly . [100 0 125 0 170 50 125 50])
>                 poly-test (help-echo "poly-test")))))
>   (svg-rectangle svg 0 0 50 50
>                  :stroke-width 0 :fill "red")
>   (svg-circle    svg 85 55 25
>                  :stroke-width 0 :fill "green")
>   (svg-polyline  svg '((100 . 0) (125 . 0) (170 . 50) (125 . 50))
>                  :stroke-width 0 :fill "blue")
>   (let ((image (svg-image svg
>                           :original-map omap
>                           :scale nil    ;; vary scale test
>                           :rotation nil ;; vary rotation test
>                           :flip nil     ;; vary flip test
>                           )))
>     ;; Must see the pointer and help-echo change when mouse hovers the
>     ;; rect, circle and poly hot spots.
>     (insert-image image)
>     (insert ?\n)
>     ;; Must see the same original map.
>     (image--compute-original-map image)))

This manual test also works on my machine.  I had been doing something
similar; I like your use of svg-create :)

Let me know if the unit tests pass with this patch applied.

Thank you!

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Compare-image-maps-with-equal-in-tests.patch --]
[-- Type: text/x-diff, Size: 4386 bytes --]

From e85fd88a2943fa95d2d7eea3a542308adaa81582 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Sat, 30 Mar 2024 12:38:52 -0700
Subject: [PATCH] Compare image maps with `equal' in tests

With the fixes to image map transformation introduced in cc212ea314d,
the tests should now exactly predict calculated image maps.

* test/lisp/image-tests.el (image-tests--map-equal): Remove function.
(image--compute-map-and-original-map): Use equal to compare image maps.
---
 test/lisp/image-tests.el | 46 +++++-----------------------------------
 1 file changed, 5 insertions(+), 41 deletions(-)

diff --git a/test/lisp/image-tests.el b/test/lisp/image-tests.el
index 020781eff50..41b87b8d337 100644
--- a/test/lisp/image-tests.el
+++ b/test/lisp/image-tests.el
@@ -178,37 +178,6 @@ image-create-image-with-map
            (got-original-map (image-property image :original-map)))
       (should (equal got-original-map original-map-other)))))
 
-(defun image-tests--map-equal (a b &optional tolerance)
-  "Return t if maps A and B have the same coordinates within TOLERANCE.
-Since image sizes calculations vary on different machines, this function
-allows for each image map coordinate in A to be within TOLERANCE to the
-corresponding coordinate in B.  When nil, TOLERANCE defaults to 5."
-  (unless tolerance (setq tolerance 5))
-  (catch 'different
-    (cl-labels ((check-tolerance
-                  (coord-a coord-b)
-                  (unless (>= tolerance (abs (- coord-a coord-b)))
-                    (throw 'different nil))))
-      (dotimes (i (length a))
-        (pcase-let ((`((,type-a . ,coords-a) ,_id ,_plist) (nth i a))
-                    (`((,type-b . ,coords-b) ,_id ,_plist) (nth i b)))
-          (unless (eq type-a type-b)
-            (throw 'different nil))
-          (pcase-exhaustive type-a
-            ('rect
-             (check-tolerance (caar coords-a) (caar coords-b))
-             (check-tolerance (cdar coords-a) (cdar coords-b))
-             (check-tolerance (cadr coords-a) (cadr coords-b))
-             (check-tolerance (cddr coords-a) (cddr coords-b)))
-            ('circle
-             (check-tolerance (caar coords-a) (caar coords-b))
-             (check-tolerance (cdar coords-a) (cdar coords-b))
-             (check-tolerance (cdar coords-a) (cdar coords-b)))
-            ('poly
-             (dotimes (i (length coords-a))
-               (check-tolerance (aref coords-a i) (aref coords-b i))))))))
-    t))
-
 (ert-deftest image--compute-map-and-original-map ()
   "Test `image--compute-map' and `image--compute-original-map'."
   (skip-unless (display-images-p))
@@ -237,25 +206,20 @@ image--compute-map-and-original-map
                               :scale 2 :rotation 90 :flip t)))
     ;; Test that `image--compute-original-map' correctly generates
     ;; original-map when creating an already transformed image.
-    (should (image-tests--map-equal (image-property image :original-map)
-                                    original-map))
+    (should (equal (image-property image :original-map) original-map))
     (setf (image-property image :flip) nil)
     (setf (image-property image :rotation) 0)
     (setf (image-property image :scale) 2)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    scaled-map))
+    (should (equal (image--compute-map image) scaled-map))
     (setf (image-property image :scale) 1)
     (setf (image-property image :rotation) 90)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    rotated-map))
+    (should (equal (image--compute-map image) rotated-map))
     (setf (image-property image :rotation) 0)
     (setf (image-property image :flip) t)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    flipped-map))
+    (should (equal (image--compute-map image) flipped-map))
     (setf (image-property image :scale) 2)
     (setf (image-property image :rotation) 90)
-    (should (image-tests--map-equal (image--compute-map image)
-                                    scaled-rotated-flipped-map))
+    (should (equal (image--compute-map image) scaled-rotated-flipped-map))
 
     ;; Uncomment to test manually by interactively transforming the
     ;; image and checking the map boundaries by hovering them.
-- 
2.41.0