unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: David Ponce via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Joseph Turner <joseph@breatheoutbreathe.in>
Cc: Eli Zaretskii <eliz@gnu.org>, 69992@debbugs.gnu.org
Subject: bug#69992: Minor improvement to image map transformation logic
Date: Thu, 28 Mar 2024 00:53:26 +0100	[thread overview]
Message-ID: <0feeb39d-f2c5-4109-adce-23df2ab443a4@orange.fr> (raw)
In-Reply-To: <87cyrfl265.fsf@breatheoutbreathe.in>

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






  reply	other threads:[~2024-03-27 23:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-25  1:00 bug#69992: Minor improvement to image map transformation logic Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-27 11:16 ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-27 12:50   ` Eli Zaretskii
2024-03-27 14:21     ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-27 22:17       ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-27 23:53         ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-03-28 22:22         ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-29 10:19           ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-30  8:10             ` Eli Zaretskii
2024-03-30  8:55               ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-30 12:59                 ` David Ponce via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-30 13:37                   ` Eli Zaretskii
2024-03-30 19:07                   ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0feeb39d-f2c5-4109-adce-23df2ab443a4@orange.fr \
    --to=bug-gnu-emacs@gnu.org \
    --cc=69992@debbugs.gnu.org \
    --cc=da_vid@orange.fr \
    --cc=eliz@gnu.org \
    --cc=joseph@breatheoutbreathe.in \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).