GNU bug report logs - #69992
Minor improvement to image map transformation logic

Previous Next

Package: emacs;

Reported by: Joseph Turner <joseph <at> ushin.org>

Date: Mon, 25 Mar 2024 01:03:01 UTC

Severity: normal

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69992 in the body.
You can then email your comments to 69992 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Mon, 25 Mar 2024 01:03:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Joseph Turner <joseph <at> ushin.org>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 25 Mar 2024 01:03:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> ushin.org>
To: Emacs Bugs Mailing List <bug-gnu-emacs <at> gnu.org>
Subject: Minor improvement to image map transformation logic
Date: Sun, 24 Mar 2024 18:00:52 -0700
[Message part 1 (text/plain, inline)]
Hello,

This patch is slight simplification/optimization.

Joseph

[0001-copy-tree-just-image-map-not-entire-image.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Wed, 27 Mar 2024 11:17:02 GMT) Full text and rfc822 format available.

Message #8 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: 69992 <at> debbugs.gnu.org
Subject: Re: Minor improvement to image map transformation logic
Date: Wed, 27 Mar 2024 12:16:11 +0100
[Message part 1 (text/plain, inline)]
Many thanks for this feature, which is particularly useful to
automatically recalculate the map of computed images like SVG.

To make the code faster, by avoiding multiple scans of the map for
copy and parsing, I propose the following patch which factors most of
the code into the functions `image--compute-map' and `image--compute
-original-map'. I have done some tests on my side which are
conclusive.

Furthermore, I wonder if the term :base-map would not be more
descriptive than :original-map?

Thanks again for this valuable feature!
[image.el.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Wed, 27 Mar 2024 12:52:02 GMT) Full text and rfc822 format available.

Message #11 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: David Ponce <da_vid <at> orange.fr>, Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 69992 <at> debbugs.gnu.org
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Wed, 27 Mar 2024 14:50:48 +0200
> Date: Wed, 27 Mar 2024 12:16:11 +0100
> From:  David Ponce via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Many thanks for this feature, which is particularly useful to
> automatically recalculate the map of computed images like SVG.
> 
> To make the code faster, by avoiding multiple scans of the map for
> copy and parsing, I propose the following patch which factors most of
> the code into the functions `image--compute-map' and `image--compute
> -original-map'. I have done some tests on my side which are
> conclusive.
> 
> Furthermore, I wonder if the term :base-map would not be more
> descriptive than :original-map?

Thanks.

Joseph, any comments or suggestions?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Wed, 27 Mar 2024 14:22:01 GMT) Full text and rfc822 format available.

Message #14 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Eli Zaretskii <eliz <at> gnu.org>, Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 69992 <at> debbugs.gnu.org
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Wed, 27 Mar 2024 15:21:18 +0100
[Message part 1 (text/plain, inline)]
On 27/03/2024 13:50, Eli Zaretskii wrote:
>> Date: Wed, 27 Mar 2024 12:16:11 +0100
>> From:  David Ponce via "Bug reports for GNU Emacs,
>>   the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> Many thanks for this feature, which is particularly useful to
>> automatically recalculate the map of computed images like SVG.
>>
>> To make the code faster, by avoiding multiple scans of the map for
>> copy and parsing, I propose the following patch which factors most of
>> the code into the functions `image--compute-map' and `image--compute
>> -original-map'. I have done some tests on my side which are
>> conclusive.
>>
>> Furthermore, I wonder if the term :base-map would not be more
>> descriptive than :original-map?
> 
> Thanks.
> 
> Joseph, any comments or suggestions?

Attached the same patch slightly cleaned up.
[image.el-compute-map-V1.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Wed, 27 Mar 2024 22:51:01 GMT) Full text and rfc822 format available.

Message #17 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: David Ponce <da_vid <at> orange.fr>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 69992 <at> debbugs.gnu.org
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Wed, 27 Mar 2024 15:17:31 -0700
David Ponce <da_vid <at> orange.fr> writes:

> On 27/03/2024 13:50, Eli Zaretskii wrote:
>>> Date: Wed, 27 Mar 2024 12:16:11 +0100
>>> From:  David Ponce via "Bug reports for GNU Emacs,
>>>   the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>>
>>> Many thanks for this feature, which is particularly useful to
>>> automatically recalculate the map of computed images like SVG.

You're welcome!

>>> To make the code faster, by avoiding multiple scans of the map for
>>> copy and parsing, I propose the following patch which factors most of
>>> the code into the functions `image--compute-map' and `image--compute
>>> -original-map'. I have done some tests on my side which are
>>> conclusive.

Thanks for reviewing and optimizing this feature.  Please share the
tests/benchmarks that you've performed.

>>> Furthermore, I wonder if the term :base-map would not be more
>>> descriptive than :original-map?

I am fine with changing :original-map to :base-map.  If you want to do
this, I suggest making this change in its own commit which also updates
the relevant docstrings and manual pages.

>> Thanks.
>> Joseph, any comments or suggestions?

On my machine, not all tests pass with the patch.  Please be sure that
these three new tests pass:

image-create-image-with-map
image--compute-map-and-original-map
image-transform-map

Personally, I find it easier to understand image map transformation when
the logic is split into multiple functions.  However, the benefit of
readability could certainly be outweighed by a noticeable improvement to
user experience.  Please share some benchmarks.

Please keep in mind that `image--delayed-change-size' already debounces
image transformation, so this code may not be so performance-critical.

Thank you,

Joseph

> Attached the same patch slightly cleaned up.
>
> [2. text/x-patch; image.el-compute-map-V1.patch]...





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Wed, 27 Mar 2024 23:54:01 GMT) Full text and rfc822 format available.

Message #20 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 69992 <at> debbugs.gnu.org
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Thu, 28 Mar 2024 00:53:26 +0100
On 27/03/2024 23:17, Joseph Turner wrote:
> 
> David Ponce <da_vid <at> orange.fr> writes:
> 
>> On 27/03/2024 13:50, Eli Zaretskii wrote:
>>>> Date: Wed, 27 Mar 2024 12:16:11 +0100
>>>> From:  David Ponce via "Bug reports for GNU Emacs,
>>>>    the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>>>
>>>> Many thanks for this feature, which is particularly useful to
>>>> automatically recalculate the map of computed images like SVG.
> 
> You're welcome!
> 
>>>> To make the code faster, by avoiding multiple scans of the map for
>>>> copy and parsing, I propose the following patch which factors most of
>>>> the code into the functions `image--compute-map' and `image--compute
>>>> -original-map'. I have done some tests on my side which are
>>>> conclusive.
> 
> Thanks for reviewing and optimizing this feature.  Please share the
> tests/benchmarks that you've performed.

OK

>>>> Furthermore, I wonder if the term :base-map would not be more
>>>> descriptive than :original-map?
> 
> I am fine with changing :original-map to :base-map.  If you want to do
> this, I suggest making this change in its own commit which also updates
> the relevant docstrings and manual pages.

I was just wondering.  If everyone is happy with :original-map, I'm fine
with it.

>>> Thanks.
>>> Joseph, any comments or suggestions?
> 
> On my machine, not all tests pass with the patch.  Please be sure that
> these three new tests pass:
> 
> image-create-image-with-map
> image--compute-map-and-original-map
> image-transform-map

Maybe some tests didn't pass because with my patch the computed hot spots
are pushed in a new map in reverse order?
I will have a look at this as soon as possible.

> Personally, I find it easier to understand image map transformation when
> the logic is split into multiple functions.  However, the benefit of
> readability could certainly be outweighed by a noticeable improvement to
> user experience.  Please share some benchmarks.

In this case, I have the opposite feeling ;-)
I find harder to read the logic splits into multiple functions that operate
by side effect on hot spots coords.  But it could be just me :-)

> Please keep in mind that `image--delayed-change-size' already debounces
> image transformation, so this code may not be so performance-critical.

Related to `image--delayed-change-size', you are probably right.
My concern is more about computed images and associated maps (I use such
kind of images+maps in computed SVG buttons grids).  In this case it could
be interesting to keep `create-image' as efficient as possible.

> Thank you,

You are welcome! Thank you for your feedback!

> 
> Joseph
> 
>> Attached the same patch slightly cleaned up.
>>
>> [2. text/x-patch; image.el-compute-map-V1.patch]...
> 





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Thu, 28 Mar 2024 22:23:01 GMT) Full text and rfc822 format available.

Message #23 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: 69992 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Thu, 28 Mar 2024 23:22:10 +0100
[Message part 1 (text/plain, inline)]
Re-sent to all (sorry)

On 27/03/2024 23:17, Joseph Turner wrote:
[...]
> On my machine, not all tests pass with the patch.  Please be sure that
> these three new tests pass:
> 
> image-create-image-with-map
> image--compute-map-and-original-map
> image-transform-map
> 
> Personally, I find it easier to understand image map transformation when
> the logic is split into multiple functions.  However, the benefit of
> readability could certainly be outweighed by a noticeable improvement to
> user experience.  Please share some benchmarks.
> 
> Please keep in mind that `image--delayed-change-size' already debounces
> image transformation, so this code may not be so performance-critical.
  
Hello,

After more work, testing and benchmarks, I can finally confirm that my
proposed version of `image--compute-*map' without the logic splits
into multiple functions is not significantly faster than the current
version with the logic splits into multiple functions :-)

What I found interesting after profiling both current and proposed
functions is that most of the time is consumed by the call to
`image-size'!

I also found that the current implementation is not correct when
rotation is not a multiple of 90 degrees.  In this case, Emacs still
scales the image if specified, but ignores rotation and flipping.
However, in this case, the `image--compute-*map' functions do not
recompute map.

The attached new patch fixes the logic to be consistent with Emacs
internal implementation, plus some other tweaks to check if a
transformation apply before to call the transformation function.
I also updated some tests according to functions changes.
Here is a possible change log:

2024-03-28  David Ponce  <da_vid <at> orange.fr>

	* lisp/image.el (image--compute-scaling)
	(image--compute-rotation): New functions.
	(image--compute-map, image--compute-original-map): Use them.
	Ensure all transformations are applied or undone according to what
	Emacs does internally.  Call a transformation function only when
	needed.  Fix doc string.
	(image--scale-map): Assume effective scale argument.
	(image--rotate-map): Assume effective rotation argument.
	(image--rotate-coord): Improve doc string.
	(image--flip-map): Remove no more used flip argument.

	* test/lisp/image-tests.el (image-create-image-with-map): Use a
	valid SVG image otherwise `image-size' will not return a valid
	value and calculation of scale could fail.
	(image-transform-map): Update according to changed signature of
	image--flip-map.

This new version passes the `image-create-image-with-map' and
`image-transform-map' tests.  But on my laptop, the
`image--compute-map-and-original-map' fails the same for both the
current and proposed version of the functions:

F image--compute-map-and-original-map
     Test ‘image--compute-map’ and ‘image--compute-original-map’.
     (ert-test-failed
      ((should
        (image-tests--map-equal (image--compute-map image) rotated-map))
       :form
       (image-tests--map-equal
        (((circle ... . 24) "a" (help-echo "A"))
         ((rect ... 127 . 77) "b" (help-echo "B"))
         ((poly . [199 161 206 160 213 154 218 146 221 136 ...]) "c"
          (help-echo "C")))
        (((circle ... . 24) "a" (help-echo "A"))
         ((rect ... 54 . 77) "b" (help-echo "B"))
         ((poly . [126 161 133 160 140 154 145 146 148 136 ...]) "c"
          (help-echo "C"))))
       :value nil))

Thanks!


[image-compute-map-V0.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Fri, 29 Mar 2024 10:20:01 GMT) Full text and rfc822 format available.

Message #26 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: 69992 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Fri, 29 Mar 2024 11:19:11 +0100
[Message part 1 (text/plain, inline)]
[...]
> The attached new patch fixes the logic to be consistent with Emacs
> internal implementation, plus some other tweaks to check if a
> transformation apply before to call the transformation function.
> I also updated some tests according to functions changes.
> Here is a possible change log:
> 
> 2024-03-28  David Ponce  <da_vid <at> orange.fr>
> 
>      * lisp/image.el (image--compute-scaling)
>      (image--compute-rotation): New functions.
>      (image--compute-map, image--compute-original-map): Use them.
>      Ensure all transformations are applied or undone according to what
>      Emacs does internally.  Call a transformation function only when
>      needed.  Fix doc string.
>      (image--scale-map): Assume effective scale argument.
>      (image--rotate-map): Assume effective rotation argument.
>      (image--rotate-coord): Improve doc string.
>      (image--flip-map): Remove no more used flip argument.
> 
>      * test/lisp/image-tests.el (image-create-image-with-map): Use a
>      valid SVG image otherwise `image-size' will not return a valid
>      value and calculation of scale could fail.
>      (image-transform-map): Update according to changed signature of
>      image--flip-map.
[...]

Hello,

Please find attached a new patch with an additional small fix I forgot to
include.  Sorry.

Regards
[image-compute-map-V1.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Sat, 30 Mar 2024 08:12:02 GMT) Full text and rfc822 format available.

Message #29 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: David Ponce <da_vid <at> orange.fr>
Cc: 69992 <at> debbugs.gnu.org, joseph <at> breatheoutbreathe.in
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Sat, 30 Mar 2024 11:10:50 +0300
> Date: Fri, 29 Mar 2024 11:19:11 +0100
> From: David Ponce <da_vid <at> orange.fr>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Joseph Turner <joseph <at> breatheoutbreathe.in>
> 
> > The attached new patch fixes the logic to be consistent with Emacs
> > internal implementation, plus some other tweaks to check if a
> > transformation apply before to call the transformation function.
> > I also updated some tests according to functions changes.
> > Here is a possible change log:
> > 
> > 2024-03-28  David Ponce  <da_vid <at> orange.fr>
> > 
> >      * lisp/image.el (image--compute-scaling)
> >      (image--compute-rotation): New functions.
> >      (image--compute-map, image--compute-original-map): Use them.
> >      Ensure all transformations are applied or undone according to what
> >      Emacs does internally.  Call a transformation function only when
> >      needed.  Fix doc string.
> >      (image--scale-map): Assume effective scale argument.
> >      (image--rotate-map): Assume effective rotation argument.
> >      (image--rotate-coord): Improve doc string.
> >      (image--flip-map): Remove no more used flip argument.
> > 
> >      * test/lisp/image-tests.el (image-create-image-with-map): Use a
> >      valid SVG image otherwise `image-size' will not return a valid
> >      value and calculation of scale could fail.
> >      (image-transform-map): Update according to changed signature of
> >      image--flip-map.
> [...]
> 
> Hello,
> 
> Please find attached a new patch with an additional small fix I forgot to
> include.  Sorry.

Thanks.  Please resend with the updated commit log message, and I will
install it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Sat, 30 Mar 2024 08:57:03 GMT) Full text and rfc822 format available.

Message #32 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69992 <at> debbugs.gnu.org, joseph <at> breatheoutbreathe.in
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Sat, 30 Mar 2024 09:55:59 +0100
On 30/03/2024 09:10, Eli Zaretskii wrote:
>> Date: Fri, 29 Mar 2024 11:19:11 +0100
>> From: David Ponce <da_vid <at> orange.fr>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Joseph Turner <joseph <at> breatheoutbreathe.in>
>>
>>> The attached new patch fixes the logic to be consistent with Emacs
>>> internal implementation, plus some other tweaks to check if a
>>> transformation apply before to call the transformation function.
>>> I also updated some tests according to functions changes.
>>> Here is a possible change log:
>>>
>>> 2024-03-28  David Ponce  <da_vid <at> orange.fr>
>>>
>>>       * lisp/image.el (image--compute-scaling)
>>>       (image--compute-rotation): New functions.
>>>       (image--compute-map, image--compute-original-map): Use them.
>>>       Ensure all transformations are applied or undone according to what
>>>       Emacs does internally.  Call a transformation function only when
>>>       needed.  Fix doc string.
>>>       (image--scale-map): Assume effective scale argument.
>>>       (image--rotate-map): Assume effective rotation argument.
>>>       (image--rotate-coord): Improve doc string.
>>>       (image--flip-map): Remove no more used flip argument.
>>>
>>>       * test/lisp/image-tests.el (image-create-image-with-map): Use a
>>>       valid SVG image otherwise `image-size' will not return a valid
>>>       value and calculation of scale could fail.
>>>       (image-transform-map): Update according to changed signature of
>>>       image--flip-map.
>> [...]
>>
>> Hello,
>>
>> Please find attached a new patch with an additional small fix I forgot to
>> include.  Sorry.
> 
> Thanks.  Please resend with the updated commit log message, and I will
> install it.

Hello Eli,

The change log is the same.  The last patch include a slightly modified
version of the new function `image--compute-rotation' to return 0 by default
when no rotation is specified, instead of nil.

Please let me know if you need anything else.
Thanks!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Sat, 30 Mar 2024 13:00:04 GMT) Full text and rfc822 format available.

Message #35 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: David Ponce <da_vid <at> orange.fr>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69992 <at> debbugs.gnu.org, joseph <at> breatheoutbreathe.in
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Sat, 30 Mar 2024 13:59:41 +0100
[Message part 1 (text/plain, inline)]
On 30/03/2024 09:55, David Ponce wrote:
> On 30/03/2024 09:10, Eli Zaretskii wrote:
>>> Date: Fri, 29 Mar 2024 11:19:11 +0100
>>> From: David Ponce <da_vid <at> orange.fr>
>>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Joseph Turner <joseph <at> breatheoutbreathe.in>
>>>
>>>> The attached new patch fixes the logic to be consistent with Emacs
>>>> internal implementation, plus some other tweaks to check if a
>>>> transformation apply before to call the transformation function.
>>>> I also updated some tests according to functions changes.
>>>> Here is a possible change log:
>>>>
>>>> 2024-03-28  David Ponce  <da_vid <at> orange.fr>
>>>>
>>>>       * lisp/image.el (image--compute-scaling)
>>>>       (image--compute-rotation): New functions.
>>>>       (image--compute-map, image--compute-original-map): Use them.
>>>>       Ensure all transformations are applied or undone according to what
>>>>       Emacs does internally.  Call a transformation function only when
>>>>       needed.  Fix doc string.
>>>>       (image--scale-map): Assume effective scale argument.
>>>>       (image--rotate-map): Assume effective rotation argument.
>>>>       (image--rotate-coord): Improve doc string.
>>>>       (image--flip-map): Remove no more used flip argument.
>>>>
>>>>       * test/lisp/image-tests.el (image-create-image-with-map): Use a
>>>>       valid SVG image otherwise `image-size' will not return a valid
>>>>       value and calculation of scale could fail.
>>>>       (image-transform-map): Update according to changed signature of
>>>>       image--flip-map.
>>> [...]
>>>
>>> Hello,
>>>
>>> Please find attached a new patch with an additional small fix I forgot to
>>> include.  Sorry.
>>
>> Thanks.  Please resend with the updated commit log message, and I will
>> install it.
> 
> Hello Eli,
> 
> The change log is the same.  The last patch include a slightly modified
> version of the new function `image--compute-rotation' to return 0 by default
> when no rotation is specified, instead of nil.
> 
> Please let me know if you need anything else.
> Thanks!

Hello,

Here is my last patch. The only change compared to the previous patch is that
now the scale factor is correctly calculated based on the size of the image and
the displayed size. To minimize the performance impact, I saved a call to
`image-size' by doing the calculation directly in the `image--compute-map' and
`image--compute-original-map' functions. I did some benchmarks and the
difference is not significant. The tests still give the same results :-)

Here is the new change log:

2024-03-30  David Ponce  <da_vid <at> orange.fr>

	* lisp/image.el (image--compute-rotation): New function.
	(image--compute-map, image--compute-original-map): Use it.
	Ensure all transformations are applied or undone according to what
	Emacs does internally.  Call a transformation function only when
	needed.  Fix doc string.
	(image--scale-map): Assume effective scale argument.
	(image--rotate-map): Assume effective rotation argument.
	(image--rotate-coord): Improve doc string.
	(image--flip-map): Remove no more used flip argument.

	* test/lisp/image-tests.el (image-create-image-with-map): Use a
	valid SVG image otherwise `image-size' will not return a valid
	value and calculation of scale could fail.
	(image-transform-map): Update according to changed signature of
	image--flip-map.

Thanks!
[image-compute-map-V2.patch (text/x-patch, attachment)]

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sat, 30 Mar 2024 13:38:04 GMT) Full text and rfc822 format available.

Notification sent to Joseph Turner <joseph <at> ushin.org>:
bug acknowledged by developer. (Sat, 30 Mar 2024 13:38:04 GMT) Full text and rfc822 format available.

Message #40 received at 69992-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: David Ponce <da_vid <at> orange.fr>
Cc: 69992-done <at> debbugs.gnu.org, joseph <at> breatheoutbreathe.in
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Sat, 30 Mar 2024 16:37:15 +0300
> Date: Sat, 30 Mar 2024 13:59:41 +0100
> From: David Ponce <da_vid <at> orange.fr>
> Cc: 69992 <at> debbugs.gnu.org, joseph <at> breatheoutbreathe.in
> 
> Here is my last patch. The only change compared to the previous patch is that
> now the scale factor is correctly calculated based on the size of the image and
> the displayed size. To minimize the performance impact, I saved a call to
> `image-size' by doing the calculation directly in the `image--compute-map' and
> `image--compute-original-map' functions. I did some benchmarks and the
> difference is not significant. The tests still give the same results :-)
> 
> Here is the new change log:

Thanks, installed on master, and closing the bug.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69992; Package emacs. (Sat, 30 Mar 2024 19:45:01 GMT) Full text and rfc822 format available.

Message #43 received at 69992 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: David Ponce <da_vid <at> orange.fr>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 69992 <at> debbugs.gnu.org
Subject: Re: bug#69992: Minor improvement to image map transformation logic
Date: Sat, 30 Mar 2024 12:07:05 -0700
David Ponce <da_vid <at> orange.fr> writes:

> Here is my last patch. The only change compared to the previous patch is that
> now the scale factor is correctly calculated based on the size of the image and
> the displayed size. To minimize the performance impact, I saved a call to
> `image-size' by doing the calculation directly in the `image--compute-map' and
> `image--compute-original-map' functions. I did some benchmarks and the
> difference is not significant. The tests still give the same results :-)

Thank you for these fixes and optimizations!!! The tests pass for me.

> Here is the new change log:
>
> 2024-03-30  David Ponce  <da_vid <at> orange.fr>
>
> 	* lisp/image.el (image--compute-rotation): New function.
> 	(image--compute-map, image--compute-original-map): Use it.
> 	Ensure all transformations are applied or undone according to what
> 	Emacs does internally.  Call a transformation function only when
> 	needed.  Fix doc string.

With this fix, I think we can remove `image-tests--map-equal'. I'll
submit a new bug.

> 	(image--scale-map): Assume effective scale argument.
> 	(image--rotate-map): Assume effective rotation argument.
> 	(image--rotate-coord): Improve doc string.
> 	(image--flip-map): Remove no more used flip argument.
>
> 	* test/lisp/image-tests.el (image-create-image-with-map): Use a
> 	valid SVG image otherwise `image-size' will not return a valid
> 	value and calculation of scale could fail.
> 	(image-transform-map): Update according to changed signature of
> 	image--flip-map.
>
> Thanks!
>
> [2. text/x-patch; image-compute-map-V2.patch]...





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 28 Apr 2024 11:24:10 GMT) Full text and rfc822 format available.

This bug report was last modified 5 days ago.

Previous Next


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