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

Previous Next

Full log

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

Previous Next

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