bug#69992: Minor improvement to image map transformation logic

[David Ponce]

On 27/03/2024 23:17, Joseph Turner wrote:
> David Ponce <da_vid@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@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]...