Re: Linking to ImageMagick by default

[Alan Third]

On Sat, Dec 29, 2018 at 08:56:29AM +0200, Eli Zaretskii wrote:
> > Date: Fri, 28 Dec 2018 21:21:15 +0000
> > From: Alan Third <address@hidden>
> > Cc: address@hidden
> > 
> > > One reason to avoid this new struct is that it makes struct glyph,
> > > struct glyph_string, and struct it larger, which will make the display
> > > code slower, especially if the larger structs cause spilling of CPU
> > > caches.  The display engine shuffles these structures to and fro in
> > > many places, so they should be as small as possible.
> > 
> > I added this struct to separate the image pixel data from the size
> > information. The idea is to avoid loading and holding multiple copies
> > of the same image in RAM when we want to display at different sizes.
> 
> I think with my suggestion you will still hold only one copy: the last
> one whose size was calculated.  AFAIU, the same happens with your
> implementation, you just hold the last size separately.

If someone were to do something like this

  (insert-image (create-image "file.png" nil nil :width 100))
  (insert-image (create-image "file.png" nil nil :width 200))

We need to be able to differentiate between the two images. The
current way is just to load file.png twice, and hold the two different
sized images separately.

My method loads it once, then makes redisplay work out the sizes
before it displays them.

I believe your method loads it once, then updates the image struct
on the fly between the two sizes as and when it needs them. I’m not
sure how that would work.

To be perfectly honest, I’m not entirely sold on the idea that we need
to hold only one copy, it just seems like a waste to load and store
the image multiple times. But, Emacs is not a graphics application and
it seems unlikely that displaying multiple, slightly different copies
of an image is a major use‐case.

I may just scrap all this and go back to the basics and provide a
resizing system that just loads and stores each image separately, like
the current setup. If we want we can revisit the caching and so on later.

(A side note: I believe the NS toolkits provide transparent image
caching anyway, so this doesn’t really matter there.)

> > I’m not sure how to check whether the image needs resizing without
> > doing the lookups for :width, :height, :max-width, etc.
> 
> Maybe I'm misreading the code, but it looks to me you only need
> :scale, :width, and :height.  Am I missing something?

As Juri also pointed out it’s often preferred to set :max-width or
:max-height purely to make sure an image fits in a certain space
without having to calculate the required width and height to maintain
the aspect ratio and so on.

> > At the moment it’s avoided simply by not using :type imagemagick.
> 
> If the caller might know whether resizing is needed, perhaps we should
> pass an extra flag argument to that effect.

Perhaps the best solution is to parse the lisp image spec in a
different way, so it only runs through the plist once and notes down
the settings, instead of the current method of running through it
every time? We’d still have to do some size checking, but I suspect
the time saved on parsing the image spec would be far larger than
that.

FWIW, the time spent throwing around pixel data is, as far as I can
tell, far greater than the time spent parsing the spec and calculating
the sizes.

-- 
Alan Third