display word wrapping

display word wrapping

[Miles Bader]

Using variable-width fonts more regularly has gotten me musing about
display word-wrapping support (for the next version, not this one :-).

I've always thought that since the display engine does `proper' wrapping
for variable-width characters already (variable-width fonts, tabs, etc),
which really seems to work awful darn well, maybe it wouldn't be too
hard to trick it into treating words in the buffer as `characters' for
wrapping purposes.  That seems like it might make everything just work
correctly.

What I'm wondering is what other support, besides the basic `wrap lines
at word boundaries' is needed.

  * A text-property that defines left/right margins -- with display
    wrapping the window boundary is probably not exactly right,
    you often want some whitespace in there.

  * A way to define how words are divided for the magic wrapping.  I
    suppose it should use syntax tables in some way; I'm not sure are
    they easy to use or not from the display engine's point of view?
    I'm thinking that to make things simple, you'd always want to wrap
    _after_ whitespace (so that it remains editable, but doesn't mess
    with your left margin)

  * You probably want to turn off the fringe wrapping glyphs, but maybe
    not the fringes...

  * Good support for display-oriented C-n/C-p (I'm not sure existing
    methods e.g using `vertical-motion' work well with variable-width
    characters)

What else?

Just babbling,

-Miles
-- 
Any man who is a triangle, has thee right, when in Cartesian Space, to
have angles, which when summed, come to know more, nor no less, than
nine score degrees, should he so wish.  [TEMPLE OV THEE LEMUR]

Re: display word wrapping

[Kai Grossjohann]

Miles Bader <miles@lsi.nec.co.jp> writes:

> Using variable-width fonts more regularly has gotten me musing about
> display word-wrapping support (for the next version, not this one :-).

Cool.

> What I'm wondering is what other support, besides the basic `wrap lines
> at word boundaries' is needed.
>
>   * A text-property that defines left/right margins -- with display
>     wrapping the window boundary is probably not exactly right,
>     you often want some whitespace in there.

Do we want hanging indents, like in the itemized list you are using
in your posting?

>   * You probably want to turn off the fringe wrapping glyphs, but maybe
>     not the fringes...

There should be a way to tell which "visual" newlines are actually
in the file and which ones are just produced from line wrapping.

>   * Good support for display-oriented C-n/C-p (I'm not sure existing
>     methods e.g using `vertical-motion' work well with variable-width
>     characters)

What about "." in regular expressions?  What should paragraph
operations do?  (I'm guessing that C-a/C-e should move visually.)


Regarding the hanging indents thing, perhaps that could lead to a
more general mode where we always stuff things to the left/right of
the "real" line.  I think that currently, there is specialized
support for M-q in multiline /* ... */ \n /* ... */ style comments.
We could also use the "line prefix/suffix" stuff for editing LaTeX
documentation (are they called DOC files?), for using text mode in
comments, and whatnot.

Does this have anything to do with mmm-mode or multi-mode?

Kai

Re: display word wrapping

[Kim F. Storm]

Miles Bader <miles@lsi.nec.co.jp> writes:

> Using variable-width fonts more regularly has gotten me musing about
> display word-wrapping support (for the next version, not this one :-).
> 

FYI, doing this is already on my to-do list.

For now, can you (and everybody else) please focus on
completing/testing/debugging the 21.4 release.  Thank you.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Miles Bader]

On Wed, May 26, 2004 at 01:29:47PM +0200, Kim F. Storm wrote:
> FYI, doing this is already on my to-do list.

Hey, it's been on my TODO list for _years_... :-)

-Miles
-- 
Occam's razor split hairs so well, I bought the whole argument!

Re: display word wrapping

[David Kastrup]

Miles Bader <miles@gnu.org> writes:

> On Wed, May 26, 2004 at 01:29:47PM +0200, Kim F. Storm wrote:
> > FYI, doing this is already on my to-do list.
> 
> Hey, it's been on my TODO list for _years_... :-)

Then let's just keep it there until we have cranked out 21.4, merged
the Unicode and tty branch and cranked out 22.0.  Somewhere along that
line, if we really must, one can create a branch with significant
display engine modifications like that.  But I think it would probably
be shortsighted to tackle automatic word wrap before the Bidi branch
has been merged as well: that one has a lot of potential for
interference with word wrapping.

We really need to get ourselved wrapped around _finishing_ some stuff
instead of resynching half-baked and new material time and again.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> We really need to get ourselved wrapped around _finishing_ some stuff
> instead of resynching half-baked and new material time and again.

Agree 100%

We (sort of) declared feature freeze on May 1st, so if there are still
major stuff waiting to be installed or merged, please concentrate on
that and nothing else.

Some pending projects for 21.4 are:

- Face remapping

  I suppose it's done, just waiting to be committed.

- Gnus update

  If this cannot be done real soon now, maybe we should postpone it.

- Tramp reentrancy rework

  This is really a bug-fix, but seems to require major changes.

  Maybe we don't really need to do this for 21.4, if there is a
  simpler way to simply avoid reentrancy (e.g. disable auto-revert in
  remote buffers).

- The "curry stuff"

  IIUC, this is not really needed in 21.4, so unless somebody really
  need this now, it should wait.

What else ?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> We (sort of) declared feature freeze on May 1st, so if there are still
> major stuff waiting to be installed or merged, please concentrate on
> that and nothing else.
> 
> Some pending projects for 21.4 are:
> 
> - Face remapping
> 
>   I suppose it's done, just waiting to be committed.

Sounded more or less like it, yes.

> - Gnus update
> 
>   If this cannot be done real soon now, maybe we should postpone it.

I think that might be a mistake: Gnus is a major subsystem and IIRC
the "new" Gnus has had several important changes and additions.
Miles, can you think of anything others could do to help speed up the
process?  Should one perhaps just do a fast scan through the ChangeLog
since the last Gnus version was checked in, and compare with the
upstream ChangeLog in order to find whether there are any changes not
reflected upstream?  And if there aren't, just scratch thinking about
them and just overwrite everything with the current Gnus versions?

> - Tramp reentrancy rework
> 
>   This is really a bug-fix, but seems to require major changes.

I am not so sure.  Most of the stuff we had up to now involved
crashes in the garbage collector, and that is supposed to be fixed by
now.

>   Maybe we don't really need to do this for 21.4, if there is a
>   simpler way to simply avoid reentrancy (e.g. disable auto-revert
>   in remote buffers).

Yup.  One must not forget that a buggy version of tramp does not
imply a regression when compared with 21.3 and no tramp, or an
equally buggy add-on tramp.  Except for those cases where ange-ftp
breaks.

> - The "curry stuff"
> 
>   IIUC, this is not really needed in 21.4, so unless somebody really
>   need this now, it should wait.
> 
> What else ?

Checking the manual, I guess.

In the realm of bugfixes, the scrolling and redisplay behavior might
still need work.  Again, I don't think that we have much of a
regression here as compared to previous versions, but it would be
convenient if the quality could be improved to a point where people
won't press for 22.0 merely because of bugfixes.  It would be nice if
we could with a reasonably good conscience not bother about any more
21.x bug fix branches/releases once 21.4 is out.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Miles Bader]

On Wed, May 26, 2004 at 09:16:27PM +0200, David Kastrup wrote:
> Miles, can you think of anything others could do to help speed up the
> process?  Should one perhaps just do a fast scan through the ChangeLog
> since the last Gnus version was checked in, and compare with the
> upstream ChangeLog in order to find whether there are any changes not
> reflected upstream?

Of course I'll look at the ChangeLogs, but people are notoriously bad at
keeping it up-to-date, especially for "trivial" things, and merges, etc.

> And if there aren't, just scratch thinking about
> them and just overwrite everything with the current Gnus versions?

No, I think this would be a mistake -- clearly there _are_ going to be
changes that get accidentally dropped one way or the other other, but I want
to maintain the property that changes made to the emacs Gnus tree are "real"
changes, which won't just disappear in the future.

> In the realm of bugfixes, the scrolling and redisplay behavior might
> still need work.

I still get hit by the "i-search invalid face-cache coredump" occasionally
(see previous threads).  It's very resistant to being found...

-Miles
-- 
P.S.  All information contained in the above letter is false,
      for reasons of military security.

i-search face-cache crash [was Re: display word wrapping]

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> I still get hit by the "i-search invalid face-cache coredump" occasionally
> (see previous threads).  It's very resistant to being found...

I do remember you reported a problem in this area, but IIRC you never really
described what happened or even hinted under what circumstances...

--
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: i-search face-cache crash [was Re: display word wrapping]

[Miles Bader]

On Wed, May 26, 2004 at 11:43:36PM +0200, Kim F. Storm wrote:
> > I still get hit by the "i-search invalid face-cache coredump" occasionally
> > (see previous threads).  It's very resistant to being found...
> 
> I do remember you reported a problem in this area, but IIRC you never really
> described what happened or even hinted under what circumstances...

Look at the old thread.

-Miles
-- 
Would you like fries with that?

Re: i-search face-cache crash [was Re: display word wrapping]

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> On Wed, May 26, 2004 at 11:43:36PM +0200, Kim F. Storm wrote:
> > > I still get hit by the "i-search invalid face-cache coredump" occasionally
> > > (see previous threads).  It's very resistant to being found...
> > 
> > I do remember you reported a problem in this area, but IIRC you never really
> > described what happened or even hinted under what circumstances...
> 
> Look at the old thread.

If you refer to the following description (the only one I can find),
it doesn't really give much info to work from ...

> I've had several crashes recently related to i-search faces, and haven't been
> able to track down the cause; it appears that the face cache ids are being
> used even after the face cache has been cleared.


Handa and Stefan provided some extra info that wasn't directly related
to your reports, but of course need to be looked into.

As I never had any problems with i-search faces (what's so special
about them?), at least you could tell us how you have configured those
faces.

++kfs

Re: i-search face-cache crash

[Miles Bader]

storm@cua.dk (Kim F. Storm) writes:
>> I still get hit by the "i-search invalid face-cache coredump" occasionally
>> (see previous threads).  It's very resistant to being found...
>
> I do remember you reported a problem in this area, but IIRC you never really
> described what happened or even hinted under what circumstances...

Here's the thread:

   http://mail.gnu.org/archive/html/emacs-devel/2004-04/msg00183.html

I'm not sure what else I can say, other than it only crashes when I'm
doing an i-search (I have some sense that it happens when I hit C-s
repeatedly quickly after having paused in the middle of an i-search, or
something, but it's only a vague feeling -- probably this indicates that
there's some relationship with the `other matches highlighting' feature
of i-search).  I can't seem to reproduce it intentionally.

As I mentioned in that thread, the immediate cause of the crash is that
it's trying to render with a face-id that isn't in the face-id cache.

-Miles
-- 
Come now, if we were really planning to harm you, would we be waiting here, 
 beside the path, in the very darkest part of the forest?

Re: i-search face-cache crash

[Richard Stallman]

    As I mentioned in that thread, the immediate cause of the crash is that
    it's trying to render with a face-id that isn't in the face-id cache.

Can you find where that face id came from?  Maybe we can add
debugging code to detect, earlier, putting the invalid face id
into that place.

Re: display word wrapping

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:

> > - Gnus update
> > 
> >   If this cannot be done real soon now, maybe we should postpone it.
> 
> I think that might be a mistake: Gnus is a major subsystem and IIRC
> the "new" Gnus has had several important changes and additions.

I use the Gnus in emacs CVS, and I'm pretty satisfied with it.

Sure, there are (lots of?) users who are using Oort Gnus now, but
not upgrading Gnus for 21.4 doesn't make 21.4 any worse than 21.3.

Of course, it would be very nice if 21.4 has a newer Gnus, and I would
probably welcome that (don't know, as I never had the time to look).

Can you tell me if upgrading from 21.3 (with old Gnus) to 21.4 (with
new Gnus) will require ANY changes to gnus configuration?  

I hope not, as Gnus setup still is pretty "magic" even to me.

> Checking the manual, I guess.

Indeed!

We also need to reorder NEWS, and possibly write anti-news for the manual.

> 
> In the realm of bugfixes, the scrolling and redisplay behavior might
> still need work.

Definitely.  Better scrolling behaviour, notably in the presense of
tall images is top of my list.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Jason Rumney]

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
>
>> I think that might be a mistake: Gnus is a major subsystem and IIRC
>> the "new" Gnus has had several important changes and additions.
>
> I use the Gnus in emacs CVS, and I'm pretty satisfied with it.

You don't use IMAP with UW imapd then. At the very least we would
have to backport some of the bugfixes, and by the time we figure out
which ones, we might as well have just installed the new version.

Re: display word wrapping

[Luc Teirlinck]

David Kastrup wrote:

   > - Tramp reentrancy rework
   > 
   >   This is really a bug-fix, but seems to require major changes.

   I am not so sure.  Most of the stuff we had up to now involved
   crashes in the garbage collector, and that is supposed to be fixed by
   now.

Hangs are pretty bad too, even if they are not crashes.
   
   >   Maybe we don't really need to do this for 21.4, if there is a
   >   simpler way to simply avoid reentrancy (e.g. disable auto-revert
   >   in remote buffers).

   Yup.  One must not forget that a buggy version of tramp does not
   imply a regression when compared with 21.3 and no tramp, or an
   equally buggy add-on tramp.  Except for those cases where ange-ftp
   breaks.

I am not completely sure of that.

In emacs-21.3 -q:

file-name-handler-alist's value is 
(("^/[^/:]*\\'" . ange-ftp-completion-hook-function)
 ("^/[^/:]*[^/:.]:" . ange-ftp-hook-function)
 ("\\`/:" . file-name-non-special))

In emacs-21.3.50 -q:

file-name-handler-alist's value is 
(("\\`/[^/:]+:" . tramp-file-name-handler)
 ("\\`/:" . file-name-non-special))

The one problem we know of is auto-revert-mode, but anything that
can call file-exists-p or its friends while Tramp is waiting for
output could cause exactly the same problems.

Sincerely,

Luc.

Re: display word wrapping

[Miles Bader]

David Kastrup <dak@gnu.org> writes:
> I think that might be a mistake: Gnus is a major subsystem and IIRC
> the "new" Gnus has had several important changes and additions.
> Miles, can you think of anything others could do to help speed up the
> process?

Give me some time to get my head around it more, but the big steps
involve going over a (really large) patch file and classifying changes
into categories like trivial-cleanup / obvious-bugfix /
emacs-tree-localization / obvious-regression / don't-know.  The
ChangeLogs sometimes provide a hint, but they aren't nearly complete
enough to be all that much help.

I tried doing some of this categorization already, and it's pretty
straight-forward, especially using diff-mode commands (most changes are
`trivial', so you can just skip over those quickly), but it's pretty
tedious given the enormous size of the diffs (and this is _just_ the
Emacs vs. Gnus 5.8 diffs -- which _should_ be relatively small, as Emacs
current version was forked from Gnus 5.8).

If I can get some volunteers to help do this categorization, it should
parallelize pretty easily I think.

Once I have a set of what seem to be `good' changes, I can hopefully
apply it to the Gnu 5.10 tree -- there will be duplicates where bugfixes
were forward-ported to 5.10, but hopefully automatic duplicate
suppression will remove some of that annoyance (maybe...).

-Miles
-- 
80% of success is just showing up.  --Woody Allen

Re: display word wrapping

[Miles Bader]

On Wed, May 26, 2004 at 05:06:57PM +0200, Kim F. Storm wrote:
> - Gnus update
> 
>   If this cannot be done real soon now, maybe we should postpone it.

No, this must happen I think.

Gnus is "safe", it's important, and the release isn't going to be tomorrow
regardless.

-Miles
-- 
97% of everything is grunge

Re: display word wrapping

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> On Wed, May 26, 2004 at 05:06:57PM +0200, Kim F. Storm wrote:
> > - Gnus update
> > 
> >   If this cannot be done real soon now, maybe we should postpone it.
> 
> No, this must happen I think.
> 
> Gnus is "safe", it's important, 

So upgrading to the new Gnus is seamless?!?!

>                                 and the release isn't going to be tomorrow
> regardless.

The sooner we get a chance to work with the new Gnus, the better.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Miles Bader]

On Wed, May 26, 2004 at 11:01:41PM +0200, Kim F. Storm wrote:
> > Gnus is "safe", it's important, 
> 
> So upgrading to the new Gnus is seamless?!?!

_Nothing_ is seamless, but the version of Gnus proposed for merging is
apparently very stable (it's been the Gnus "stable" version for a year).

Anyway, in my opinion, Emacs should not be released without a Gnus update,
even if it delays the release by some amount.  Waiting until the next Emacs
release would be absurd.

-Miles
-- 
My spirit felt washed.  With blood.  [Eli Shin, on "The Passion of the Christ"]

Re: display word wrapping

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> On Wed, May 26, 2004 at 11:01:41PM +0200, Kim F. Storm wrote:
> > > Gnus is "safe", it's important, 
> > 
> > So upgrading to the new Gnus is seamless?!?!
> 
> _Nothing_ is seamless, but the version of Gnus proposed for merging is
> apparently very stable (it's been the Gnus "stable" version for a year).

I meant from a user's point of view...  Can I continue using the
(exact) same Gnus configuration and setup after the upgrade?

> 
> Anyway, in my opinion, Emacs should not be released without a Gnus update,
> even if it delays the release by some amount.  Waiting until the next Emacs
> release would be absurd.

If the new Gnus is stable, why does this have to take so much time?

Perhaps you could install the new Gnus now and later fix/merge changes
from the current emacs CVS version if there are problems (I suppose
you can do that easily with arch :-)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Miles Bader]

On Wed, May 26, 2004 at 11:55:28PM +0200, Kim F. Storm wrote:
> > _Nothing_ is seamless, but the version of Gnus proposed for merging is
> > apparently very stable (it's been the Gnus "stable" version for a year).
> 
> I meant from a user's point of view...  Can I continue using the
> (exact) same Gnus configuration and setup after the upgrade?

Yes

> If the new Gnus is stable, why does this have to take so much time?

Because the Emacs Gnus and the `Gnus Gnus' have not been kept in sync well
[there's been some occasional merging of changes, but if anything it's made
the situation worse, because as far as I can see it's only been done
haphazardly, and it's muddied the waters].

One of the big annoyances is the many `trivial changes' that have been done
to the emacs tree, e.g. whitespace tweaks, spelling fixes, etc., which tend
to hide real bugfixes merely by their huge quantity.

> Perhaps you could install the new Gnus now and later fix/merge changes
> from the current emacs CVS version if there are problems

I will do that if I must, but I'd like to see if I can do things properly
first.  I still have some hope I can sort things out.

> (I suppose you can do that easily with arch :-)

Ha, ha, I wish arch was magic like that ... :-)

Hopefully arch will make things easier in the future, but...  [I have been
careful to create the new Gnus arch tree in a way that it knows which files
correspond to each other in Emacs and Gnus.]

-Miles
-- 
"1971 pickup truck; will trade for guns"

Re: display word wrapping

[Stefan Monnier]

>> - Gnus update
>> If this cannot be done real soon now, maybe we should postpone it.
> No, this must happen I think.

Agreed.


        Stefan

Re: display word wrapping

[Richard Stallman]

We definitely need to update Gnus before the next Emacs release.
Why have the Gnus developers have not installed their changes?

Re: display word wrapping

[Juanma Barranquero]

On 26 May 2004 17:06:57 +0200
storm@cua.dk (Kim F. Storm) wrote:

> Some pending projects for 21.4 are:

[...]

> What else ?

IMO the image support on Windows is still shaky. Current problems with
that (at least, these are the problems I'm experiencing):

 - There's no single, good, Windows version of the relevant libraries,
so we must try to load each library with several names, hoping for the
best (for example, we try to load libjpeg.dll, jpeg-62.dll and jpeg.dll,
in that order). That seems fragile.

 - The best-so-far libraries (that I know of) are the GnuWin32 ones, but
(relatively) recent upgrades in some of them changed some names, so if
you try to use them you end loading several, differently named, zlib
DLLs, for example.

 - On MSVC optimized builds, several libraries (most notably the TIFF
one) crash on loading an image. I've tried to debug it several times,
with no success; I'm not even sure whether is a real Emacs bug hidden
somewhere, or a bug in the MSVC optimizer. Worse, with newest GnuWin32
releases the problem affect more libraries, and some of them (libpng)
don't seem to work at all.

I'm not entirely sure what to do. Perhaps we could compile our own
graphics libs and distribute them with Emacs, though that's quite a lot
of additional work. But as I stands, I'd add a section to nt/INSTALL
saying: "Don't compile with image support unless you know what you're
doing and are willing to do some debugging."

                                                                Juanma

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 26 May 2004 17:06:57 +0200
> storm@cua.dk (Kim F. Storm) wrote:
> 
> > Some pending projects for 21.4 are:
> 
> [...]
> 
> > What else ?
> 
> IMO the image support on Windows is still shaky. Current problems with
> that (at least, these are the problems I'm experiencing):

It certainly sounds messy, and quite tricky to do right for
making a binary distribution for W32!

Would some W32 experts step forward to look into this?


> I'm not entirely sure what to do. Perhaps we could compile our own
> graphics libs and distribute them with Emacs, though that's quite a lot
> of additional work. But as I stands, I'd add a section to nt/INSTALL
> saying: "Don't compile with image support unless you know what you're
> doing and are willing to do some debugging."

Wouldn't it be better to tell what the problems are -- and what
combinations are known/supposed to [not] work...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 27 May 2004 12:41:49 +0200
storm@cua.dk (Kim F. Storm) wrote:

> It certainly sounds messy, and quite tricky to do right for
> making a binary distribution for W32!

Perhaps I've made it sound worse that it is.  I think Jason had it
working fine on his setup (with gcc, IIRC) last time I exchanged e-mail
with him about the issue (though that was more than a year ago).

> Would some W32 experts step forward to look into this?

More "image experts" than "W32 experts", I think.

But anyway, some issues are tricky because... well, just "because".  For
example, the current method of trying several DLL names is a feature, in
the sense that it would allow the user to use other DLLs than the ones
used by the binary-distribution packager.  But, for what I gather from
my limited experience, just switching DLLs is *not* likely to work on
Windows.  That's why I would advocate suppling our own (not necessarily
compiled by us), either on the binary tarball, or as a .zip pointed from
somewhere in http://www.gnu.org/software/emacs/windows/ntemacs.html.

> Wouldn't it be better to tell what the problems are -- and what
> combinations are known/supposed to [not] work...

I suppose so.  The main problem, it seems, is that not many people out
there (that we know of) has tried the Windows image support, so we still
know little about how much it works, and in how many different setups. 
Perhaps I'm the lone exception, but I don't think so.  I've got the same
crashes on every combination of W2K/XP and MSVC 6.0/.NET/.NET 2003, and
that are not uncommon setups.

OTOH, for a 21.4 (or 21.5, did we decide on that?) release point of view,
if it works on GCC builds, that would be enough to make a Windows binary
tarball.  We can always put as much information as we have on nt/INSTALL
for the relatively few people who's going to build their own Emacsen.

                                                                Juanma

Re: display word wrapping

[Jason Rumney]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 27 May 2004 12:41:49 +0200
> storm@cua.dk (Kim F. Storm) wrote:
>
>> It certainly sounds messy, and quite tricky to do right for
>> making a binary distribution for W32!
>
> Perhaps I've made it sound worse that it is.  I think Jason had it
> working fine on his setup (with gcc, IIRC) last time I exchanged e-mail
> with him about the issue (though that was more than a year ago).

Image support has always worked for me with both gcc and msvc
(non-optimising version though). I recall we made some changes to
detect broken versions of some libraries and refuse to use them, but
was not aware (had forgotten?) that the problems were still there.

I think the problems with MSVC's optimisations is likely due to
alignment differences in structs we are passing into the library. If
the problem is just with MSVC, we can just add a note to PROBLEMS,
since the official binaries will be built with gcc anyway.

> That's why I would advocate suppling our own (not necessarily
> compiled by us), either on the binary tarball, or as a .zip pointed
> from somewhere in http://www.gnu.org/software/emacs/windows/ntemacs.html.

Remember anything we distribute in binary form, we also need to
distribute source for. Rather than clutter the gnu.org servers with
non-GNU image library code just because some Windows users might want
to recompile their Emacs with a proprietary compiler, I think we can
just say that such people should recompile the image libraries
themselves too. That is the easiest way to ensure that the image
libraries will work.


There may also be problems with specific versions of image libraries
that affect a gcc build of Emacs as well. These cases we should try
to find out why and test for it when loading the libraries.

Re: display word wrapping

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> I think the problems with MSVC's optimisations is likely due to
> alignment differences in structs we are passing into the library. If
> the problem is just with MSVC, we can just add a note to PROBLEMS,
> since the official binaries will be built with gcc anyway.

Better put it in nt/INSTALL

>                                                       I think we can
> just say that such people should recompile the image libraries
> themselves too. That is the easiest way to ensure that the image
> libraries will work.

That's an OK solution IMO.

Or it could recommend to not use optimization with MSVC.


> There may also be problems with specific versions of image libraries
> that affect a gcc build of Emacs as well. These cases we should try
> to find out why and test for it when loading the libraries.

If possible, yes.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On Fri, 28 May 2004 08:57:43 +0100
Jason Rumney <jasonr@gnu.org> wrote:

> Image support has always worked for me with both gcc and msvc
> (non-optimising version though).

It's been a while since I last tested all this. I'll try to find some
time to debug it properly ASAP (for some definition of "soon"). But the
gist of it is that before (about a year ago), it worked fine on
MSVC non-opt builds, and TIFF failed (crashed Emacs, really) on opt
builds. Now almost all graphics libs crash on opt builds, but I'm using
newer GnuWin32 libraries, so perhaps the problem is not in Emacs. I've
not tested recently non-opt Emacs builds, but I seem to remember I did
it once on the past two or three months and it also didn't work right.
I'll have to check it.

> I think the problems with MSVC's optimisations is likely due to
> alignment differences in structs we are passing into the library. If
> the problem is just with MSVC, we can just add a note to PROBLEMS,
> since the official binaries will be built with gcc anyway.

Well, I'd like for it to work, so I'll investigate what you've suggested.
But yes, this is a minor issue because most Emacs Windows users will use
the official binary tarball.

> I think we can
> just say that such people should recompile the image libraries
> themselves too. That is the easiest way to ensure that the image
> libraries will work.

OK, though I'm not sure how easy is to compile libtiff, libungif etc. on
Windows.

> There may also be problems with specific versions of image libraries
> that affect a gcc build of Emacs as well. These cases we should try
> to find out why and test for it when loading the libraries.

Yes. Definitely image libs on Windows builds of Emacs need to get more
testing.

                                                                Juanma

Re: display word wrapping

[Juanma Barranquero]

On Fri, 28 May 2004 08:57:43 +0100, Jason Rumney <jasonr@gnu.org> wrote:

> I think the problems with MSVC's optimisations is likely due to
> alignment differences in structs we are passing into the library.

Hmm.  I don't think so.  On one hand, we're compiling MSVC builds (opt
or non-opt) with -Zp8; on the other, it'd be weird that an alignment bug
crashed practically *every* graphics lib.

In fact, the crash happens on image_spec_value.  Disabling global
optimizations for the function makes all work fine again.

As a last (ugly) resort, we can put

#ifdef _MSC_VER_
#pragma optimize("g", off)
#endif

just before image_spec_value.  But before resorting to that I'm gonna
try to understand what's happening.

                                                           /L/e/k/t/u

Re: display word wrapping

[Juanma Barranquero]

On Sat, 29 May 2004 22:54:58 +0200, Juanma Barranquero <lektu@mi.madritel.es> wrote:

> In fact, the crash happens on image_spec_value.

Well, not exactly (I spoke too fast).

I can make it work fine (for all kinds of graphics) by disabling -Og for
just two functions: `lookup_image' and `png_read_from_memory'.

On most debugging sessions, lookup_image fails while calling
image_spec_value or postprocess_image, because img->spec is no longer
valid (somewhere, img->spec is overwritten). But something's afoul,
because inserting debugging code into lookup_image makes it fail in
different ways (for example, failing to load the image). Either the
optimization is doing something very wrong, or an undetected bug is
causing data corruption, I'd say.

Ideas?

                                                           /L/e/k/t/u

Re: display word wrapping

[Eli Zaretskii]

> Date: Sun, 30 May 2004 03:38:48 +0200
> From: Juanma Barranquero <lektu@mi.madritel.es>
> 
> I can make it work fine (for all kinds of graphics) by disabling -Og for
> just two functions: `lookup_image' and `png_read_from_memory'.
> 
> On most debugging sessions, lookup_image fails while calling
> image_spec_value or postprocess_image, because img->spec is no longer
> valid (somewhere, img->spec is overwritten). But something's afoul,
> because inserting debugging code into lookup_image makes it fail in
> different ways (for example, failing to load the image). Either the
> optimization is doing something very wrong, or an undetected bug is
> causing data corruption, I'd say.
> 
> Ideas?

Does it crash (it _is_ a crash, isn't it?) under a debugger, or does
that, too, cause the bug to change its behavior?

If the former, run an unmodified optimized code under a debugger and
use debugger facilities instead of debugging code to see what's wrong.

Comparison of machine code in the optimized and unoptimized versions
might also tell you something useful.

Finally, assuming that we are talking about a crash, posting the
detailed description of the crash (exception number, register dump,
stack dump, etc.) here could give some further ideas.

Re: display word wrapping

[Juanma Barranquero]

On 30 May 2004 08:12:41 +0200, Eli Zaretskii <eliz@gnu.org> wrote:

> Does it crash (it _is_ a crash, isn't it?) under a debugger, or does
> that, too, cause the bug to change its behavior?

Yes, it is a crash. No, it fails the same way under debugging.

> If the former, run an unmodified optimized code under a debugger and
> use debugger facilities instead of debugging code to see what's wrong.

Yeah, I've already tried, and I see *where* it is failing, but debugging
optimized code is a bit of a pain.

> Comparison of machine code in the optimized and unoptimized versions
> might also tell you something useful.

It would, if I were more sure about what the code is exactly supposed to
do. I'm no expert in the image code.

> Finally, assuming that we are talking about a crash, posting the
> detailed description of the crash (exception number, register dump,
> stack dump, etc.) here could give some further ideas.

When I try to load a 811-byte .gif, I get:

  Unhandled exception at 0x0106dd08 in emacs.exe: 0xC0000005: Access
  violation reading location 0x00000004.

The stack trace is:

  image_spec_value(int spec=-536870912, int key=557610632, int * found=0x00000000) Line 931 + 0x8	C
  lookup_image(frame * f=0x015dbe00, int spec=-1572937664) Line 1682	C
  handle_single_display_prop(it * it=0x00000000, int prop=5, int object=-2109577216, text_pos * position=0x0082f04c, int display_replaced_before_p=0) Line 3702	C
  handle_display_prop(it * it=0x0082f04c) Line 3339 + 0x10	C
  handle_stop(it * it=0x0082efb0) Line 2591 + 0x3	C
  reseat(it * it=0x00000000, text_pos pos={...}, int force_p=1) Line 4676 + 0x6	C
  init_iterator(it * it=0x0082efb0, window * w=0x015dbe00, int charpos=1, int bytepos=1, glyph_row * row=0x02426800, face_id base_face_id=DEFAULT_FACE_ID) Line 2269 + 0x18	C
  start_display(it * it=0x0082efb0, window * w=0x0211ac00, text_pos pos={...}) Line 2288 + 0x1f	C
  try_window(int window=-2112771072, text_pos pos={...}) Line 12211 + 0x20	C
  redisplay_window(int window=-2112771072, int just_this_one_p=0) Line 11858 + 0xc	C
  redisplay_window_0(int window=-2112771072) Line 10588 + 0xa	C
  internal_condition_case_1(int (void)* bfun=0x0103fbcb, int arg=-2112771072, int handlers=-1589824248, int (void)* hfun=0x01029d11) Line 1374 + 0x35	C
  redisplay_windows(int window=-2112771072) Line 10569 + 0x1d	C
  redisplay_internal(int preserve_echo_area=0) Line 10152 + 0x8	C
  redisplay() Line 9386 + 0x7	C
  read_char(int commandflag=1, int nmaps=5, int * maps=0x0082fbec, int prev_event=557609984, int * used_mouse_menu=0x0082fc44) Line 2487	C
  read_key_sequence(int * keybuf=0x0082fce8, int bufsize=30, int prompt=557609984, int dont_downcase_last=0, int can_return_switch_frame=1, int fix_current_buffer=1) Line 8783 + 0x24	C
  command_loop_1() Line 1490 + 0x25	C
  internal_condition_case(int (void)* bfun=0x0105b380, int handlers=557688384, int (void)* hfun=0x01058003) Line 1334	C
  command_loop_2() Line 1271 + 0x15	C
  internal_catch(int tag=557684672, int (void)* func=0x0105c094, int arg=557609984) Line 1094 + 0x6	C
  command_loop() Line 1251	C
  recursive_edit_1() Line 961 + 0x5	C
  Frecursive_edit() Line 1023	C
  main() Line 1693 + 0x5	C
  mainCRTStartup() Line 259 + 0x12	C
  GetCurrentDirectoryW() + 0x44	

lookup_image is calling image_spec_value with the following code:

	  if (! img->background_valid)
	    {
	      bg = image_spec_value (img->spec, QCbackground, NULL);
	      if (!NILP (bg))
		{
		  img->background
		    = x_alloc_image_color (f, img, bg,
					   FRAME_BACKGROUND_PIXEL (f));
		  img->background_valid = 1;
		}
	    }

At that point, img->spec is not valid. Previously, img has been
initialized from spec (the arg passed to the function), so spec and
img->spec should be equal, but they aren't anymore. If you change the
call to do

 bg = image_spec_value (spec, QCbackground, NULL);

it succeeds, but then the call to postprocess_image

	  if (!EQ (*img->type->type, Qpostscript))
	    postprocess_image (f, img);

fails for the same reason (img->spec is not valid).

On optimized code, img is not on the stack, but on a register. In fact,
if I change

  struct image *img;

to

  static struct image *img;

Emacs works fine. So it looks like it *is* really a bug in the optimizer
code, which is clobbering a register or something like that.


                                                           /L/e/k/t/u

Re: display word wrapping

[Eli Zaretskii]

> Date: Sun, 30 May 2004 18:50:27 +0200
> From: Juanma Barranquero <lektu@mi.madritel.es>
> 
> Yes, it is a crash. No, it fails the same way under debugging.

So we are lucky ;-)

> > Finally, assuming that we are talking about a crash, posting the
> > detailed description of the crash (exception number, register dump,
> > stack dump, etc.) here could give some further ideas.

[I apologize in advance if I'm going to say things that you already
know.]
 
> When I try to load a 811-byte .gif, I get:
> 
>   Unhandled exception at 0x0106dd08 in emacs.exe: 0xC0000005: Access
>   violation reading location 0x00000004.

This means Emacs tried to dereference an invalid pointer and read from
the invalid address.  Exception 0xC0000005 is Page Fault, the Windows
equivalent of SIGSEGV, and the bad address is 0x4 (a.k.a. a NULL
pointer).

> The stack trace is:
> 
>   image_spec_value(int spec=-536870912, int key=557610632, int * found=0x00000000) Line 931 + 0x8	C
>   lookup_image(frame * f=0x015dbe00, int spec=-1572937664) Line 1682	C
> [...]
> lookup_image is calling image_spec_value with the following code:
> 
> 	  if (! img->background_valid)
> 	    {
> 	      bg = image_spec_value (img->spec, QCbackground, NULL);
> 	      if (!NILP (bg))
> 		{
> 		  img->background
> 		    = x_alloc_image_color (f, img, bg,
> 					   FRAME_BACKGROUND_PIXEL (f));
> 		  img->background_valid = 1;
> 		}
> 	    }
> 
> At that point, img->spec is not valid.

How exactly is img->spec invalid?  Can you post the details (xtype
etc.)?

> Previously, img has been
> initialized from spec (the arg passed to the function), so spec and
> img->spec should be equal, but they aren't anymore. If you change the
> call to do
> 
>  bg = image_spec_value (spec, QCbackground, NULL);
> 
> it succeeds

If you think that img->spec is somehow clobbered in between the point
where it is computed and the point where image_spec_value is called,
the way to see who is clobbering it is to put a data breakpoint
(a.k.a. watchpoint) on img->spec, and see when that watchpoint
triggers.

> On optimized code, img is not on the stack, but on a register. In fact,
> if I change
> 
>   struct image *img;
> 
> to
> 
>   static struct image *img;
> 
> Emacs works fine.

If you change it to

   volatile struct image *img;

does the problem go away as well?

> So it looks like it *is* really a bug in the optimizer
> code, which is clobbering a register or something like that.

If the clobbered pointer is in a register, then data breakpoint will
not help, but you could still do it by single-stepping the program and
looking for a line in the code where the value of that register is
clobbered.  This could be a bit tedious, but it's simple and
straightforward, and will probably reveal some real bug in the code
(the possibility of a bug in the compiler's optimizer exists, but I
think it's only a distant second).

Re: display word wrapping

[Juanma Barranquero]

On Sun, 30 May 2004 20:27:40 +0200, "Eli Zaretskii" <eliz@gnu.org> wrote:

> So we are lucky ;-)

Yeah ;)

> [I apologize in advance if I'm going to say things that you already
> know.]

[Don't worry; some things I already know, some not, some I had forgotten
to try...]

> This means Emacs tried to dereference an invalid pointer and read from
> the invalid address.  Exception 0xC0000005 is Page Fault, the Windows
> equivalent of SIGSEGV, and the bad address is 0x4 (a.k.a. a NULL
> pointer).

[...]

> How exactly is img->spec invalid?  Can you post the details (xtype
> etc.)?

Basically, img points to nowhere, so img->spec is some random value.

> If you change it to
> 
>    volatile struct image *img;
> 
> does the problem go away as well?

No.

> If the clobbered pointer is in a register, then data breakpoint will
> not help, but you could still do it by single-stepping the program and
> looking for a line in the code where the value of that register is
> clobbered.

The register used to store img (edi, in my tests) is being modified by
this call:

    img->load_failed_p = img->type->load (f, img) == 0;

If I put

  __asm push img
  img->load_failed_p = img->type->load (f, img) == 0;
  __asm pop img

(which forces a reload of the edi register from the stored value of img),
it works.

More ideas?

                                                           /L/e/k/t/u

Re: display word wrapping

[Eli Zaretskii]

> Date: Mon, 31 May 2004 02:40:19 +0200
> From: Juanma Barranquero <lektu@mi.madritel.es>
> 
> > How exactly is img->spec invalid?  Can you post the details (xtype
> > etc.)?
> 
> Basically, img points to nowhere, so img->spec is some random value.

Ah, yes, I forgot that Windows doesn't Page Fault on NULL pointer
dereferences, but instead fetches some random garbage...

> > If you change it to
> > 
> >    volatile struct image *img;
> > 
> > does the problem go away as well?
> 
> No.

This probably means that the optimizer is not in itself the direct
culprit here, as declaring img `volatile' should have prevented the
compiler from keeping it in a register.

However, this observation:

> If I put
> 
>   __asm push img
>   img->load_failed_p = img->type->load (f, img) == 0;
>   __asm pop img
> 
> (which forces a reload of the edi register from the stored value of img),
> it works.

seems to say that img is still in a register, even if declared
`volatile'.  Could you please look at the machine code near the
invocation of "img->type->load (f, img)" and see whether `volatile'
changes something there, and if so, what?

Also, the above suggests that a work-around, if we don't find anything
better, is to save img into a temporary variable before the call that
clobbers it and restore it after that.  But such a workaround needs
to be more clever than the push/pop pair above, since it needs to
update img->load_failed_p AFTER restoring img, not before it.

> The register used to store img (edi, in my tests) is being modified by
> this call:
> 
>     img->load_failed_p = img->type->load (f, img) == 0;

IIUC, this boils down to calling gif_load (since the image file is
GIF), right?  If so, either the compiler didn't save and restore EDI
around the call due to a bug in the compiler, or perhaps the compiler
assumes something about the registers touched by gif_load or one of
its subroutines (I suspect the DGif* functions from libungif), but
those functions somehow violate that assumption.

Does the code save and restore EDI around the call to img->type->load?
If it did, perhaps the call to img->type->load clobbers the stack
where EDI is saved.  If EDI is not saved, could you trace where does
EDI change inside gif_load?

(FWIW, I think EDI should be saved here, as it is a register
frequently used for keeping variables in optimized code.)

Re: display word wrapping

[Juanma Barranquero]

On 31 May 2004 09:34:34 +0200
Eli Zaretskii <eliz@gnu.org> wrote:

> This probably means that the optimizer is not in itself the direct
> culprit here, as declaring img `volatile' should have prevented the
> compiler from keeping it in a register.

OTOH, I'm more and more convinced it is an optimizer bug...

> However, this observation:
> 
> > If I put
> > 
> >   __asm push img
> >   img->load_failed_p = img->type->load (f, img) == 0;
> >   __asm pop img
> > 
> > (which forces a reload of the edi register from the stored value of img),
> > it works.
> 
> seems to say that img is still in a register, even if declared
> `volatile'.  Could you please look at the machine code near the
> invocation of "img->type->load (f, img)" and see whether `volatile'
> changes something there, and if so, what?

It doesn't affect. Code generated with or without "volatile" is
byte-by-byte identical (I suppose the optimizer's assuming the variable
doesn't need to be really volatile, because it is a stack var whose
address is not taken).

The relevant code, with a (non-volatile) local img, is:

; 1629 :       img->load_failed_p = img->type->load (f, img) == 0;

  04e88 8b 47 38         mov     eax, DWORD PTR [edi+56]
  04e8b 57               push    edi
  04e8c ff 75 08         push    DWORD PTR _f$[ebp]
  04e8f ff 50 08         call    DWORD PTR [eax+8]
  04e92 83 c4 0c         add     esp, 12                        ; 0000000cH
  04e95 f7 d8            neg     eax
  04e97 1b c0            sbb     eax, eax
  04e99 40               inc     eax
  04e9a 89 47 3c         mov     DWORD PTR [edi+60], eax

edi contains img.  Note that it is not reloaded after the call to [eax+8]

The code, for a static (non-volatile) img:

; 1629 :       img->load_failed_p = img->type->load (f, img) == 0;

  04e9f a1 00 00 00 00   mov     eax, DWORD PTR ?img@?1??lookup_image@@9@9
  04ea4 8b 48 38         mov     ecx, DWORD PTR [eax+56]
  04ea7 50               push    eax
  04ea8 ff 75 08         push    DWORD PTR _f$[ebp]
  04eab ff 51 08         call    DWORD PTR [ecx+8]
  04eae 8b 3d 00 00 00
        00               mov     edi, DWORD PTR ?img@?1??lookup_image@@9@9
  04eb4 83 c4 0c         add     esp, 12                        ; 0000000cH
  04eb7 f7 d8            neg     eax
  04eb9 1b c0            sbb     eax, eax
  04ebb 40               inc     eax
  04ebc 89 47 3c         mov     DWORD PTR [edi+60], eax

(the register usage is different, but note the load to edi after the
call...)

> But such a workaround needs
> to be more clever than the push/pop pair above, since it needs to
> update img->load_failed_p AFTER restoring img, not before it.

Yes, I know. I was just testing different local/static situations.

> IIUC, this boils down to calling gif_load (since the image file is
> GIF), right?

Yeah, but it happens with all libraries.

                                                                Juanma

Re: display word wrapping

[Andreas Schwab]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> The relevant code, with a (non-volatile) local img, is:
>
> ; 1629 :       img->load_failed_p = img->type->load (f, img) == 0;
>
>   04e88 8b 47 38         mov     eax, DWORD PTR [edi+56]
>   04e8b 57               push    edi
>   04e8c ff 75 08         push    DWORD PTR _f$[ebp]
>   04e8f ff 50 08         call    DWORD PTR [eax+8]
>   04e92 83 c4 0c         add     esp, 12                        ; 0000000cH
>   04e95 f7 d8            neg     eax
>   04e97 1b c0            sbb     eax, eax
>   04e99 40               inc     eax
>   04e9a 89 47 3c         mov     DWORD PTR [edi+60], eax
>
> edi contains img.  Note that it is not reloaded after the call to [eax+8]

Is edi a call-used register on windows?  It isn't on i386-linux, so the
code would be correct there.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: display word wrapping

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> 
> Is edi a call-used register on windows?

I don't know (anyone?).

Anyway, if gif_load clobbers edi or even simply doesn't push/pop it
(the machine code will show), then we know that this is the cause of
the crash.

Juanma, were the image support libraries compiled with MSVC or with
another compiler, like GCC?  Perhaps there are subtle differences in
the ABI supported by the two compilers.

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 19:41:20 +0200
"Eli Zaretskii" <eliz@gnu.org> wrote:

> Juanma, were the image support libraries compiled with MSVC or with
> another compiler, like GCC?

I'm not 100% sure, but looking at the symbols inside the binary it looks
like GCC 2.95.2 (MinGW).

                                                                Juanma

Re: display word wrapping

[Eli Zaretskii]

> Date: Tue, 01 Jun 2004 09:14:12 +0200
> From: Juanma Barranquero <jmbarranquero@wke.es>
> 
> I'm not 100% sure, but looking at the symbols inside the binary it looks
> like GCC 2.95.2 (MinGW).

Perhaps it's a problem with linking code generated by different
compilers, then.  Or a bug in GCC 2.95.2.

Re: display word wrapping

[Andreas Schwab]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> Date: Tue, 01 Jun 2004 09:14:12 +0200
>> From: Juanma Barranquero <jmbarranquero@wke.es>
>> 
>> I'm not 100% sure, but looking at the symbols inside the binary it looks
>> like GCC 2.95.2 (MinGW).
>
> Perhaps it's a problem with linking code generated by different
> compilers, then.  Or a bug in GCC 2.95.2.

It could also be a buffer overflow bug somewhere in the library that
happens to overwrite the saved value of edi.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: display word wrapping

[Juanma Barranquero]

On Tue, 01 Jun 2004 22:53:19 +0200, Andreas Schwab <schwab@suse.de> wrote:

> It could also be a buffer overflow bug somewhere in the library that
> happens to overwrite the saved value of edi.

When I find the time I'll try to ask some questions on the GnuWin32
lists.

But I confess I'm growing disenchanted with the whole "image
support/GnuWin32 libraries/MSVC build" issue. It *is* unstable, errors
on input data bring Emacs to its knees, and it's all fragile
(configuration- and run-time-wise) beyond belief. I'm starting to think
it'd be good to build reference library binaries with MSVC for the
MSVC-built Emacs, and put them (and sources) somewhere in the net, with
a link from the Emacs docs. I'm glad the GCC build doesn't have the
problems.

Still, I'll try to find the time to add some info to nt/INSTALL and
etc/PROBLEMS before the 21.[45] release.


                                                           /L/e/k/t/u

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> On Tue, 01 Jun 2004 22:53:19 +0200, Andreas Schwab <schwab@suse.de> wrote:
> 
> > It could also be a buffer overflow bug somewhere in the library that
> > happens to overwrite the saved value of edi.
> 
> When I find the time I'll try to ask some questions on the GnuWin32
> lists.
> 
> But I confess I'm growing disenchanted with the whole "image
> support/GnuWin32 libraries/MSVC build" issue. It *is* unstable, errors
> on input data bring Emacs to its knees, and it's all fragile
> (configuration- and run-time-wise) beyond belief. I'm starting to think
> it'd be good to build reference library binaries with MSVC for the
> MSVC-built Emacs, and put them (and sources) somewhere in the net, with
> a link from the Emacs docs. I'm glad the GCC build doesn't have the
> problems.
> 
> Still, I'll try to find the time to add some info to nt/INSTALL and
> etc/PROBLEMS before the 21.[45] release.

Maybe we need a separate nt/PROBLEMS :-|  

And refer explicitly to it from nt/INSTALL.  

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 02 Jun 2004 09:58:08 +0200
storm@cua.dk (Kim F. Storm) wrote:

> Maybe we need a separate nt/PROBLEMS :-|  

Oh, I don't think so. etc/PROBLEMS is generic and already contains some
Windows-specific stuff.

                                                                Juanma

Re: display word wrapping

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Date: Tue, 01 Jun 2004 22:53:19 +0200
> 
> It could also be a buffer overflow bug somewhere in the library that
> happens to overwrite the saved value of edi.

I think Juanma said that this happens with all types of images, so
it's not a bug in a single library.

Re: display word wrapping

[Juanma Barranquero]

On Wed, 02 Jun 2004 06:55:27 +0200
"Eli Zaretskii" <eliz@gnu.org> wrote:

> I think Juanma said that this happens with all types of images, so
> it's not a bug in a single library.

Yes. Without the patch to "protect" img I get a fault on all image types
loaded from a DLL.

                                                                Juanma

Re: display word wrapping

[Eli Zaretskii]

> Date: Mon, 31 May 2004 16:40:41 +0200
> From: Juanma Barranquero <jmbarranquero@wke.es>
> 
> I suppose the optimizer's assuming the variable doesn't need to be
> really volatile, because it is a stack var whose address is not
> taken.

It isn't allowed to assume that.  AFAIK, unlike the `register'
qualifier, `volatile' is not an advisory one, it's mandatory.

But as Andreas points out, the declaration itself is incorrect.  If
so, changing it to whatever is right should make a difference.  Here,
try this:

     struct image * volatile img;

Did that work?

> > But such a workaround needs
> > to be more clever than the push/pop pair above, since it needs to
> > update img->load_failed_p AFTER restoring img, not before it.
> 
> Yes, I know. I was just testing different local/static situations.

So perhaps we should just do it now.

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 19:55:53 +0200, "Eli Zaretskii" <eliz@gnu.org> wrote:

> It isn't allowed to assume that.  AFAIK, unlike the `register'
> qualifier, `volatile' is not an advisory one, it's mandatory.

Yes. But unless I've made some kind of mistake (I'll recheck), the code
generated does not vary.

> But as Andreas points out, the declaration itself is incorrect.  If
> so, changing it to whatever is right should make a difference.  Here,
> try this:
> 
>      struct image * volatile img;
> 
> Did that work?

That's what I really tried, and no, it doesn't make any difference.


                                                           /L/e/k/t/u

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 19:55:53 +0200, "Eli Zaretskii" <eliz@gnu.org> wrote:

[Warning: big listing ahead]

There are the two full listings of the function, with the very same
compiler settings.  If you look at line 1599 on both listings, you'll
see one of them uses

  struct image * img;

while the other has

  struct image * volatile img;

But the resulting code is the same.

However, I've done tests with small programs, and definitely "volatile"
is not ignored on these. So it looks more and more like the optimizer is
making a mess of itself when dealing with lookup_image and it's not
"sure" what to do with img.


                                                           /L/e/k/t/u





---------------------------------------------------------------------------
non-volatile version:
---------------------------------------------------------------------------


_TEXT	SEGMENT
_now$ = -8						; size = 8
_f$ = 8							; size = 4
_spec$ = 12						; size = 4
_lookup_image PROC NEAR

; 1597 : {

  04e1d	55		 push	 ebp
  04e1e	8b ec		 mov	 ebp, esp
  04e20	51		 push	 ecx
  04e21	51		 push	 ecx
  04e22	53		 push	 ebx
  04e23	56		 push	 esi

; 1598 :   struct image_cache *c = FRAME_X_IMAGE_CACHE (f);
; 1599 :   struct image *img;
; 1600 :   int i;
; 1601 :   unsigned hash;
; 1602 :   struct gcpro gcpro1;
; 1603 :   EMACS_TIME now;
; 1604 : 
; 1605 :   /* F must be a window-system frame, and SPEC must be a valid image
; 1606 :      specification.  */
; 1607 :   xassert (FRAME_WINDOW_P (f));
; 1608 :   xassert (valid_image_p (spec));
; 1609 : 
; 1610 :   GCPRO1 (spec);
; 1611 : 
; 1612 :   /* Look up SPEC in the hash table of the image cache.  */
; 1613 :   hash = sxhash (spec, 0);

  04e24	8b 75 0c	 mov	 esi, DWORD PTR _spec$[ebp]
  04e27	57		 push	 edi
  04e28	8b 3d e4 00 00
	00		 mov	 edi, DWORD PTR _one_w32_display_info+228
  04e2e	6a 00		 push	 0
  04e30	56		 push	 esi
  04e31	e8 00 00 00 00	 call	 _sxhash
  04e36	59		 pop	 ecx
  04e37	59		 pop	 ecx

; 1614 :   i = hash % IMAGE_CACHE_BUCKETS_SIZE;

  04e38	33 d2		 xor	 edx, edx
  04e3a	8b d8		 mov	 ebx, eax
  04e3c	b9 e9 03 00 00	 mov	 ecx, 1001		; 000003e9H
  04e41	f7 f1		 div	 ecx

; 1615 : 
; 1616 :   for (img = c->buckets[i]; img; img = img->next)

  04e43	8b 07		 mov	 eax, DWORD PTR [edi]
  04e45	8b 3c 90	 mov	 edi, DWORD PTR [eax+edx*4]
  04e48	85 ff		 test	 edi, edi
  04e4a	74 27		 je	 SHORT $L34653
$L27272:

; 1617 :     if (img->hash == hash && !NILP (Fequal (img->spec, spec)))

  04e4c	39 5f 4c	 cmp	 DWORD PTR [edi+76], ebx
  04e4f	75 13		 jne	 SHORT $L27273
  04e51	56		 push	 esi
  04e52	ff 77 28	 push	 DWORD PTR [edi+40]
  04e55	e8 00 00 00 00	 call	 _Fequal
  04e5a	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qnil
  04e60	59		 pop	 ecx
  04e61	59		 pop	 ecx
  04e62	75 07		 jne	 SHORT $L34654
$L27273:

; 1615 : 
; 1616 :   for (img = c->buckets[i]; img; img = img->next)

  04e64	8b 7f 54	 mov	 edi, DWORD PTR [edi+84]
  04e67	85 ff		 test	 edi, edi
  04e69	75 e1		 jne	 SHORT $L27272
$L34654:

; 1618 :       break;
; 1619 : 
; 1620 :   /* If not found, create a new image and cache it.  */
; 1621 :   if (img == NULL)

  04e6b	85 ff		 test	 edi, edi
  04e6d	0f 85 8a 01 00
	00		 jne	 $L27350
$L34653:

; 1622 :     {
; 1623 :       extern Lisp_Object Qpostscript;
; 1624 : 
; 1625 :       BLOCK_INPUT;

  04e73	ff 05 00 00 00
	00		 inc	 DWORD PTR _interrupt_input_blocked

; 1626 :       img = make_image (spec, hash);

  04e79	53		 push	 ebx
  04e7a	8b de		 mov	 ebx, esi
  04e7c	e8 00 00 00 00	 call	 _make_image
  04e81	8b f8		 mov	 edi, eax

; 1627 :       cache_image (f, img);

  04e83	e8 00 00 00 00	 call	 _cache_image

; 1628 :       //__asm push img
; 1629 :       img->load_failed_p = img->type->load (f, img) == 0;

  04e88	8b 47 38	 mov	 eax, DWORD PTR [edi+56]
  04e8b	57		 push	 edi
  04e8c	ff 75 08	 push	 DWORD PTR _f$[ebp]
  04e8f	ff 50 08	 call	 DWORD PTR [eax+8]
  04e92	83 c4 0c	 add	 esp, 12			; 0000000cH
  04e95	f7 d8		 neg	 eax
  04e97	1b c0		 sbb	 eax, eax
  04e99	40		 inc	 eax
  04e9a	89 47 3c	 mov	 DWORD PTR [edi+60], eax

; 1630 :       //__asm pop img
; 1631 : 
; 1632 :       /* If we can't load the image, and we don't have a width and
; 1633 : 	 height, use some arbitrary width and height so that we can
; 1634 : 	 draw a rectangle for it.  */
; 1635 :       if (img->load_failed_p)
; 1636 : 	{
; 1637 : 	  Lisp_Object value;
; 1638 : 
; 1639 : 	  value = image_spec_value (spec, QCwidth, NULL);

  04e9d	8b ce		 mov	 ecx, esi
  04e9f	6a 00		 push	 0
  04ea1	74 3c		 je	 SHORT $L27279
  04ea3	ff 35 00 00 00
	00		 push	 DWORD PTR _QCwidth
  04ea9	e8 00 00 00 00	 call	 _image_spec_value

; 1640 : 	  img->width = (INTEGERP (value)
; 1641 : 			? XFASTINT (value) : DEFAULT_IMAGE_WIDTH);

  04eae	bb 00 00 00 e0	 mov	 ebx, -536870912		; e0000000H
  04eb3	85 c3		 test	 eax, ebx
  04eb5	59		 pop	 ecx
  04eb6	59		 pop	 ecx
  04eb7	74 03		 je	 SHORT $L34647
  04eb9	6a 1e		 push	 30			; 0000001eH
  04ebb	58		 pop	 eax
$L34647:
  04ebc	89 47 1c	 mov	 DWORD PTR [edi+28], eax

; 1642 : 	  value = image_spec_value (spec, QCheight, NULL);

  04ebf	6a 00		 push	 0
  04ec1	ff 35 00 00 00
	00		 push	 DWORD PTR _QCheight
  04ec7	8b ce		 mov	 ecx, esi
  04ec9	e8 00 00 00 00	 call	 _image_spec_value

; 1643 : 	  img->height = (INTEGERP (value)
; 1644 : 			 ? XFASTINT (value) : DEFAULT_IMAGE_HEIGHT);

  04ece	85 c3		 test	 eax, ebx
  04ed0	59		 pop	 ecx
  04ed1	59		 pop	 ecx
  04ed2	74 03		 je	 SHORT $L34649
  04ed4	6a 1e		 push	 30			; 0000001eH
  04ed6	58		 pop	 eax
$L34649:
  04ed7	89 47 20	 mov	 DWORD PTR [edi+32], eax

; 1645 : 	}
; 1646 :       else

  04eda	e9 fa 00 00 00	 jmp	 $L27348
$L27279:

; 1647 : 	{
; 1648 : 	  /* Handle image type independent image attributes
; 1649 : 	     `:ascent ASCENT', `:margin MARGIN', `:relief RELIEF',
; 1650 : 	     `:background COLOR'.  */
; 1651 : 	  Lisp_Object ascent, margin, relief, bg;
; 1652 : 
; 1653 : 	  ascent = image_spec_value (spec, QCascent, NULL);

  04edf	ff 35 00 00 00
	00		 push	 DWORD PTR _QCascent
  04ee5	e8 00 00 00 00	 call	 _image_spec_value

; 1654 : 	  if (INTEGERP (ascent))

  04eea	bb 00 00 00 e0	 mov	 ebx, -536870912		; e0000000H
  04eef	85 c3		 test	 eax, ebx
  04ef1	59		 pop	 ecx
  04ef2	59		 pop	 ecx
  04ef3	75 05		 jne	 SHORT $L27295

; 1655 : 	    img->ascent = XFASTINT (ascent);

  04ef5	89 47 24	 mov	 DWORD PTR [edi+36], eax

; 1656 : 	  else if (EQ (ascent, Qcenter))

  04ef8	eb 0c		 jmp	 SHORT $L27297
$L27295:
  04efa	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qcenter
  04f00	75 04		 jne	 SHORT $L27297

; 1657 : 	    img->ascent = CENTERED_IMAGE_ASCENT;

  04f02	83 4f 24 ff	 or	 DWORD PTR [edi+36], -1
$L27297:

; 1658 : 
; 1659 : 	  margin = image_spec_value (spec, QCmargin, NULL);

  04f06	6a 00		 push	 0
  04f08	ff 35 00 00 00
	00		 push	 DWORD PTR _QCmargin
  04f0e	8b ce		 mov	 ecx, esi
  04f10	e8 00 00 00 00	 call	 _image_spec_value
  04f15	59		 pop	 ecx
  04f16	59		 pop	 ecx

; 1660 : 	  if (INTEGERP (margin) && XINT (margin) >= 0)

  04f17	8b c8		 mov	 ecx, eax
  04f19	c1 e9 1d	 shr	 ecx, 29			; 0000001dH
  04f1c	75 0c		 jne	 SHORT $L27302
  04f1e	8b d0		 mov	 edx, eax
  04f20	c1 e2 03	 shl	 edx, 3
  04f23	78 05		 js	 SHORT $L27302

; 1661 : 	    img->vmargin = img->hmargin = XFASTINT (margin);

  04f25	89 47 30	 mov	 DWORD PTR [edi+48], eax

; 1662 : 	  else if (CONSP (margin) && INTEGERP (XCAR (margin))

  04f28	eb 2d		 jmp	 SHORT $L34658
$L27302:

; 1663 : 		   && INTEGERP (XCDR (margin)))

  04f2a	83 f9 05	 cmp	 ecx, 5
  04f2d	75 2b		 jne	 SHORT $L27334
  04f2f	25 ff ff ff 1f	 and	 eax, 536870911		; 1fffffffH
  04f34	8b 08		 mov	 ecx, DWORD PTR [eax]
  04f36	85 cb		 test	 ecx, ebx
  04f38	75 20		 jne	 SHORT $L27334
  04f3a	85 58 04	 test	 DWORD PTR [eax+4], ebx
  04f3d	75 1b		 jne	 SHORT $L27334

; 1664 : 	    {
; 1665 : 	      if (XINT (XCAR (margin)) > 0)

  04f3f	8b d1		 mov	 edx, ecx
  04f41	c1 e2 03	 shl	 edx, 3
  04f44	85 d2		 test	 edx, edx
  04f46	7e 03		 jle	 SHORT $L27324

; 1666 : 		img->hmargin = XFASTINT (XCAR (margin));

  04f48	89 4f 30	 mov	 DWORD PTR [edi+48], ecx
$L27324:

; 1667 : 	      if (XINT (XCDR (margin)) > 0)

  04f4b	8b 40 04	 mov	 eax, DWORD PTR [eax+4]
  04f4e	8b c8		 mov	 ecx, eax
  04f50	c1 e1 03	 shl	 ecx, 3
  04f53	85 c9		 test	 ecx, ecx
  04f55	7e 03		 jle	 SHORT $L27334
$L34658:

; 1668 : 		img->vmargin = XFASTINT (XCDR (margin));

  04f57	89 47 34	 mov	 DWORD PTR [edi+52], eax
$L27334:

; 1669 : 	    }
; 1670 : 
; 1671 : 	  relief = image_spec_value (spec, QCrelief, NULL);

  04f5a	6a 00		 push	 0
  04f5c	ff 35 00 00 00
	00		 push	 DWORD PTR _QCrelief
  04f62	8b ce		 mov	 ecx, esi
  04f64	e8 00 00 00 00	 call	 _image_spec_value

; 1672 : 	  if (INTEGERP (relief))

  04f69	85 c3		 test	 eax, ebx
  04f6b	59		 pop	 ecx
  04f6c	59		 pop	 ecx
  04f6d	75 14		 jne	 SHORT $L27342

; 1673 : 	    {
; 1674 : 	      img->relief = XINT (relief);

  04f6f	c1 e0 03	 shl	 eax, 3
  04f72	c1 f8 03	 sar	 eax, 3
  04f75	89 47 2c	 mov	 DWORD PTR [edi+44], eax

; 1675 : 	      img->hmargin += abs (img->relief);

  04f78	99		 cdq
  04f79	33 c2		 xor	 eax, edx
  04f7b	2b c2		 sub	 eax, edx
  04f7d	01 47 30	 add	 DWORD PTR [edi+48], eax

; 1676 : 	      img->vmargin += abs (img->relief);

  04f80	01 47 34	 add	 DWORD PTR [edi+52], eax
$L27342:

; 1677 : 	    }
; 1678 : 
; 1679 : 	  if (! img->background_valid)

  04f83	f6 47 18 02	 test	 BYTE PTR [edi+24], 2
  04f87	75 36		 jne	 SHORT $L34655

; 1680 : 	    {
; 1681 : 	      bg = image_spec_value (img->spec, QCbackground, NULL);

  04f89	8b 4f 28	 mov	 ecx, DWORD PTR [edi+40]
  04f8c	6a 00		 push	 0
  04f8e	ff 35 00 00 00
	00		 push	 DWORD PTR _QCbackground
  04f94	e8 00 00 00 00	 call	 _image_spec_value

; 1682 : 	      if (!NILP (bg))

  04f99	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qnil
  04f9f	59		 pop	 ecx
  04fa0	59		 pop	 ecx
  04fa1	74 1c		 je	 SHORT $L34655

; 1683 : 		{
; 1684 : 		  img->background
; 1685 : 		    = x_alloc_image_color (f, img, bg,
; 1686 : 					   FRAME_BACKGROUND_PIXEL (f));

  04fa3	8b 4d 08	 mov	 ecx, DWORD PTR _f$[ebp]
  04fa6	8b 91 e0 00 00
	00		 mov	 edx, DWORD PTR [ecx+224]
  04fac	ff 32		 push	 DWORD PTR [edx]
  04fae	8b f7		 mov	 esi, edi
  04fb0	51		 push	 ecx
  04fb1	e8 00 00 00 00	 call	 _x_alloc_image_color

; 1687 : 		  img->background_valid = 1;

  04fb6	83 4f 18 02	 or	 DWORD PTR [edi+24], 2
  04fba	59		 pop	 ecx
  04fbb	59		 pop	 ecx
  04fbc	89 47 14	 mov	 DWORD PTR [edi+20], eax
$L34655:

; 1688 : 		}
; 1689 : 	    }
; 1690 : 
; 1691 : 	  /* Do image transformations and compute masks, unless we
; 1692 : 	     don't have the image yet.  */
; 1693 : 	  if (!EQ (*img->type->type, Qpostscript))

  04fbf	8b 47 38	 mov	 eax, DWORD PTR [edi+56]
  04fc2	8b 00		 mov	 eax, DWORD PTR [eax]
  04fc4	8b 00		 mov	 eax, DWORD PTR [eax]
  04fc6	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qpostscript
  04fcc	74 0b		 je	 SHORT $L27348

; 1694 : 	    postprocess_image (f, img);

  04fce	ff 75 08	 push	 DWORD PTR _f$[ebp]
  04fd1	8b f7		 mov	 esi, edi
  04fd3	e8 00 00 00 00	 call	 _postprocess_image
  04fd8	59		 pop	 ecx
$L27348:

; 1695 : 	}
; 1696 : 
; 1697 :       UNBLOCK_INPUT;

  04fd9	ff 0d 00 00 00
	00		 dec	 DWORD PTR _interrupt_input_blocked
  04fdf	75 36		 jne	 SHORT $L27351
  04fe1	83 3d 00 00 00
	00 00		 cmp	 DWORD PTR _interrupt_input_pending, 0
  04fe8	74 05		 je	 SHORT $L27352
  04fea	e8 00 00 00 00	 call	 _reinvoke_input_signal
$L27352:
  04fef	83 3d 00 00 00
	00 00		 cmp	 DWORD PTR _pending_atimers, 0
  04ff6	74 05		 je	 SHORT $L27350
  04ff8	e8 00 00 00 00	 call	 _do_pending_atimers
$L27350:

; 1698 :     }
; 1699 : 
; 1700 :   /* We're using IMG, so set its timestamp to `now'.  */
; 1701 :   EMACS_GET_TIME (now);

  04ffd	8d 45 f8	 lea	 eax, DWORD PTR _now$[ebp]
  05000	6a 00		 push	 0
  05002	50		 push	 eax
  05003	e8 00 00 00 00	 call	 _gettimeofday

; 1702 :   img->timestamp = EMACS_SECS (now);

  05008	8b 45 f8	 mov	 eax, DWORD PTR _now$[ebp]
  0500b	59		 pop	 ecx
  0500c	59		 pop	 ecx
  0500d	89 07		 mov	 DWORD PTR [edi], eax

; 1703 : 
; 1704 :   UNGCPRO;
; 1705 : 
; 1706 :   /* Value is the image id.  */
; 1707 :   return img->id;

  0500f	8b 47 50	 mov	 eax, DWORD PTR [edi+80]
  05012	5f		 pop	 edi
  05013	5e		 pop	 esi
  05014	5b		 pop	 ebx

; 1708 : }

  05015	c9		 leave
  05016	c3		 ret	 0
$L27351:

; 1695 : 	}
; 1696 : 
; 1697 :       UNBLOCK_INPUT;

  05017	83 3d 00 00 00
	00 00		 cmp	 DWORD PTR _interrupt_input_blocked, 0
  0501e	7d dd		 jge	 SHORT $L27350
  05020	e9 00 00 00 00	 jmp	 _w32_abort
_lookup_image ENDP
_TEXT	ENDS



---------------------------------------------------------------------------
volatile version:
---------------------------------------------------------------------------

_TEXT	SEGMENT
_now$ = -8						; size = 8
_f$ = 8							; size = 4
_spec$ = 12						; size = 4
_lookup_image PROC NEAR

; 1597 : {

  04e1d	55		 push	 ebp
  04e1e	8b ec		 mov	 ebp, esp
  04e20	51		 push	 ecx
  04e21	51		 push	 ecx
  04e22	53		 push	 ebx
  04e23	56		 push	 esi

; 1598 :   struct image_cache *c = FRAME_X_IMAGE_CACHE (f);
; 1599 :   struct image * volatile img;
; 1600 :   int i;
; 1601 :   unsigned hash;
; 1602 :   struct gcpro gcpro1;
; 1603 :   EMACS_TIME now;
; 1604 : 
; 1605 :   /* F must be a window-system frame, and SPEC must be a valid image
; 1606 :      specification.  */
; 1607 :   xassert (FRAME_WINDOW_P (f));
; 1608 :   xassert (valid_image_p (spec));
; 1609 : 
; 1610 :   GCPRO1 (spec);
; 1611 : 
; 1612 :   /* Look up SPEC in the hash table of the image cache.  */
; 1613 :   hash = sxhash (spec, 0);

  04e24	8b 75 0c	 mov	 esi, DWORD PTR _spec$[ebp]
  04e27	57		 push	 edi
  04e28	8b 3d e4 00 00
	00		 mov	 edi, DWORD PTR _one_w32_display_info+228
  04e2e	6a 00		 push	 0
  04e30	56		 push	 esi
  04e31	e8 00 00 00 00	 call	 _sxhash
  04e36	59		 pop	 ecx
  04e37	59		 pop	 ecx

; 1614 :   i = hash % IMAGE_CACHE_BUCKETS_SIZE;

  04e38	33 d2		 xor	 edx, edx
  04e3a	8b d8		 mov	 ebx, eax
  04e3c	b9 e9 03 00 00	 mov	 ecx, 1001		; 000003e9H
  04e41	f7 f1		 div	 ecx

; 1615 : 
; 1616 :   for (img = c->buckets[i]; img; img = img->next)

  04e43	8b 07		 mov	 eax, DWORD PTR [edi]
  04e45	8b 3c 90	 mov	 edi, DWORD PTR [eax+edx*4]
  04e48	85 ff		 test	 edi, edi
  04e4a	74 27		 je	 SHORT $L34653
$L27272:

; 1617 :     if (img->hash == hash && !NILP (Fequal (img->spec, spec)))

  04e4c	39 5f 4c	 cmp	 DWORD PTR [edi+76], ebx
  04e4f	75 13		 jne	 SHORT $L27273
  04e51	56		 push	 esi
  04e52	ff 77 28	 push	 DWORD PTR [edi+40]
  04e55	e8 00 00 00 00	 call	 _Fequal
  04e5a	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qnil
  04e60	59		 pop	 ecx
  04e61	59		 pop	 ecx
  04e62	75 07		 jne	 SHORT $L34654
$L27273:

; 1615 : 
; 1616 :   for (img = c->buckets[i]; img; img = img->next)

  04e64	8b 7f 54	 mov	 edi, DWORD PTR [edi+84]
  04e67	85 ff		 test	 edi, edi
  04e69	75 e1		 jne	 SHORT $L27272
$L34654:

; 1618 :       break;
; 1619 : 
; 1620 :   /* If not found, create a new image and cache it.  */
; 1621 :   if (img == NULL)

  04e6b	85 ff		 test	 edi, edi
  04e6d	0f 85 8a 01 00
	00		 jne	 $L27350
$L34653:

; 1622 :     {
; 1623 :       extern Lisp_Object Qpostscript;
; 1624 : 
; 1625 :       BLOCK_INPUT;

  04e73	ff 05 00 00 00
	00		 inc	 DWORD PTR _interrupt_input_blocked

; 1626 :       img = make_image (spec, hash);

  04e79	53		 push	 ebx
  04e7a	8b de		 mov	 ebx, esi
  04e7c	e8 00 00 00 00	 call	 _make_image
  04e81	8b f8		 mov	 edi, eax

; 1627 :       cache_image (f, img);

  04e83	e8 00 00 00 00	 call	 _cache_image

; 1628 :       //__asm push img
; 1629 :       img->load_failed_p = img->type->load (f, img) == 0;

  04e88	8b 47 38	 mov	 eax, DWORD PTR [edi+56]
  04e8b	57		 push	 edi
  04e8c	ff 75 08	 push	 DWORD PTR _f$[ebp]
  04e8f	ff 50 08	 call	 DWORD PTR [eax+8]
  04e92	83 c4 0c	 add	 esp, 12			; 0000000cH
  04e95	f7 d8		 neg	 eax
  04e97	1b c0		 sbb	 eax, eax
  04e99	40		 inc	 eax
  04e9a	89 47 3c	 mov	 DWORD PTR [edi+60], eax

; 1630 :       //__asm pop img
; 1631 : 
; 1632 :       /* If we can't load the image, and we don't have a width and
; 1633 : 	 height, use some arbitrary width and height so that we can
; 1634 : 	 draw a rectangle for it.  */
; 1635 :       if (img->load_failed_p)
; 1636 : 	{
; 1637 : 	  Lisp_Object value;
; 1638 : 
; 1639 : 	  value = image_spec_value (spec, QCwidth, NULL);

  04e9d	8b ce		 mov	 ecx, esi
  04e9f	6a 00		 push	 0
  04ea1	74 3c		 je	 SHORT $L27279
  04ea3	ff 35 00 00 00
	00		 push	 DWORD PTR _QCwidth
  04ea9	e8 00 00 00 00	 call	 _image_spec_value

; 1640 : 	  img->width = (INTEGERP (value)
; 1641 : 			? XFASTINT (value) : DEFAULT_IMAGE_WIDTH);

  04eae	bb 00 00 00 e0	 mov	 ebx, -536870912		; e0000000H
  04eb3	85 c3		 test	 eax, ebx
  04eb5	59		 pop	 ecx
  04eb6	59		 pop	 ecx
  04eb7	74 03		 je	 SHORT $L34647
  04eb9	6a 1e		 push	 30			; 0000001eH
  04ebb	58		 pop	 eax
$L34647:
  04ebc	89 47 1c	 mov	 DWORD PTR [edi+28], eax

; 1642 : 	  value = image_spec_value (spec, QCheight, NULL);

  04ebf	6a 00		 push	 0
  04ec1	ff 35 00 00 00
	00		 push	 DWORD PTR _QCheight
  04ec7	8b ce		 mov	 ecx, esi
  04ec9	e8 00 00 00 00	 call	 _image_spec_value

; 1643 : 	  img->height = (INTEGERP (value)
; 1644 : 			 ? XFASTINT (value) : DEFAULT_IMAGE_HEIGHT);

  04ece	85 c3		 test	 eax, ebx
  04ed0	59		 pop	 ecx
  04ed1	59		 pop	 ecx
  04ed2	74 03		 je	 SHORT $L34649
  04ed4	6a 1e		 push	 30			; 0000001eH
  04ed6	58		 pop	 eax
$L34649:
  04ed7	89 47 20	 mov	 DWORD PTR [edi+32], eax

; 1645 : 	}
; 1646 :       else

  04eda	e9 fa 00 00 00	 jmp	 $L27348
$L27279:

; 1647 : 	{
; 1648 : 	  /* Handle image type independent image attributes
; 1649 : 	     `:ascent ASCENT', `:margin MARGIN', `:relief RELIEF',
; 1650 : 	     `:background COLOR'.  */
; 1651 : 	  Lisp_Object ascent, margin, relief, bg;
; 1652 : 
; 1653 : 	  ascent = image_spec_value (spec, QCascent, NULL);

  04edf	ff 35 00 00 00
	00		 push	 DWORD PTR _QCascent
  04ee5	e8 00 00 00 00	 call	 _image_spec_value

; 1654 : 	  if (INTEGERP (ascent))

  04eea	bb 00 00 00 e0	 mov	 ebx, -536870912		; e0000000H
  04eef	85 c3		 test	 eax, ebx
  04ef1	59		 pop	 ecx
  04ef2	59		 pop	 ecx
  04ef3	75 05		 jne	 SHORT $L27295

; 1655 : 	    img->ascent = XFASTINT (ascent);

  04ef5	89 47 24	 mov	 DWORD PTR [edi+36], eax

; 1656 : 	  else if (EQ (ascent, Qcenter))

  04ef8	eb 0c		 jmp	 SHORT $L27297
$L27295:
  04efa	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qcenter
  04f00	75 04		 jne	 SHORT $L27297

; 1657 : 	    img->ascent = CENTERED_IMAGE_ASCENT;

  04f02	83 4f 24 ff	 or	 DWORD PTR [edi+36], -1
$L27297:

; 1658 : 
; 1659 : 	  margin = image_spec_value (spec, QCmargin, NULL);

  04f06	6a 00		 push	 0
  04f08	ff 35 00 00 00
	00		 push	 DWORD PTR _QCmargin
  04f0e	8b ce		 mov	 ecx, esi
  04f10	e8 00 00 00 00	 call	 _image_spec_value
  04f15	59		 pop	 ecx
  04f16	59		 pop	 ecx

; 1660 : 	  if (INTEGERP (margin) && XINT (margin) >= 0)

  04f17	8b c8		 mov	 ecx, eax
  04f19	c1 e9 1d	 shr	 ecx, 29			; 0000001dH
  04f1c	75 0c		 jne	 SHORT $L27302
  04f1e	8b d0		 mov	 edx, eax
  04f20	c1 e2 03	 shl	 edx, 3
  04f23	78 05		 js	 SHORT $L27302

; 1661 : 	    img->vmargin = img->hmargin = XFASTINT (margin);

  04f25	89 47 30	 mov	 DWORD PTR [edi+48], eax

; 1662 : 	  else if (CONSP (margin) && INTEGERP (XCAR (margin))

  04f28	eb 2d		 jmp	 SHORT $L34658
$L27302:

; 1663 : 		   && INTEGERP (XCDR (margin)))

  04f2a	83 f9 05	 cmp	 ecx, 5
  04f2d	75 2b		 jne	 SHORT $L27334
  04f2f	25 ff ff ff 1f	 and	 eax, 536870911		; 1fffffffH
  04f34	8b 08		 mov	 ecx, DWORD PTR [eax]
  04f36	85 cb		 test	 ecx, ebx
  04f38	75 20		 jne	 SHORT $L27334
  04f3a	85 58 04	 test	 DWORD PTR [eax+4], ebx
  04f3d	75 1b		 jne	 SHORT $L27334

; 1664 : 	    {
; 1665 : 	      if (XINT (XCAR (margin)) > 0)

  04f3f	8b d1		 mov	 edx, ecx
  04f41	c1 e2 03	 shl	 edx, 3
  04f44	85 d2		 test	 edx, edx
  04f46	7e 03		 jle	 SHORT $L27324

; 1666 : 		img->hmargin = XFASTINT (XCAR (margin));

  04f48	89 4f 30	 mov	 DWORD PTR [edi+48], ecx
$L27324:

; 1667 : 	      if (XINT (XCDR (margin)) > 0)

  04f4b	8b 40 04	 mov	 eax, DWORD PTR [eax+4]
  04f4e	8b c8		 mov	 ecx, eax
  04f50	c1 e1 03	 shl	 ecx, 3
  04f53	85 c9		 test	 ecx, ecx
  04f55	7e 03		 jle	 SHORT $L27334
$L34658:

; 1668 : 		img->vmargin = XFASTINT (XCDR (margin));

  04f57	89 47 34	 mov	 DWORD PTR [edi+52], eax
$L27334:

; 1669 : 	    }
; 1670 : 
; 1671 : 	  relief = image_spec_value (spec, QCrelief, NULL);

  04f5a	6a 00		 push	 0
  04f5c	ff 35 00 00 00
	00		 push	 DWORD PTR _QCrelief
  04f62	8b ce		 mov	 ecx, esi
  04f64	e8 00 00 00 00	 call	 _image_spec_value

; 1672 : 	  if (INTEGERP (relief))

  04f69	85 c3		 test	 eax, ebx
  04f6b	59		 pop	 ecx
  04f6c	59		 pop	 ecx
  04f6d	75 14		 jne	 SHORT $L27342

; 1673 : 	    {
; 1674 : 	      img->relief = XINT (relief);

  04f6f	c1 e0 03	 shl	 eax, 3
  04f72	c1 f8 03	 sar	 eax, 3
  04f75	89 47 2c	 mov	 DWORD PTR [edi+44], eax

; 1675 : 	      img->hmargin += abs (img->relief);

  04f78	99		 cdq
  04f79	33 c2		 xor	 eax, edx
  04f7b	2b c2		 sub	 eax, edx
  04f7d	01 47 30	 add	 DWORD PTR [edi+48], eax

; 1676 : 	      img->vmargin += abs (img->relief);

  04f80	01 47 34	 add	 DWORD PTR [edi+52], eax
$L27342:

; 1677 : 	    }
; 1678 : 
; 1679 : 	  if (! img->background_valid)

  04f83	f6 47 18 02	 test	 BYTE PTR [edi+24], 2
  04f87	75 36		 jne	 SHORT $L34655

; 1680 : 	    {
; 1681 : 	      bg = image_spec_value (img->spec, QCbackground, NULL);

  04f89	8b 4f 28	 mov	 ecx, DWORD PTR [edi+40]
  04f8c	6a 00		 push	 0
  04f8e	ff 35 00 00 00
	00		 push	 DWORD PTR _QCbackground
  04f94	e8 00 00 00 00	 call	 _image_spec_value

; 1682 : 	      if (!NILP (bg))

  04f99	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qnil
  04f9f	59		 pop	 ecx
  04fa0	59		 pop	 ecx
  04fa1	74 1c		 je	 SHORT $L34655

; 1683 : 		{
; 1684 : 		  img->background
; 1685 : 		    = x_alloc_image_color (f, img, bg,
; 1686 : 					   FRAME_BACKGROUND_PIXEL (f));

  04fa3	8b 4d 08	 mov	 ecx, DWORD PTR _f$[ebp]
  04fa6	8b 91 e0 00 00
	00		 mov	 edx, DWORD PTR [ecx+224]
  04fac	ff 32		 push	 DWORD PTR [edx]
  04fae	8b f7		 mov	 esi, edi
  04fb0	51		 push	 ecx
  04fb1	e8 00 00 00 00	 call	 _x_alloc_image_color

; 1687 : 		  img->background_valid = 1;

  04fb6	83 4f 18 02	 or	 DWORD PTR [edi+24], 2
  04fba	59		 pop	 ecx
  04fbb	59		 pop	 ecx
  04fbc	89 47 14	 mov	 DWORD PTR [edi+20], eax
$L34655:

; 1688 : 		}
; 1689 : 	    }
; 1690 : 
; 1691 : 	  /* Do image transformations and compute masks, unless we
; 1692 : 	     don't have the image yet.  */
; 1693 : 	  if (!EQ (*img->type->type, Qpostscript))

  04fbf	8b 47 38	 mov	 eax, DWORD PTR [edi+56]
  04fc2	8b 00		 mov	 eax, DWORD PTR [eax]
  04fc4	8b 00		 mov	 eax, DWORD PTR [eax]
  04fc6	3b 05 00 00 00
	00		 cmp	 eax, DWORD PTR _Qpostscript
  04fcc	74 0b		 je	 SHORT $L27348

; 1694 : 	    postprocess_image (f, img);

  04fce	ff 75 08	 push	 DWORD PTR _f$[ebp]
  04fd1	8b f7		 mov	 esi, edi
  04fd3	e8 00 00 00 00	 call	 _postprocess_image
  04fd8	59		 pop	 ecx
$L27348:

; 1695 : 	}
; 1696 : 
; 1697 :       UNBLOCK_INPUT;

  04fd9	ff 0d 00 00 00
	00		 dec	 DWORD PTR _interrupt_input_blocked
  04fdf	75 36		 jne	 SHORT $L27351
  04fe1	83 3d 00 00 00
	00 00		 cmp	 DWORD PTR _interrupt_input_pending, 0
  04fe8	74 05		 je	 SHORT $L27352
  04fea	e8 00 00 00 00	 call	 _reinvoke_input_signal
$L27352:
  04fef	83 3d 00 00 00
	00 00		 cmp	 DWORD PTR _pending_atimers, 0
  04ff6	74 05		 je	 SHORT $L27350
  04ff8	e8 00 00 00 00	 call	 _do_pending_atimers
$L27350:

; 1698 :     }
; 1699 : 
; 1700 :   /* We're using IMG, so set its timestamp to `now'.  */
; 1701 :   EMACS_GET_TIME (now);

  04ffd	8d 45 f8	 lea	 eax, DWORD PTR _now$[ebp]
  05000	6a 00		 push	 0
  05002	50		 push	 eax
  05003	e8 00 00 00 00	 call	 _gettimeofday

; 1702 :   img->timestamp = EMACS_SECS (now);

  05008	8b 45 f8	 mov	 eax, DWORD PTR _now$[ebp]
  0500b	59		 pop	 ecx
  0500c	59		 pop	 ecx
  0500d	89 07		 mov	 DWORD PTR [edi], eax

; 1703 : 
; 1704 :   UNGCPRO;
; 1705 : 
; 1706 :   /* Value is the image id.  */
; 1707 :   return img->id;

  0500f	8b 47 50	 mov	 eax, DWORD PTR [edi+80]
  05012	5f		 pop	 edi
  05013	5e		 pop	 esi
  05014	5b		 pop	 ebx

; 1708 : }

  05015	c9		 leave
  05016	c3		 ret	 0
$L27351:

; 1695 : 	}
; 1696 : 
; 1697 :       UNBLOCK_INPUT;

  05017	83 3d 00 00 00
	00 00		 cmp	 DWORD PTR _interrupt_input_blocked, 0
  0501e	7d dd		 jge	 SHORT $L27350
  05020	e9 00 00 00 00	 jmp	 _w32_abort
_lookup_image ENDP
_TEXT	ENDS

Re: display word wrapping

[Andreas Schwab]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> On Mon, 31 May 2004 19:55:53 +0200, "Eli Zaretskii" <eliz@gnu.org> wrote:
>
> [Warning: big listing ahead]
>
> There are the two full listings of the function, with the very same
> compiler settings.  If you look at line 1599 on both listings, you'll
> see one of them uses

I think you are barking up the wrong tree.  You should rather look the the
image loading functions that are indirectly called by lookup_image.  The
compiler is correct in not reloading edi across function calls because it
is a call-saved register.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 22:19:22 +0200, Andreas Schwab <schwab@suse.de> wrote:

> I think you are barking up the wrong tree.

You're probably right :)

> You should rather look the the
> image loading functions that are indirectly called by lookup_image.

Yeah, but the whole point of the exercise is being able to call
the precompiled binary libraries from an MSVC build of Emacs. If the
libraries are wrong, I'll have to work around the issue. I don't want to
be forced to make and distribute "correct" versions of the libs (and
source :)

                                                           /L/e/k/t/u

Re: display word wrapping

[Miles Bader]

On Mon, May 31, 2004 at 07:55:53PM +0200, Eli Zaretskii wrote:
> It isn't allowed to assume that.  AFAIK, unlike the `register'
> qualifier, `volatile' is not an advisory one, it's mandatory.

IIRC, the _meaning_ of `volatile' is only described vaguely in the standard
(so much so that many people consider it useless, e.g., I think Linus refuses
to allow it).  I wouldn't depend too much on it....

-Miles
-- 
`...the Soviet Union was sliding in to an economic collapse so comprehensive
 that in the end its factories produced not goods but bads: finished products
 less valuable than the raw materials they were made from.'  [The Economist]

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 15:18:28 -0400, Miles Bader <miles@gnu.org> wrote:

> I wouldn't depend too much on it....

And, as a workaround for the current problem, it isn't even working.


                                                           /L/e/k/t/u

Re: display word wrapping

[Jason Rumney]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> It doesn't affect. Code generated with or without "volatile" is
> byte-by-byte identical (I suppose the optimizer's assuming the variable
> doesn't need to be really volatile, because it is a stack var whose
> address is not taken).

Yes, MSVC is notorious for ignoring directives like this.

Re: display word wrapping

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Mon, 31 May 2004 02:40:19 +0200
>> From: Juanma Barranquero <lektu@mi.madritel.es>
>> 
>> > How exactly is img->spec invalid?  Can you post the details (xtype
>> > etc.)?
>> 
>> Basically, img points to nowhere, so img->spec is some random value.
>
> Ah, yes, I forgot that Windows doesn't Page Fault on NULL pointer
> dereferences, but instead fetches some random garbage...
>
>> > If you change it to
>> > 
>> >    volatile struct image *img;
>> > 
>> > does the problem go away as well?
>> 
>> No.
>
> This probably means that the optimizer is not in itself the direct
> culprit here, as declaring img `volatile' should have prevented the
> compiler from keeping it in a register.

The use of volatile as above does not mark img as volatile, only the
memory it points to.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 17:27:55 +0200, Andreas Schwab <schwab@suse.de> wrote:

> The use of volatile as above does not mark img as volatile, only the
> memory it points to.

Yes, I know. I tried with

  struct image * volatile img;

but the generated code does not change.

                                                           /L/e/k/t/u

Re: display word wrapping

[Jason Rumney]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> On optimized code, img is not on the stack, but on a register.

Could it be that the register state is not fully saved before a
library call (or restored after)?

Re: display word wrapping

[Juanma Barranquero]

On Mon, 31 May 2004 20:05:07 +0100, Jason Rumney <jasonr@gnu.org> wrote:

> Could it be that the register state is not fully saved before a
> library call (or restored after)?

It is the case, yes.

Andreas Schwab says that the callee should preserve edi, and after a
look at the relevant MSVC docs I tend to agree. The GnuWin32 libraries
seem to be compiled with GCC 2.95.2 from MinGW, I
think. I don't know whether MinGW's GCC follows the MSVC calling
conventions or not; it seems they don't.

Certainly I don't want to go the route of providing MSVC builds of
libungif, tiff, libXpm and libjpeg.

There are workarounds; in the case of lookup_image, just making the
variable "static" works (and it's a bit less ugly than interspersing
_asm calls or whatnot). I haven't yet researched what happens with
png_read_from_memory, but it's small, so there's no problem on
de-optimizing it.

So, unless someone has a better idea, I propose the attached patch.

                                                           /L/e/k/t/u



Index: image.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/image.c,v
retrieving revision 1.11
diff -u -2 -r1.11 image.c
--- image.c	12 May 2004 02:23:37 -0000	1.11
+++ image.c	31 May 2004 22:14:27 -0000
@@ -1597,4 +1597,9 @@
 {
   struct image_cache *c = FRAME_X_IMAGE_CACHE (f);
+#ifdef _MSC_VER
+  /* Work around a problem with MinGW builds of graphic libraries
+     not honoring calling conventions */
+  static
+#endif
   struct image *img;
   int i;
@@ -5689,4 +5695,8 @@
    bytes from the input to DATA.  */
 
+#ifdef _MSC_VER
+#pragma optimize("g", off)
+#endif
+
 static void
 png_read_from_memory (png_ptr, data, length)
@@ -5705,4 +5715,8 @@
 }
 
+#ifdef _MSC_VER
+#pragma optimize("", on)
+#endif
+
 /* Load PNG image IMG for use on frame F.  Value is non-zero if
    successful.  */

Re: display word wrapping

[Eli Zaretskii]

> Date: Tue, 01 Jun 2004 00:19:14 +0200
> From: Juanma Barranquero <lektu@mi.madritel.es>
> 
> There are workarounds; in the case of lookup_image, just making the
> variable "static" works (and it's a bit less ugly than interspersing
> _asm calls or whatnot). I haven't yet researched what happens with
> png_read_from_memory, but it's small, so there's no problem on
> de-optimizing it.
> 
> So, unless someone has a better idea, I propose the attached patch.

This change looks good to me, but I suggest to put a comment where
you turn off optimizations explaining why it is being done (it could
be the same text as where you use `static').

Re: display word wrapping

[Juanma Barranquero]

On Tue, 01 Jun 2004 06:43:01 +0200
"Eli Zaretskii" <eliz@gnu.org> wrote:

> This change looks good to me, but I suggest to put a comment where
> you turn off optimizations explaining why it is being done (it could
> be the same text as where you use `static').

Will do.

Thanks,

                                                                Juanma

Re: display word wrapping

[Benjamin Riefenstahl]

Hi Juanma,

Juanma Barranquero <jmbarranquero@wke.es> writes:
> the current method of trying several DLL names [...] just switching
> DLLs is *not* likely to work on Windows.  [...] suppling our own
> (not necessarily compiled by us), [...]

Would it make sense to look at the GIMP project for ideas?  Maybe the
two projects could pool resources (or at least experiences).

Just a thought,
benny

Re: display word wrapping

[Juanma Barranquero]

On Fri, 28 May 2004 11:15:51 +0200
Benjamin Riefenstahl <Benjamin.Riefenstahl@epost.de> wrote:

> Would it make sense to look at the GIMP project for ideas? Maybe the
> two projects could pool resources (or at least experiences).

Yes, that's a good idea.

I don't know whether I'll have the time to look into it (I know next no
nothing about GIMP, apart of what it is), but I'll try.

                                                                Juanma

Re: display word wrapping

[Eli Zaretskii]

> Date: Thu, 27 May 2004 13:15:39 +0200
> From: Juanma Barranquero <jmbarranquero@wke.es>
> 
> That's why I would advocate suppling our own (not necessarily
> compiled by us), either on the binary tarball, or as a .zip pointed from
> somewhere in http://www.gnu.org/software/emacs/windows/ntemacs.html.

We cannot distribute anything without a source: it's against the GPL.

But we could have a list of recommended libraries in nt/INSTALL,
including the URLs where to find them.  (We already have something
similar in INSTALL for Unix versions of those libraries.)  We could
also have an entry in PROBLEMS with a description of possible trouble
if people use DLLs other than what we recommend.

Re: display word wrapping

[Juanma Barranquero]

On Fri, 28 May 2004 11:24:22 +0200
"Eli Zaretskii" <eliz@gnu.org> wrote:

> We cannot distribute anything without a source: it's against the GPL.

I didn't say anything about *not* distributing the sources :)

But anyway, it was just an idea to simplify life for Windows users. So
what you say

> But we could have a list of recommended libraries in nt/INSTALL,
> including the URLs where to find them.

is fine, *if* we can find binary libraries to recommend. I was thinking
more of situations where we would be shipping Emacs on Windows with our
own build of the libraries. That's what many projects do with
libeay32/ssleay32, for example.

> We could
> also have an entry in PROBLEMS with a description of possible trouble
> if people use DLLs other than what we recommend.

Yes, after we've sorted out the mess we definitely need to add more info
to etc/PROBLEMS and nt/INSTALL.

                                                                Juanma

Re: display word wrapping

[Richard Stallman]

    We cannot distribute anything without a source: it's against the GPL.

It's worse than that; it's against our basic principles.
Even if the GPL allowed it, we would not do it.

Note that linking Emacs with libraries not available in source form
and distributing the linked executable also violates the GPL except
in certain special cases (libraries distribute with the kernel or
compiler).

Re: display word wrapping

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Fri, 28 May 2004 21:43:46 -0400
> 
>     We cannot distribute anything without a source: it's against the GPL.
> 
> It's worse than that; it's against our basic principles.

Right.

> Note that linking Emacs with libraries not available in source form
> and distributing the linked executable also violates the GPL

That's not a problem in this case: all the libraries are Free Software
(AFAIK, they are Windows ports of the same libraries used for image
support on GNU and Unix systems).

Re: display word wrapping

[Juanma Barranquero]

On Fri, 28 May 2004 21:43:46 -0400, Richard Stallman <rms@gnu.org> wrote:

> It's worse than that; it's against our basic principles.
> Even if the GPL allowed it, we would not do it.

Just for the record: I wasn't suggesting that.  I suggested making
available to Windows users the graphics libraries in a binary tarball,
but that doesn't imply *not* making available the source, too.  AFAIK,
we distribute binary tarballs of Emacs, separate from the source
tarballs.

> Note that linking Emacs with libraries not available in source form
> and distributing the linked executable also violates the GPL except
> in certain special cases (libraries distribute with the kernel or
> compiler).

The graphics libraries on Windows are DLLs.  They not only aren't
statically linked, they are not even dynamically linked in the
traditional sense.  Even after Emacs is configured to use them, it does
not depend on them existing at all; it "discovers" whether they're at
hand or not (by trying to load them) and uses the ones it can find.  My
head *hurts* thinking about the legal status of this :)

But anyway, the point is moot.  We'll describe in the docs how to build
Emacs on Windows, which are the recommended libraries and where to find
them.


                                                           /L/e/k/t/u

Re: display word wrapping

[Richard Stallman]

    > Note that linking Emacs with libraries not available in source form
    > and distributing the linked executable also violates the GPL except
    > in certain special cases (libraries distribute with the kernel or
    > compiler).

    The graphics libraries on Windows are DLLs.  They not only aren't
    statically linked, they are not even dynamically linked in the
    traditional sense.

Our position is that when a program is designed to use certain
specific libraries, it has been combined with them, regardless
of whether the linking is static or dynamic.

In other words, we won't let people get around the GPL merely
by putting code into DLLs.

Re: display word wrapping

[Juanma Barranquero]

On Sun, 30 May 2004 10:30:46 -0400, Richard Stallman <rms@gnu.org> wrote:

> Our position is that when a program is designed to use certain
> specific libraries, it has been combined with them, regardless
> of whether the linking is static or dynamic.

I know.

What do you want to do? I can send you the licenses included in the
source of the libraries (I can look at them, but I'm not going to try to
understand Legalese English, thankyouverymuch).

                                                           /L/e/k/t/u

Re: display word wrapping

[Richard Stallman]

    What do you want to do? I can send you the licenses included in the
    source of the libraries (I can look at them, but I'm not going to try to
    understand Legalese English, thankyouverymuch).

If someone already checked these libraries' licenses when we started
using them on GNU/Linux, we don't need to check them again now.

Re: display word wrapping

[Jason Rumney]

Richard Stallman <rms@gnu.org> writes:

> Our position is that when a program is designed to use certain
> specific libraries, it has been combined with them, regardless
> of whether the linking is static or dynamic.
>
> In other words, we won't let people get around the GPL merely
> by putting code into DLLs.

We are not trying to get around the GPL, the libraries are all
Free. But this does raise an interesting question. If we distribute a
binary version of Emacs that can use these Free libraries if they are
installed on the user's system, but does not depend on them being
present, and does not include the libraries by default, then is it
OUR responsibility to distribute the source for those libraries, or
can we tell the user that if they install those libraries, they can
get the source from the same place they got the library?

My idea when designing the dynamic loading for image libraries, was
that we do not want to distribute non-GNU software from ftp.gnu.org.
So I wanted to make Emacs not depend on those libraries, but I wanted
the binary distribution to be as useful as possible to users. If this
is not possible, then perhaps we should change the code to statically
link the libraries, and distribute an Emacs that has only XBM/PPM
support, and leave it to others to provide binaries capable of
displaying the other formats.

Re: display word wrapping

[Richard Stallman]

    We are not trying to get around the GPL, the libraries are all
    Free. But this does raise an interesting question. If we distribute a
    binary version of Emacs that can use these Free libraries if they are
    installed on the user's system, but does not depend on them being
    present, and does not include the libraries by default, then is it
    OUR responsibility to distribute the source for those libraries, or
    can we tell the user that if they install those libraries, they can
    get the source from the same place they got the library?

If we don't distribute the binaries for these libraries, I think
we do not have to distribute the source either.  But that is an
interesting question, and I will discuss it with Eben Moglen
in regard to clarifying the GPL in the next version.

Re: display word wrapping

[David Kastrup]

"Eli Zaretskii" <eliz@gnu.org> writes:

> > Date: Thu, 27 May 2004 13:15:39 +0200
> > From: Juanma Barranquero <jmbarranquero@wke.es>
> > 
> > That's why I would advocate suppling our own (not necessarily
> > compiled by us), either on the binary tarball, or as a .zip pointed from
> > somewhere in http://www.gnu.org/software/emacs/windows/ntemacs.html.
> 
> We cannot distribute anything without a source: it's against the GPL.

GPL, item 3:

If distribution of executable or object code is made by offering
access to copy from a designated place, then offering equivalent
access to copy the source code from the same place counts as
distribution of the source code, even though third parties are not
compelled to copy the source along with the object code.


-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

W32 image support (was Re: display word wrapping)

[Jason Rumney]

storm@cua.dk (Kim F. Storm) writes:

> Juanma Barranquero <jmbarranquero@wke.es> writes:

Looking back through this extensive thread which seems to be covering
all manner of subjects now, I don't seem to have received Juanma's
message, so I don't know how to respond to the following...

>> IMO the image support on Windows is still shaky. Current problems with
>> that (at least, these are the problems I'm experiencing):
>
> It certainly sounds messy, and quite tricky to do right for
> making a binary distribution for W32!
>
> Would some W32 experts step forward to look into this?

Re: display word wrapping

[Peter Lee]

>>>> Juanma Barranquero writes:

    Juanma> On 26 May 2004 17:06:57 +0200
    Juanma> storm@cua.dk (Kim F. Storm) wrote:

    Juanma> IMO the image support on Windows is still shaky. Current
    Juanma> problems with that (at least, these are the problems I'm
    Juanma> experiencing):

    Juanma>  - There's no single, good, Windows version of the
    Juanma> relevant libraries, so we must try to load each library
    Juanma> with several names, hoping for the best (for example, we
    Juanma> try to load libjpeg.dll, jpeg-62.dll and jpeg.dll, in that
    Juanma> order). That seems fragile.

Indeed.  It took some digging before I discovered the LoadLibrary
calls in image.c.

    Juanma>  - The best-so-far libraries (that I know of) are the
    Juanma> GnuWin32 ones, but (relatively) recent upgrades in some of
    Juanma> them changed some names, so if you try to use them you end
    Juanma> loading several, differently named, zlib DLLs, for
    Juanma> example.

After getting the latest version of the image libraries from gnuwin32,
I had to rename the following files:

jpeg62.dll to jpeg.dll
libtiff3.dll to libtiff.dll
libXpm-noX4.dll to libxpm.dll
zlib1.dll to zlib.dll

    Juanma>  - On MSVC optimized builds, several libraries (most
    Juanma> notably the TIFF one) crash on loading an image. I've
    Juanma> tried to debug it several times, with no success; I'm not
    Juanma> even sure whether is a real Emacs bug hidden somewhere, or
    Juanma> a bug in the MSVC optimizer. Worse, with newest GnuWin32
    Juanma> releases the problem affect more libraries, and some of
    Juanma> them (libpng) don't seem to work at all.

I had this same problem.  I was never able to view images with msvc
build.  At best it would just show an empty box, but usually
crashed.  I always suspected msvc optimization, but I never tried a
non-optimized build.  I just started using mingw.

    Juanma> I'm not entirely sure what to do. Perhaps we could compile
    Juanma> our own graphics libs and distribute them with Emacs,
    Juanma> though that's quite a lot of additional work. But as I
    Juanma> stands, I'd add a section to nt/INSTALL saying: "Don't
    Juanma> compile with image support unless you know what you're
    Juanma> doing and are willing to do some debugging."

I think it would be enough to put a link to gnuwin32 and explain what
libraries are needed for image support.  Which are:
  zlib
  xpm
  libtiff
  libjpeg
  libpng
  libungif
  liburt (needed by ungif)
  fdlibm (needed by ungif)
  libgw32c (needed by ungif)

I'd also note in the nt/INSTALL that the dll's are expected to have
certain names and list those names for each required library.  Then
just explain to the user that after downloading the image binaries
they may need to rename some dll's as well as making sure those dll's
are in the path.

With mingw gcc build and the latest image libs from gnuwin32, I'm
able to view all images supported and have had no real problems.

Re: display word wrapping

[Jason Rumney]

Peter,

Thanks for your valuable feedback.

> I'd also note in the nt/INSTALL that the dll's are expected to have
> certain names and list those names for each required library.

I think we should try to load all the names that we know of, and
request the gnuwin32 maintainers to stop changing the names of their
shared libraries with every release.

Re: display word wrapping

[Juanma Barranquero]

On Fri, 28 May 2004 10:54:39 -0500, Peter Lee <pete_lee@swbell.net> wrote:

> After getting the latest version of the image libraries from gnuwin32,
> I had to rename the following files:
> 
> jpeg62.dll to jpeg.dll
> libtiff3.dll to libtiff.dll
> libXpm-noX4.dll to libxpm.dll
> zlib1.dll to zlib.dll

Well, I prefer to change image.c so it looks also for zlib1.dll and
jpeg62.dll. This way you don't need to have both zlib.dll and zlib1.dll
(which you'll need, because some libraries load zlib1.dll, like
libpng12.dll), and the same for jpeg/jpeg62/jpeg-62.

> I always suspected msvc optimization, but I never tried a
> non-optimized build.  I just started using mingw.

On a non-optimized build I can see every test image I have, except one
called quad-jpeg.tif that (I think) contains an embedded jpeg (it's only
24K, I can send it to you if you want to try with it on gcc builds).
That one causes a crash.  OTOH I can see normal jpeg's just fine.

> I think it would be enough to put a link to gnuwin32 and explain what
> libraries are needed for image support.

We also need to explain what include files must be used, because some
libraries have VC-specific includes (I must use
src\jpeg\6b\jpeg-6b\jconfig.vc as jconfig.h to get libjpeg to work, for
example).

>   liburt (needed by ungif)
>   fdlibm (needed by ungif)
>   libgw32c (needed by ungif)

Which ungif are you using? The one I've found (libungif-4.1.0b1) does
not need/use URT.

> I'd also note in the nt/INSTALL that the dll's are expected to have
> certain names and list those names for each required library.  Then
> just explain to the user that after downloading the image binaries
> they may need to rename some dll's as well as making sure those dll's
> are in the path.

I think is better, as Jason suggests, to try all known names (suitably
sorted to try first the preferred ones).

                                                           /L/e/k/t/u

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> On Fri, 28 May 2004 10:54:39 -0500, Peter Lee <pete_lee@swbell.net> wrote:
> 
> > After getting the latest version of the image libraries from gnuwin32,
> > I had to rename the following files:
> > 
> > jpeg62.dll to jpeg.dll
> > libtiff3.dll to libtiff.dll
> > libXpm-noX4.dll to libxpm.dll
> > zlib1.dll to zlib.dll
> 
> Well, I prefer to change image.c so it looks also for zlib1.dll and
> jpeg62.dll.

Perhaps we could define some lisp variables which are initialized to a
list of strings with library names to try?  Then it would be easier to
tailor a prebuilt emacs to use the available libraries...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 30 May 2004 01:10:03 +0200, storm@cua.dk (Kim F. Storm) wrote:

> Perhaps we could define some lisp variables which are initialized to a
> list of strings with library names to try?

Yes, that'd be cool. An alist:

((tiff "libtiff3.dll" "libtiff.dll")
 (gif "libungif")
 (xpm "libxpm.dll" "libxpm-nox4.dll")
 (zlib "zlib1.dll" "zlib.dll" "zlibd.dll")
 ...)



                                                           /L/e/k/t/u

Re: display word wrapping

[Richard Stallman]

    > Perhaps we could define some lisp variables which are initialized to a
    > list of strings with library names to try?

    Yes, that'd be cool. An alist:

Please make sure this variable is marked as risky.  Any variable
that somehow specifies programs to run must be marked as risky.

Re: display word wrapping

[Jason Rumney]

storm@cua.dk (Kim F. Storm) writes:

> Perhaps we could define some lisp variables which are initialized to a
> list of strings with library names to try?  Then it would be easier to
> tailor a prebuilt emacs to use the available libraries...

Then we'd have to delay the loading of image libraries until after
the user's init file was loaded, and I'm not sure we'd then be in
time to display the splash screen (since its all done in lisp from
then on).

Re: display word wrapping

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > Perhaps we could define some lisp variables which are initialized to a
> > list of strings with library names to try?  Then it would be easier to
> > tailor a prebuilt emacs to use the available libraries...
> 
> Then we'd have to delay the loading of image libraries until after
> the user's init file was loaded, and I'm not sure we'd then be in
> time to display the splash screen (since its all done in lisp from
> then on).
> 

Both term/w32-win.el and .emacs are read before loading the
splash screen.  In addition, IIRC, PBM support is built-in
even on w32.  So it should work.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 31 May 2004 22:18:39 +0200
storm@cua.dk (Kim F. Storm) wrote:

> Both term/w32-win.el and .emacs are read before loading the
> splash screen.  In addition, IIRC, PBM support is built-in
> even on w32.  So it should work.

There's a first cut of implementing that.  The patch seems bigger than
it is, because I've had to extract functionality from init_image and
move it to a new function.

With this patch term/w32-win.el sets the new variable
`w32-image-libraries-alist'.

There's a new `init-image-libraries' which can be called to load all
image libraries. This is called from `init_image' on non-Windows environments,
and does not depend on the alist (so nothing changes). On Windows it
uses the variable, and can be called... where? This is my biggest doubt. If
I call `init-image-libraries' from my .emacs, for example, the splash
screen is shown correctly. But there's no way the user must be
responsible for calling it. So, where should it be called so it is done
automatically on Windows, but *after* loading .emacs?

Another question: currently, I'm not checking whether the func is called
several times. Should it check it and refuse, or allow it (and, in this
case, get sure that image-types is checked before adding image-type
symbols to it)?

I've used the same machinery to load zlib, because libpng12 and libtiff3
need it, and it also can appear under several names (at least zlib1.dll
and zlib.dll are common).

                                                                Juanma



--- image.c.orig	2004-06-02 10:50:59.000000000 +0200
+++ image.c	2004-06-02 16:45:23.000000000 +0200
@@ -63,4 +63,6 @@
 #include "w32term.h"
 
+Lisp_Object Vw32_image_libraries_alist;
+
 /* W32_TODO : Color tables on W32.  */
 #undef COLOR_TABLE_SUPPORT
@@ -1789,4 +1791,31 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `w32-image-libraries-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+  Lisp_Object dll_list;
+
+  dll_list = Fassq (library_id, Vw32_image_libraries_alist);
+
+  if (CONSP (dll_list))
+    for (dll_list = XCDR (dll_list); !NILP (dll_list); dll_list = XCDR (dll_list))
+      {
+        Lisp_Object lib = XCAR (dll_list);
+
+        if (!STRINGP (lib))
+          error ("Library name is not a string");
+        else if (library = LoadLibrary (SDATA (lib)))
+          break;
+      }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3495,5 +3522,5 @@
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (Qxpm)))
     return 0;
 
@@ -5490,4 +5517,9 @@
 Lisp_Object Qpng;
 
+#ifndef NEED_ZLIB
+Lisp_Object Qzlib;
+#define NEED_ZLIB
+#endif
+
 /* Indices of image specification fields in png_format, below.  */
 
@@ -5593,15 +5625,10 @@
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
+  /* Ensure zlib is loaded.  */
+  if (!w32_dynaload (Qzlib))
     return 0;
 
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (Qpng)))
     return 0;
 
@@ -6251,7 +6278,5 @@
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (Qjpeg)))
     return 0;
 
@@ -6605,4 +6630,9 @@
 Lisp_Object Qtiff;
 
+#ifndef NEED_ZLIB
+Lisp_Object Qzlib;
+#define NEED_ZLIB
+#endif
+
 /* Indices of image specification fields in tiff_format, below.  */
 
@@ -6688,5 +6718,9 @@
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  /* Ensure zlib is loaded.  */
+  if (!w32_dynaload (Qzlib))
+    return 0;
+
+  if (!(library = w32_dynaload (Qtiff)))
     return 0;
 
@@ -7108,5 +7142,5 @@
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (Qgif)))
     return 0;
 
@@ -7881,4 +7915,58 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
+#else
+#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-libraries", Finit_image_libraries, Sinit_image_libraries, 0, 0, 0,
+       doc: /* Initialize image libraries.
+Image types pbm and xbm are prebuilt; other types are loaded here.
+On Windows, what gets loaded depends on the value of the variable
+`w32-image-libraries-alist'.  */)
+     ()
+{
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_xpm_functions)
+    define_image_type (&xpm_type);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_jpeg_functions)
+    define_image_type (&jpeg_type);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_tiff_functions)
+    define_image_type (&tiff_type);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_gif_functions)
+    define_image_type (&gif_type);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_png_functions)
+    define_image_type (&png_type);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+  define_image_type (&gs_type);
+#endif
+
+#ifdef MAC_OS
+  /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
+  EnterMovies ();
+#ifdef MAC_OSX
+  init_image_func_pointer ();
+#endif
+#endif
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8046,10 @@
 #endif
 
+#if defined (NEED_ZLIB)
+  Qzlib = intern ("zlib");
+  staticpro (&Qzlib);
+#endif
+
+  defsubr (&Sinit_image_libraries);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7983,14 +8077,17 @@
 meaning don't clear the cache.  */);
   Vimage_cache_eviction_delay = make_number (30 * 60);
-}
-
 
 #ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
+  DEFVAR_LISP ("w32-image-libraries-alist", &Vw32_image_libraries_alist,
+   doc: /* Alist of image-types vs external libraries needed to display them.
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.  */);
+  Vw32_image_libraries_alist = Qnil;
+#endif
+}
 
 void
@@ -8003,39 +8100,8 @@
   define_image_type (&pbm_type);
 
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
-
-#ifdef MAC_OS
-  /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
-  EnterMovies ();
-#ifdef MAC_OSX
-  init_image_func_pointer ();
-#endif
+#ifndef HAVE_NTGUI
+  /* For now, delayed loading of image libraries is only needed
+     or useful on Windows.  */
+  Finit_image_libraries ();
 #endif
 }
--- term/w32-win.el.orig	2004-06-02 16:34:20.000000000 +0200
+++ term/w32-win.el	2004-06-02 16:48:08.000000000 +0200
@@ -1262,4 +1262,16 @@
 	    (error "Font not found")))))
 
+;;; Set default known names for image libraries
+(setq w32-image-libraries-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")
+        ;; A trick: some libraries need to preload zlib, which can appear under several
+        ;; names, so we reuse the machinery
+        (zlib "zlib1.dll" "zlibd.dll" "zlib.dll")))
+(put 'w32-image-libraries-alist 'risky-local-variable t)
+
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
 ;;; w32-win.el ends here

Re: display word wrapping

[Jason Rumney]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> I've used the same machinery to load zlib, because libpng12 and libtiff3
> need it, and it also can appear under several names (at least zlib1.dll
> and zlib.dll are common).

There should be no need to do that. libpng and libtiff will load it
as required.

Re: display word wrapping

[Juanma Barranquero]

On Wed, 02 Jun 2004 23:17:21 +0100, Jason Rumney <jasonr@gnu.org> wrote:

> There should be no need to do that. libpng and libtiff will load it
> as required.

?? AFAICS, it was you who introduced this code in rev 1.200 of w32fns.c:

  /* Ensure zlib is loaded.  Try debug version first.  */
  if (!LoadLibrary ("zlibd.dll"))
    LoadLibrary ("zlib.dll");

If it's not needed, I'll remove all zlib stuff from my patch (which is
cleaner IMHO).

BTW, what do you think of the patch? Any idea where to run
init-image-libraries?

I was thinking of letting the user run it on .emacs if he wants (so he
can see the supported libraries by checking image-types after the fact),
and making sure it is run afterwards if it wasn't run by
emacs.el/default.el.  But still I don't know how to run it. At that
point, we're on Lisp land, aren't we?

Thanks,

                                                           /L/e/k/t/u

Re: display word wrapping

[Jason Rumney]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> On Wed, 02 Jun 2004 23:17:21 +0100, Jason Rumney <jasonr@gnu.org> wrote:
>
>> There should be no need to do that. libpng and libtiff will load it
>> as required.
>
> ?? AFAICS, it was you who introduced this code in rev 1.200 of w32fns.c:
>
>   /* Ensure zlib is loaded.  Try debug version first.  */
>   if (!LoadLibrary ("zlibd.dll"))
>     LoadLibrary ("zlib.dll");

What happens if libpng is in the PATH, but zlib is not? Does Emacs
decide that png is available, then crash trying to use it? If that is
the case, then we should check for it. But if the libpng load fails,
then we do not need to check for it separately.

> BTW, what do you think of the patch? Any idea where to run
> init-image-libraries?

It seems we will have to do it from startup.el, after loading the
init files.

> I was thinking of letting the user run it on .emacs if he wants (so he
> can see the supported libraries by checking image-types after the
> fact)

Make sure that running it a second time has no bad effects. Ideally,
it should try to load only those image libraries that are not already
supported. But making it a no-op the second time through is OK if
that is too difficult.

Re: display word wrapping

[Juanma Barranquero]

On Thu, 03 Jun 2004 08:49:46 +0100
Jason Rumney <jasonr@gnu.org> wrote:

> What happens if libpng is in the PATH, but zlib is not? Does Emacs
> decide that png is available, then crash trying to use it? If that is
> the case, then we should check for it. But if the libpng load fails,
> then we do not need to check for it separately.

libpng and libtiff simply fail to load, so I've removed the explicit
zlib loading from my patch.

> It seems we will have to do it from startup.el, after loading the
> init files.

Perhaps it would make sense to add it to after-init-hook in
term/*-win.el modules.

> Make sure that running it a second time has no bad effects. Ideally,
> it should try to load only those image libraries that are not already
> supported.

OK, I'll do that.

Another question.  It is a bug that a -nw run of Emacs has a non-nil
image-types?

C-h v image-types [RET]  => "image-types's value is (pbm xbm)"


                                                                Juanma

Re: display word wrapping

[David Kastrup]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> Another question.  It is a bug that a -nw run of Emacs has a non-nil
> image-types?
> 
> C-h v image-types [RET]  => "image-types's value is (pbm xbm)"

I don't think so.  The supported image types do not depend on the
actual display currently in use: they specify what kind of images
Emacs is able to decipher and load.

If you want to know whether they can be _displayed_, use
display-images-p.

There is one possible inconsistency I see: postscript images work only
on X displays because they are implemented using X properties and X
Pixmaps.  So their availability is a mixture of the decoding routines
and the actual display type involved.

But for everything else, I don't see an incitement to have image-types
be display-dependent.  B&W bitmap displays should probably _prefer_
monochrome image formats if available, but I don't know if they should
actually fail when only color images are there.  Given current
hardware, we probably don't need to worry too much about that, though.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

On 03 Jun 2004 11:00:38 +0200
David Kastrup <dak@gnu.org> wrote:

> I don't think so.  The supported image types do not depend on the
> actual display currently in use: they specify what kind of images
> Emacs is able to decipher and load.

What does mean "decipher and load" in this context?

If I find-file a .pbm in emacs -nw, what I get is the same that I would
get opening a .gif, i.e., a buffer with binary contents, in Fundamental
mode.

In what sense is emacs -nw able to decipher pbm (which is in image-types)
differently than gif (which is not)?

> But for everything else, I don't see an incitement to have image-types
> be display-dependent.  B&W bitmap displays should probably _prefer_
> monochrome image formats if available, but I don't know if they should
> actually fail when only color images are there.

Either we're miscommunicating, or I'm lost.  I'm talking of a case where
window-system is nil, so no image can ever shown, B&W or otherwise.



                                                                Juanma

Re: display word wrapping

[David Kastrup]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 03 Jun 2004 11:00:38 +0200
> David Kastrup <dak@gnu.org> wrote:
> 
> > I don't think so.  The supported image types do not depend on the
> > actual display currently in use: they specify what kind of images
> > Emacs is able to decipher and load.
> 
> What does mean "decipher and load" in this context?

That Emacs is able to interpret the "image" display property.  The
image display property is a property for strings.  Whether or not I
can have those strings interpreted on a display depends on the
display, not on Emacs.

> In what sense is emacs -nw able to decipher pbm (which is in
> image-types) differently than gif (which is not)?

It can read them into display properties.  It can use find-image on
them.

> > But for everything else, I don't see an incitement to have
> > image-types be display-dependent.  B&W bitmap displays should
> > probably _prefer_ monochrome image formats if available, but I
> > don't know if they should actually fail when only color images are
> > there.
> 
> Either we're miscommunicating, or I'm lost.  I'm talking of a case
> where window-system is nil, so no image can ever shown, B&W or
> otherwise.

It still can be read in, and nobody keeps me from using
make-frame-on-display in order to display it.  Actually, at the
current point of time this won't work, but I suppose that once the
multi-tty branch has been merged, it would.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

On 03 Jun 2004 11:26:44 +0200
David Kastrup <dak@gnu.org> wrote:

> It still can be read in, and nobody keeps me from using
> make-frame-on-display in order to display it.  Actually, at the
> current point of time this won't work, but I suppose that once the
> multi-tty branch has been merged, it would.

Aha, now I understand.

Thanks,

                                                                Juanma

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On Thu, 03 Jun 2004 08:49:46 +0100
> Jason Rumney <jasonr@gnu.org> wrote:

> > It seems we will have to do it from startup.el, after loading the
> > init files.
>
> Perhaps it would make sense to add it to after-init-hook in
> term/*-win.el modules.

You could modify image-type-available-p to do it on the fly:

(defun image-type-available-p (type)
  "Value is non-nil if image type TYPE is available.
Image types are symbols like `xbm' or `jpeg'."
  (when (and (boundp 'image-library-alist) image-library-alist)
     (init-image-libraries image-library-alist)
     (setq image-library-alist nil))
  (and (boundp 'image-types) (not (null (memq type image-types)))))

This would have the effect that the user could try different settings
of image-library-alist and just use the normal image functions to
have the changes loaded on the fly.

>
> > Make sure that running it a second time has no bad effects. Ideally,
> > it should try to load only those image libraries that are not already
> > supported.
>

The above patch guards against that to some extent, but explicitly skipping
those elements which are already in image-types is a good thing.


As you see, I also changed the variable to image-library-alist -- there's
no need to emphasize that this is w32 specific (other systems may need it
later).

Finally, I propose to specify the alist as arg to init-image-libraries;
there's no need to "pollute" the C code with a Vw32_image_libraries_alist
variable.

--
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 03 Jun 2004 14:04:03 +0200
storm@cua.dk (Kim F. Storm) wrote:

> You could modify image-type-available-p to do it on the fly:

> (defun image-type-available-p (type)
>   "Value is non-nil if image type TYPE is available.
> Image types are symbols like `xbm' or `jpeg'."
>   (when (and (boundp 'image-library-alist) image-library-alist)
>      (init-image-libraries image-library-alist)
>      (setq image-library-alist nil))
>   (and (boundp 'image-types) (not (null (memq type image-types)))))

Good idea.

> The above patch guards against that to some extent, but explicitly skipping
> those elements which are already in image-types is a good thing.

It's already implemented.  With my current patch (not included) you can
change image-library-alist and call init-image-libraries and it only
loads (and adds to Vimage_types, etc.) the newly loaded libraries (if
any).

> As you see, I also changed the variable to image-library-alist -- there's
> no need to emphasize that this is w32 specific (other systems may need it
> later).

Ok, but then:

> Finally, I propose to specify the alist as arg to init-image-libraries;
> there's no need to "pollute" the C code with a Vw32_image_libraries_alist
> variable.

why that? It would seem logical to just define Vimage_library_alist on
image.c and make init-image-libraries to use it.  term/w32-win.el would
set up Vimage_library_alist for Windows, and other modes could do it in
their respective term/*.el config files if, or when, delayed loading is
implemented for these systems.

If there's just one variable, there's no need to pass it as argument,
unless you're interested in the flexibility of passing
init-image-libraries arbitrary alists; that seems like a long shot,
though.

                                                                Juanma

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> > The above patch guards against that to some extent, but explicitly skipping
> > those elements which are already in image-types is a good thing.
> 
> It's already implemented.  With my current patch (not included) you can
> change image-library-alist and call init-image-libraries and it only
> loads (and adds to Vimage_types, etc.) the newly loaded libraries (if
> any).

Good.

> > Finally, I propose to specify the alist as arg to init-image-libraries;
> > there's no need to "pollute" the C code with a Vw32_image_libraries_alist
> > variable.
> 
> why that? 

Because we prefer to define things in Lisp whenever possible.

If you can declare the variable in lisp and pass it as a parameter,
that is cleaner than declaring it in C and only use it implicitly.

>           It would seem logical to just define Vimage_library_alist on
> image.c and make init-image-libraries to use it.

IMHO, using an explicit parameter is cleaner than an implicit global variable.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

OK, there goes the complete patch (minus docs, which I'll write when the
issue is ready to commit) implementing all your proposed changes.

It works really well.  The only thing that I would add is calling
`init-image-libraries' from startup, just after loading .emacs.el.  The
reason is that, without this, image-types contains on startup just xbm
and pbm, because the other libraries haven't been yet loaded (unless the
user calls init-image-libraries explicitly on his .emacs.el), even if
image-library-alist is correctly set up and the DLLs are available.

> > It's already implemented.  With my current patch (not included) you can
> > change image-library-alist and call init-image-libraries and it only
> > loads (and adds to Vimage_types, etc.) the newly loaded libraries (if
> > any).
> 
> Good.

In my patch, both w32_dynaload and define_image_type check whether the
image type is already supported (the check in define_image_type is not
strictly necessary, but I prefer to be safe). They do that by searching
on Vimage_types, so the user could "cheat" by modifying this variable,
in which case it'd be posible to force a reload.  I consider this a
clear "Don't Do That" situation: if the user really insists, he gets
what he asked for.

> Because we prefer to define things in Lisp whenever possible.

OK. `image-library-alist' is now defined in lisp/image.el.  Its value is
initializated on term/w32-win.el; other ports wanting to support dynamic
loading of image libraries should do the same in their respective
term/*.el files.

> IMHO, using an explicit parameter is cleaner than an implicit global variable.

I agree, of course, except in cases where the parameter has to be
shoe-horned into already chock-full arglists.  Fortunately this is not
the case (just the init_xxx_functions and w32_dynaload need to be
changed).

Last question (for now). As init-image-libraries needs to return
something, I've done it return Vimage_types instead of Qnil, so the
caller can see at once whether the list of supported image types has
changed. If OK'ed, I'll add this tiny bit of info to the function's
docstring.

                                                           /L/e/k/t/u



Index: lisp/image.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/image.el,v
retrieving revision 1.40
diff -u -2 -r1.40 image.el
--- lisp/image.el	26 Apr 2004 22:11:36 -0000	1.40
+++ lisp/image.el	4 Jun 2004 00:37:28 -0000
@@ -49,4 +49,17 @@
 a non-nil value, TYPE is the image's type.")
 
+;;;###autoload
+(defvar image-library-alist nil
+  "Alist of image-types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.
+
+This variable is reset to nil each time its entries are processed.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +125,8 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
+  (when image-library-alist
+    (init-image-libraries image-library-alist)
+    (setq image-library-alist nil))
   (and (boundp 'image-types) (not (null (memq type image-types)))))
-
 
 ;;;###autoload
Index: lisp/term/w32-win.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/term/w32-win.el,v
retrieving revision 1.62
diff -u -2 -r1.62 w32-win.el
--- lisp/term/w32-win.el	9 May 2004 15:01:17 -0000	1.62
+++ lisp/term/w32-win.el	4 Jun 2004 00:15:48 -0000
@@ -1,5 +1,5 @@
 ;;; w32-win.el --- parse switches controlling interface with W32 window system
 
-;; Copyright (C) 1993, 1994, 2003 Free Software Foundation, Inc.
+;; Copyright (C) 1993, 1994, 2003, 2004 Free Software Foundation, Inc.
 
 ;; Author: Kevin Gallo
@@ -1261,4 +1261,12 @@
 	(if (null font)
 	    (error "Font not found")))))
+
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
 
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
Index: src/image.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/image.c,v
retrieving revision 1.12
diff -u -2 -r1.12 image.c
--- src/image.c	2 Jun 2004 00:50:09 -0000	1.12
+++ src/image.c	4 Jun 2004 01:09:36 -0000
@@ -646,11 +646,15 @@
      struct image_type *type;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  /* Protect against accidental redefinion.  */
+  if (NILP (Fmemq (*type->type, Vimage_types)))
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+      Vimage_types = Fcons (*p->type, Vimage_types);
+    }
 }
 
@@ -1789,4 +1793,34 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (NILP (Fmemq (library_id, Vimage_types)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); !NILP (dlls); dlls = XCDR (dlls))
+          {
+            Lisp_Object lib = XCAR (dlls);
+
+            if (!STRINGP (lib))
+              error ("Invalid entry in library alist");
+            else if (library = LoadLibrary (SDATA (lib)))
+              break;
+          }
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3522,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5621,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6270,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6705,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7125,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,4 +7902,59 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn (libraries))
+#else
+#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-libraries", Finit_image_libraries, Sinit_image_libraries, 1, 1, 0,
+       doc: /* Initialize image libraries.
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Which libraries are loaded depends on LIBRARIES (usually, the value
+of `image-library-alist'.  */)
+     (libraries)
+{
+#if defined (HAVE_XPM) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_xpm_functions)
+    define_image_type (&xpm_type);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_jpeg_functions)
+    define_image_type (&jpeg_type);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_tiff_functions)
+    define_image_type (&tiff_type);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_gif_functions)
+    define_image_type (&gif_type);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+  IF_LIB_AVAILABLE(init_png_functions)
+    define_image_type (&png_type);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+  define_image_type (&gs_type);
+#endif
+
+#ifdef MAC_OS
+  /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
+  EnterMovies ();
+#ifdef MAC_OSX
+  init_image_func_pointer ();
+#endif
+#endif
+
+  return Vimage_types;
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8034,5 @@
 #endif
 
+  defsubr (&Sinit_image_libraries);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,13 +8062,4 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
@@ -8002,41 +8070,4 @@
   define_image_type (&xbm_type);
   define_image_type (&pbm_type);
-
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
-
-#ifdef MAC_OS
-  /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
-  EnterMovies ();
-#ifdef MAC_OSX
-  init_image_func_pointer ();
-#endif
-#endif
 }

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> OK, there goes the complete patch (minus docs, which I'll write when the
> issue is ready to commit) implementing all your proposed changes.
> 
> It works really well.  The only thing that I would add is calling
> `init-image-libraries' from startup, just after loading .emacs.el.  The
> reason is that, without this, image-types contains on startup just xbm
> and pbm, because the other libraries haven't been yet loaded (unless the
> user calls init-image-libraries explicitly on his .emacs.el), even if
> image-library-alist is correctly set up and the DLLs are available.

You can do that, but in practice, image-types is only used by
image-type-available-p which will do init-image-libraries when needed.

So in fact, image-type-available-p could be implemented like this:

(defun image-type-available-p (type)
  (prog1
     (memq type (init-image-libraries image-library-alist))
     (setq image-library-alist nil)))

supposing that init-image-libraries just returns the current image-types
if image-library-alist is nil (and the updated list otherwise).

And then we could discard the image-types variable alltogether --
except that it is still probably good for debugging/error reporting
purposes.

> Last question (for now). As init-image-libraries needs to return
> something, I've done it return Vimage_types instead of Qnil, so the
> caller can see at once whether the list of supported image types has
> changed. If OK'ed, I'll add this tiny bit of info to the function's
> docstring.

Do you mean that it returns nil if no changes were made to image-types?
I think that it should just return image-types in all cases.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 04 Jun 2004 10:07:10 +0200
storm@cua.dk (Kim F. Storm) wrote:

> (defun image-type-available-p (type)
>   (prog1
>      (memq type (init-image-libraries image-library-alist))
>      (setq image-library-alist nil)))
> 
> supposing that init-image-libraries just returns the current image-types
> if image-library-alist is nil (and the updated list otherwise).

Yes, that's what it does: it always returns image-types (which is
already updated if any image lib is ever loaded).

I've implemented image-type-available-p as you say; to do so, I've
updated init-image-libraries to do nothing (except returning image-types)
if its argument is nil, as this is going to be the most frequent use.

> And then we could discard the image-types variable alltogether --
> except that it is still probably good for debugging/error reporting
> purposes.

Well, I use it in my patch to check whether images are loaded (I could
use the image_types list, though, but I also think is better to keep
image-types).

> Do you mean that it returns nil if no changes were made to image-types?
> I think that it should just return image-types in all cases.

Bad wording, sorry.  image-types is always returned.

New question. I'm doing 

        for (dlls = XCDR (dlls); !NILP (dlls); dlls = XCDR (dlls))
          if (!CONSP (dlls) || !STRINGP (XCAR (dlls)))
            error ("Invalid data in library alist");
          else if (library = LoadLibrary (SDATA (XCAR (dlls))))
            break;

What if I wanted to show in the error the wrong data? I cannot use
"SDATA (dlls)" because dlls it could be anything:

  (init-image-libraries '((gif . I'm-bad-bad-bad)))


                                                                Juanma

Re: display word wrapping

[Juanma Barranquero]

On Fri, 04 Jun 2004 11:09:28 +0200
Juanma Barranquero <jmbarranquero@wke.es> wrote:

> I've
> updated init-image-libraries to do nothing (except returning image-types)
> if its argument is nil, as this is going to be the most frequent use.

Scratch that: if init-image-libraries does nothing on nil arguments,
image_types would not be initialized in ports with static libraries.

Two fixes:

 a) Initialize image-library-alist with a non-nil empty alist, like

    (setq image-library-alist '(t))

    (overrided on term/*-win.el), which will be reset to nil on first
    use anyway.

 b) Implement image-type-available-p like that:

    (defun image-type-available-p (type)
      "Value is non-nil if image type TYPE is available.
    Image types are symbols like `xbm' or `jpeg'."
      (if image-library-alist
          (prog1
              (memq type (init-image-libraries image-library-alist))
            (setq image-library-alist nil))
        image-types))

    which is less elegant.

I favor option a), with a suitable comment in code, of course,
explaining what's up.

                                                                Juanma

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On Fri, 04 Jun 2004 11:09:28 +0200
> Juanma Barranquero <jmbarranquero@wke.es> wrote:
> 
> > I've
> > updated init-image-libraries to do nothing (except returning image-types)
> > if its argument is nil, as this is going to be the most frequent use.
> 
> Scratch that: if init-image-libraries does nothing on nil arguments,
> image_types would not be initialized in ports with static libraries.
> 
> Two fixes:
> 
>  a) Initialize image-library-alist with a non-nil empty alist, like
> 
>     (setq image-library-alist '(t))

Just init it to t, and let init-image-libraries test for that special value.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 04 Jun 2004 10:07:10 +0200
> storm@cua.dk (Kim F. Storm) wrote:
> 
> > (defun image-type-available-p (type)
> >   (prog1
> >      (memq type (init-image-libraries image-library-alist))
> >      (setq image-library-alist nil)))
> > 
> > supposing that init-image-libraries just returns the current image-types
> > if image-library-alist is nil (and the updated list otherwise).
> 
> Yes, that's what it does: it always returns image-types (which is
> already updated if any image lib is ever loaded).
> 
> I've implemented image-type-available-p as you say; to do so, I've
> updated init-image-libraries to do nothing (except returning image-types)
> if its argument is nil, as this is going to be the most frequent use.
> 
> > And then we could discard the image-types variable alltogether --
> > except that it is still probably good for debugging/error reporting
> > purposes.
> 
> Well, I use it in my patch to check whether images are loaded (I could
> use the image_types list, though, but I also think is better to keep
> image-types).

I'm sure you need Vimage_types internally, but I meant that you could
drop exporting the Vimage_types to lisp.


BTW, what happens if you compile emacs --without-x ?

Maybe we need a dummy init-image-libraries which returns nil on non-window platforms.

> New question. I'm doing 
> 
>         for (dlls = XCDR (dlls); !NILP (dlls); dlls = XCDR (dlls))
>           if (!CONSP (dlls) || !STRINGP (XCAR (dlls)))
>             error ("Invalid data in library alist");
>           else if (library = LoadLibrary (SDATA (XCAR (dlls))))
>             break;
> 
> What if I wanted to show in the error the wrong data? I cannot use
> "SDATA (dlls)" because dlls it could be anything:

Maybe

        Fsignal (Qwrong_type_argument, Fcons (dlls, Qnil));

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 04 Jun 2004 14:45:30 +0200
storm@cua.dk (Kim F. Storm) wrote:

>>     (setq image-library-alist '(t))

> Just init it to t, and let init-image-libraries test for that special value.

OK.  init-image-libraries does proceed as usual if image-library-alist
is t (but it is responsability of the init_xxx_functions to get sure
that everything's OK if they use image-library-alist in any way).

> I'm sure you need Vimage_types internally, but I meant that you could
> drop exporting the Vimage_types to lisp.

It could be done easily, but, as you say, is useful for debugging.

> BTW, what happens if you compile emacs --without-x ?

I *think* (but I haven't tested it, I don't have a non-Windows
environment on which to try it) it should compile OK and just initialize
the built-in image types (xbm and pbm).

>         Fsignal (Qwrong_type_argument, Fcons (dlls, Qnil));

Ah, I see, thanks.  I've done instead

   wrong_type_argument (Qstringp, dlls);

so the message mentions that a string was expected.

Attached is the current iteration of the patch.  Could you (or anyone)
try it on non-Windows environments?

                                                                Juanma


--- lisp/term/w32-win.el.orig	2004-06-04 15:29:20.000000000 +0200
+++ lisp/term/w32-win.el	2004-06-04 02:15:48.000000000 +0200
@@ -1262,4 +1262,12 @@
 	    (error "Font not found")))))
 
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
+
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
 ;;; w32-win.el ends here
--- lisp/image.el.orig	2004-06-04 15:28:50.000000000 +0200
+++ lisp/image.el	2004-06-04 15:19:33.000000000 +0200
@@ -49,4 +49,19 @@
 a non-nil value, TYPE is the image's type.")
 
+;;; 
+
+;;;###autoload
+(defvar image-library-alist t
+  "Alist of image types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.
+
+This variable is reset to nil each time its entries are processed.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +127,7 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
-  (and (boundp 'image-types) (not (null (memq type image-types)))))
-
+  (prog1
+      (memq type (init-image-libraries image-library-alist))
+    (setq image-library-alist nil)))
 
 ;;;###autoload
--- src/image.c.orig	2004-06-04 15:27:59.000000000 +0200
+++ src/image.c	2004-06-04 15:12:20.000000000 +0200
@@ -646,11 +646,15 @@
      struct image_type *type;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  /* Protect against accidental redefinion.  */
+  if (NILP (Fmemq (*type->type, Vimage_types)))
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+      Vimage_types = Fcons (*p->type, Vimage_types);
+    }
 }
 
@@ -1789,4 +1793,30 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (CONSP (libraries) && NILP (Fmemq (library_id, Vimage_types)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); !NILP (dlls); dlls = XCDR (dlls))
+          if (!CONSP (dlls) || !STRINGP (XCAR (dlls)))
+            wrong_type_argument (Qstringp, dlls);
+          else if (library = LoadLibrary (SDATA (XCAR (dlls))))
+            break;
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3518,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5617,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6266,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6701,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7121,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,4 +7898,63 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn (libraries))
+#else
+#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-libraries", Finit_image_libraries, Sinit_image_libraries, 1, 1, 0,
+       doc: /* Initialize image libraries.
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Which libraries are loaded depends on LIBRARIES (usually, the value
+of `image-library-alist'.  */)
+     (libraries)
+{
+  if (EQ (libraries, Qt) || !NILP (libraries))
+    {
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_xpm_functions)
+        define_image_type (&xpm_type);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_jpeg_functions)
+        define_image_type (&jpeg_type);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_tiff_functions)
+        define_image_type (&tiff_type);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_gif_functions)
+        define_image_type (&gif_type);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_png_functions)
+        define_image_type (&png_type);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+      define_image_type (&gs_type);
+#endif
+
+#ifdef MAC_OS
+      /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
+      EnterMovies ();
+#ifdef MAC_OSX
+      init_image_func_pointer ();
+#endif
+#endif
+    }
+
+  return Vimage_types;
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8034,5 @@
 #endif
 
+  defsubr (&Sinit_image_libraries);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,13 +8062,4 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
@@ -8002,41 +8070,4 @@
   define_image_type (&xbm_type);
   define_image_type (&pbm_type);
-
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
-
-#ifdef MAC_OS
-  /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
-  EnterMovies ();
-#ifdef MAC_OSX
-  init_image_func_pointer ();
-#endif
-#endif
 }

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> > BTW, what happens if you compile emacs --without-x ?
> 
> I *think* (but I haven't tested it, I don't have a non-Windows
> environment on which to try it) it should compile OK and just initialize
> the built-in image types (xbm and pbm).

Problem is that image.c is not compiled in this env. so the
init-image-libraries function does not exist!  This will fix it:

(defun image-type-available-p
  ...
  (and (fboundp 'init-image-libraries)
       (prog1 ...)))


> 
> >         Fsignal (Qwrong_type_argument, Fcons (dlls, Qnil));
> 
> Ah, I see, thanks.  I've done instead
> 
>    wrong_type_argument (Qstringp, dlls);

Using
  CHECK_STRING (dlls);
is cleaner then.


> 
> so the message mentions that a string was expected.
> 
> Attached is the current iteration of the patch.  Could you (or anyone)
> try it on non-Windows environments?

I don't have time right now, but maybe during the weekend.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 04 Jun 2004 16:16:32 +0200, storm@cua.dk (Kim F. Storm) wrote:

> Problem is that image.c is not compiled in this env. so the
> init-image-libraries function does not exist!

Ah, OK.  I thought it was compiled, but mostly to no-ops.

> This will fix it:
> 
> (defun image-type-available-p
>   ...
>   (and (fboundp 'init-image-libraries)
>        (prog1 ...)))

OK, added.

> Using
>   CHECK_STRING (dlls);
> is cleaner then.

I should've put more attention while perusing lisp.h.

BTW, this is turning into a great learning experience.  Thanks!  (And
sorry for taking so much of your time.)

> I don't have time right now, but maybe during the weekend.

Patch attached with newest mods (but still no docs).

                                                           /L/e/k/t/u



Index: lisp/image.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/image.el,v
retrieving revision 1.40
diff -u -2 -r1.40 image.el
--- lisp/image.el	26 Apr 2004 22:11:36 -0000	1.40
+++ lisp/image.el	5 Jun 2004 00:47:31 -0000
@@ -49,4 +49,17 @@
 a non-nil value, TYPE is the image's type.")
 
+;;;###autoload
+(defvar image-library-alist t
+  "Alist of image types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.
+
+This variable is reset to nil each time its entries are processed.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +125,8 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
-  (and (boundp 'image-types) (not (null (memq type image-types)))))
-
+  (and (fboundp 'init-image-libraries)
+       (prog1
+           (memq type (init-image-libraries image-library-alist))
+         (setq image-library-alist nil))))
 
 ;;;###autoload
Index: lisp/term/w32-win.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/term/w32-win.el,v
retrieving revision 1.62
diff -u -2 -r1.62 w32-win.el
--- lisp/term/w32-win.el	9 May 2004 15:01:17 -0000	1.62
+++ lisp/term/w32-win.el	4 Jun 2004 00:15:48 -0000
@@ -1261,4 +1261,12 @@
 	(if (null font)
 	    (error "Font not found")))))
+
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
 
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
Index: src/image.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/image.c,v
retrieving revision 1.12
diff -u -2 -r1.12 image.c
--- src/image.c	2 Jun 2004 00:50:09 -0000	1.12
+++ src/image.c	5 Jun 2004 01:10:44 -0000
@@ -646,11 +646,15 @@
      struct image_type *type;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  /* Protect against accidental redefinion.  */
+  if (NILP (Fmemq (*type->type, Vimage_types)))
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+      Vimage_types = Fcons (*p->type, Vimage_types);
+    }
 }
 
@@ -1789,4 +1793,31 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (CONSP (libraries) && NILP (Fmemq (library_id, Vimage_types)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); CONSP (dlls); dlls = XCDR (dlls))
+          {
+            CHECK_STRING_CAR (dlls);
+            if (library = LoadLibrary (SDATA (XCAR (dlls))))
+              break;
+          }
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3519,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5618,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6267,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6702,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7122,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,4 +7899,63 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn (libraries))
+#else
+#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-libraries", Finit_image_libraries, Sinit_image_libraries, 1, 1, 0,
+       doc: /* Initialize image libraries.
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Which libraries are loaded depends on LIBRARIES (usually, the value
+of `image-library-alist'.  */)
+     (libraries)
+{
+  if (EQ (libraries, Qt) || !NILP (libraries))
+    {
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_xpm_functions)
+        define_image_type (&xpm_type);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_jpeg_functions)
+        define_image_type (&jpeg_type);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_tiff_functions)
+        define_image_type (&tiff_type);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_gif_functions)
+        define_image_type (&gif_type);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(init_png_functions)
+        define_image_type (&png_type);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+      define_image_type (&gs_type);
+#endif
+
+#ifdef MAC_OS
+      /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
+      EnterMovies ();
+#ifdef MAC_OSX
+      init_image_func_pointer ();
+#endif
+#endif
+    }
+
+  return Vimage_types;
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8035,5 @@
 #endif
 
+  defsubr (&Sinit_image_libraries);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,13 +8063,4 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
@@ -8002,41 +8071,4 @@
   define_image_type (&xbm_type);
   define_image_type (&pbm_type);
-
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
-
-#ifdef MAC_OS
-  /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
-  EnterMovies ();
-#ifdef MAC_OSX
-  init_image_func_pointer ();
-#endif
-#endif
 }

Re: display word wrapping

[Juanma Barranquero]

I've finally gone the full length and made image loading delayed till
the type is needed, so

  (image-type-available-p 'jpeg)

will load only jpeg, if not yet available.  That way, most of my runs of
Emacs on Windows will never load any image library at all (so, less
memory footprint).

It has the nice side effect that it eliminates the need for any special
handling of t in image-library-alist, so it's in fact simpler that the
previous incarnation of the patch.

                                                                Juanma




--- src/image.c.orig	2004-06-07 13:14:55.000000000 +0200
+++ src/image.c	2004-06-07 14:36:43.000000000 +0200
@@ -646,11 +646,15 @@
      struct image_type *type;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  /* Protect against accidental redefinion.  */
+  if (NILP (Fmemq (*type->type, Vimage_types)))
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+      Vimage_types = Fcons (*p->type, Vimage_types);
+    }
 }
 
@@ -1789,4 +1793,31 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (CONSP (libraries) && NILP (Fmemq (library_id, Vimage_types)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); CONSP (dlls); dlls = XCDR (dlls))
+          {
+            CHECK_STRING_CAR (dlls);
+            if (library = LoadLibrary (SDATA (XCAR (dlls))))
+              break;
+          }
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3519,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5618,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6267,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6702,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7122,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,4 +7899,56 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define IF_LIB_AVAILABLE(img_type, init_lib_fn)  if (EQ (type, img_type) && init_lib_fn (libraries))
+#else
+#define IF_LIB_AVAILABLE(img_type, init_lib_fn)  if (EQ (type, img_type))
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 2, 2, 0,
+       doc: /* Initialize image library implementing image type TYPE.
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Libraries to load are specified in alist LIBRARIES (usually, the value
+of `image-library-alist', which see.  */)
+  (type, libraries)
+{
+  if (NILP (Fmemq (type, Vimage_types)))
+    {
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(Qxpm, init_xpm_functions)
+        define_image_type (&xpm_type);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(Qjpeg, init_jpeg_functions)
+        define_image_type (&jpeg_type);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(Qtiff, init_tiff_functions)
+        define_image_type (&tiff_type);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(Qgif, init_gif_functions)
+        define_image_type (&gif_type);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(Qpng, init_png_functions)
+        define_image_type (&png_type);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+      IF_LIB_AVAILABLE(Qpostscript, init_gs_functions)
+        define_image_type (&gs_type);
+#endif
+    }
+
+  return Vimage_types;
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8028,5 @@
 #endif
 
+  defsubr (&Sinit_image_library);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,13 +8056,4 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
@@ -8003,33 +8065,4 @@
   define_image_type (&pbm_type);
 
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
-
 #ifdef MAC_OS
   /* Animated gifs use QuickTime Movie Toolbox.  So initialize it here. */
--- lisp/image.el.orig	2004-06-07 13:13:55.000000000 +0200
+++ lisp/image.el	2004-06-07 14:33:21.000000000 +0200
@@ -49,4 +49,15 @@
 a non-nil value, TYPE is the image's type.")
 
+;;;###autoload
+(defvar image-library-alist nil
+  "Alist of image types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +123,6 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
-  (and (boundp 'image-types) (not (null (memq type image-types)))))
-
+  (and (fboundp 'init-image-library)
+       (memq type (init-image-library type image-library-alist))))
 
 ;;;###autoload
--- lisp/term/w32-win.el.orig	2004-06-07 13:16:33.000000000 +0200
+++ lisp/term/w32-win.el	2004-06-04 02:15:48.000000000 +0200
@@ -1262,4 +1262,12 @@
 	    (error "Font not found")))))
 
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
+
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
 ;;; w32-win.el ends here

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> I've finally gone the full length and made image loading delayed till
> the type is needed, so
> 
>   (image-type-available-p 'jpeg)
> 
> will load only jpeg, if not yet available.  That way, most of my runs of
> Emacs on Windows will never load any image library at all (so, less
> memory footprint).

Nice.

> It has the nice side effect that it eliminates the need for any special
> handling of t in image-library-alist, so it's in fact simpler that the
> previous incarnation of the patch.

But you can improve it even further, see below.
> +DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 2, 2, 0,
> +       doc: /* Initialize image library implementing image type TYPE.

Add:
  Returns non-nil if TYPE is a supported image type.

> +Image types pbm and xbm are prebuilt; other types are loaded here.
> +Libraries to load are specified in alist LIBRARIES (usually, the value
> +of `image-library-alist', which see.  */)
> +  (type, libraries)
> +{
> +  if (NILP (Fmemq (type, Vimage_types)))
> +    {

Replace by:

     if (!NILP (Fmemq (type, Vimage_types)))
       return Qt;

[...]

> +    }
> +
> +  return Vimage_types;
> +}

Replace by:

    return Fmemq (type, Vimage_types);

to redo lookup after initializing the image type.

Alternatively, you could let define_image_type return Qt or Qnil
depending on whether the type was added to Vimage_types, 
and then write the code like this:

+#if defined (HAVE_PNG) || defined (MAC_OS)
+      IF_LIB_AVAILABLE(Qpng, init_png_functions)
+        return define_image_type (&png_type);
+#endif
+

and then just return Qnil if you fall through to the
end of the function.


> @@ -112,6 +123,6 @@
>    "Value is non-nil if image type TYPE is available.
>  Image types are symbols like `xbm' or `jpeg'."
> -  (and (boundp 'image-types) (not (null (memq type image-types)))))
> -
> +  (and (fboundp 'init-image-library)
> +       (memq type (init-image-library type image-library-alist))))

Replace by:

> +  (and (fboundp 'init-image-library)
> +       (init-image-library type image-library-alist))



However, one problem with the dynamic loading is that for image
formats that fails to load, you will try to load them again every
time you test for the availablity of that image type.

Maybe Vimage_types should be changed to an alist

        ((png . t) (jpeg . nil) ...)

storing the results of the type lookups already performed.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 07 Jun 2004 15:34:18 +0200
storm@cua.dk (Kim F. Storm) wrote:

> Add:
>   Returns non-nil if TYPE is a supported image type.

Of course!  Silly me.

> Replace by:
> 
>      if (!NILP (Fmemq (type, Vimage_types)))
>        return Qt;
> 
> [...]
> 
> > +    }
> > +
> > +  return Vimage_types;
> > +}
> 
> Replace by:
> 
>     return Fmemq (type, Vimage_types);
> 
> to redo lookup after initializing the image type.

Yes, you're right.

> Alternatively, you could let define_image_type return Qt or Qnil
> depending on whether the type was added to Vimage_types, 
> and then write the code like this:
> 
> +#if defined (HAVE_PNG) || defined (MAC_OS)
> +      IF_LIB_AVAILABLE(Qpng, init_png_functions)
> +        return define_image_type (&png_type);
> +#endif
> +

I initially argued why shouldn't be so, but after giving some thought to the
problem below, I came to think it's the best answer.

> However, one problem with the dynamic loading is that for image
> formats that fails to load, you will try to load them again every
> time you test for the availablity of that image type.

Right.

> Maybe Vimage_types should be changed to an alist
> 
>         ((png . t) (jpeg . nil) ...)
> 
> storing the results of the type lookups already performed.

That's what I've implemented.

                                                                Juanma



--- src/image.c.orig	2004-06-07 13:14:55.000000000 +0200
+++ src/image.c	2004-06-07 17:55:26.000000000 +0200
@@ -630,5 +630,5 @@
 /* Function prototypes.  */
 
-static void define_image_type P_ ((struct image_type *type));
+static int define_image_type P_ ((struct image_type *type, int loaded));
 static struct image_type *lookup_image_type P_ ((Lisp_Object symbol));
 static void image_error P_ ((char *format, Lisp_Object, Lisp_Object));
@@ -640,17 +640,24 @@
 
 /* Define a new image type from TYPE.  This adds a copy of TYPE to
-   image_types and adds the symbol *TYPE->type to Vimage_types.  */
+   image_types and adds (*TYPE->type, loaded) to Vimage_types.  */
 
-static void
-define_image_type (type)
+static int
+define_image_type (type, loaded)
      struct image_type *type;
+     int loaded;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  if (loaded)
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+    }
+
+  Vimage_types = Fcons (Fcons (*type->type, loaded ? Qt : Qnil), Vimage_types);
+
+  return loaded;
 }
 
@@ -1789,4 +1796,31 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (CONSP (libraries) && NILP (Fassq (library_id, Vimage_types)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); CONSP (dlls); dlls = XCDR (dlls))
+          {
+            CHECK_STRING_CAR (dlls);
+            if (library = LoadLibrary (SDATA (XCAR (dlls))))
+              break;
+          }
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3522,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5621,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6270,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6705,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7125,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,4 +7902,63 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define CHECK_LIB_AVAILABLE(image_type, init_lib_fn) \
+  define_image_type (image_type, init_lib_fn (libraries))
+#else
+#define CHECK_LIB_AVAILABLE(image_type, init_lib_fn) \
+  define_image_type (image_type, TRUE)
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 2, 2, 0,
+       doc: /* Initialize image library implementing image type TYPE.
+Return non-nil if TYPE is a supported image type.
+
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Libraries to load are specified in alist LIBRARIES (usually, the value
+of `image-library-alist', which see.  */)
+  (type, libraries)
+{
+  Lisp_Object tested;
+
+  /* Don't try to reload the library.  */
+  tested = Fassq (type, Vimage_types);
+  if (CONSP (tested))
+    return XCDR (tested);
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+  if (EQ (type, Qxpm))
+    return CHECK_LIB_AVAILABLE(&xpm_type, init_xpm_functions);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+  if (EQ (type, Qjpeg))
+    return CHECK_LIB_AVAILABLE(&jpeg_type, init_jpeg_functions);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+  if (EQ (type, Qtiff))
+    return CHECK_LIB_AVAILABLE(&tiff_type, init_tiff_functions);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+  if (EQ (type, Qgif))
+    return CHECK_LIB_AVAILABLE(&gif_type, init_gif_functions);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+  if (EQ (type, Qpng))
+    return CHECK_LIB_AVAILABLE(&png_type, init_png_functions);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+  if (EQ (type, Qpostscript))
+    return CHECK_LIB_AVAILABLE(&gs_type, init_gs_functions);
+#endif
+
+  return Qnil;
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8038,5 @@
 #endif
 
+  defsubr (&Sinit_image_library);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,13 +8066,4 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
@@ -8000,35 +8072,6 @@
   Vimage_types = Qnil;
 
-  define_image_type (&xbm_type);
-  define_image_type (&pbm_type);
-
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
+  define_image_type (&xbm_type, TRUE);
+  define_image_type (&pbm_type, TRUE);
 
 #ifdef MAC_OS
--- lisp/image.el.orig	2004-06-07 13:13:55.000000000 +0200
+++ lisp/image.el	2004-06-07 17:34:51.000000000 +0200
@@ -49,4 +49,15 @@
 a non-nil value, TYPE is the image's type.")
 
+;;;###autoload
+(defvar image-library-alist nil
+  "Alist of image types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +123,6 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
-  (and (boundp 'image-types) (not (null (memq type image-types)))))
-
+  (and (fboundp 'init-image-library)
+       (init-image-library type image-library-alist)))
 
 ;;;###autoload
--- lisp/term/w32-win.el.orig	2004-06-07 13:16:33.000000000 +0200
+++ lisp/term/w32-win.el	2004-06-04 02:15:48.000000000 +0200
@@ -1262,4 +1262,12 @@
 	    (error "Font not found")))))
 
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
+
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
 ;;; w32-win.el ends here

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> That's what I've implemented.

Looks really good now!!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 01:11:21 +0200, storm@cua.dk (Kim F. Storm) wrote:

> Looks really good now!!

Yeah, it looks ;)

Anyway, this is the latest revision, almost identical to the previous
one, with two small changes:

  - Fixing a Lisp_Object/int mixup (I was returning an int from
    define_image_type and passing it as is from init-image-library,
    which returns a Lisp_Object).

  - Unknown image types (like 'zzz) are now added to Vimage_types, i.e.,
    after (image-type-available-p 'zzz), image-types contains
    '(zzz . nil).

If people tests it on non-Windows environments and it does not cause
any regresion, I'd suggest installing it for 21.[45]***.  With a
suitable entry on etc/NEWS, of course, and any required info on the manual,
if any (I'm not entirely sure this should be mentioned in any place
other than nt/INSTALL, unless there's a plan to someday support delayed
loading on non-Windows.)

Although delayed loading is, technically speaking, a new feature, it is
not visible outside Windows (other than the fact that image-types is
populated on use, but that shouldn't affect usability at all), and on
Windows it will certainly help if the GnuWin32 people changes names
again, or other ports of the image libraries are made available.

*** Did we reach any decision about 21.4 vs 21.5, BTW?

                                                           /L/e/k/t/u



Index: lisp/image.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/image.el,v
retrieving revision 1.40
diff -u -2 -B -r1.40 image.el
--- lisp/image.el	26 Apr 2004 22:11:36 -0000	1.40
+++ lisp/image.el	8 Jun 2004 00:06:53 -0000
@@ -49,4 +49,15 @@
 a non-nil value, TYPE is the image's type.")
 
+;;;###autoload
+(defvar image-library-alist nil
+  "Alist of image types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +123,6 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
-  (and (boundp 'image-types) (not (null (memq type image-types)))))
-
+  (and (fboundp 'init-image-library)
+       (init-image-library type image-library-alist)))
 
 ;;;###autoload
Index: lisp/term/w32-win.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/term/w32-win.el,v
retrieving revision 1.62
diff -u -2 -B -r1.62 w32-win.el
--- lisp/term/w32-win.el	9 May 2004 15:01:17 -0000	1.62
+++ lisp/term/w32-win.el	8 Jun 2004 00:06:58 -0000
@@ -1262,4 +1262,12 @@
 	    (error "Font not found")))))
 
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
+
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
 ;;; w32-win.el ends here
Index: src/image.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/image.c,v
retrieving revision 1.12
diff -u -2 -B -r1.12 image.c
--- src/image.c	2 Jun 2004 00:50:09 -0000	1.12
+++ src/image.c	8 Jun 2004 00:29:42 -0000
@@ -630,5 +630,5 @@
 /* Function prototypes.  */
 
-static void define_image_type P_ ((struct image_type *type));
+static Lisp_Object define_image_type P_ ((struct image_type *type, int loaded));
 static struct image_type *lookup_image_type P_ ((Lisp_Object symbol));
 static void image_error P_ ((char *format, Lisp_Object, Lisp_Object));
@@ -638,19 +638,32 @@
 				       Lisp_Object));
 
+#define SET_IMAGE_TYPE_STATUS(type, status) \
+  do { Vimage_types = Fcons (Fcons (type, status), Vimage_types); } while (0)
 
 /* Define a new image type from TYPE.  This adds a copy of TYPE to
-   image_types and adds the symbol *TYPE->type to Vimage_types.  */
+   image_types and adds (*TYPE->type, loaded) to Vimage_types.  */
 
-static void
-define_image_type (type)
+static Lisp_Object
+define_image_type (type, loaded)
      struct image_type *type;
+     int loaded;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  Lisp_Object success;
+
+  if (!loaded)
+    success = Qnil;
+  else
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+      success = Qt;
+    }
+
+  SET_IMAGE_TYPE_STATUS(*type->type, success);
+  return success;
 }
 
@@ -1789,4 +1802,31 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (CONSP (libraries) && NILP (Fassq (library_id, Vimage_types)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); CONSP (dlls); dlls = XCDR (dlls))
+          {
+            CHECK_STRING_CAR (dlls);
+            if (library = LoadLibrary (SDATA (XCAR (dlls))))
+              break;
+          }
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3529,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5628,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6277,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6712,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7132,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,4 +7909,65 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define CHECK_LIB_AVAILABLE(image_type, init_lib_fn) \
+  define_image_type (image_type, init_lib_fn (libraries))
+#else
+#define CHECK_LIB_AVAILABLE(image_type, init_lib_fn) \
+  define_image_type (image_type, TRUE)
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 2, 2, 0,
+       doc: /* Initialize image library implementing image type TYPE.
+Return non-nil if TYPE is a supported image type.
+
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Libraries to load are specified in alist LIBRARIES (usually, the value
+of `image-library-alist', which see.  */)
+  (type, libraries)
+{
+  Lisp_Object tested;
+
+  /* Don't try to reload the library.  */
+  tested = Fassq (type, Vimage_types);
+  if (CONSP (tested))
+    return XCDR (tested);
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+  if (EQ (type, Qxpm))
+    return CHECK_LIB_AVAILABLE(&xpm_type, init_xpm_functions);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+  if (EQ (type, Qjpeg))
+    return CHECK_LIB_AVAILABLE(&jpeg_type, init_jpeg_functions);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+  if (EQ (type, Qtiff))
+    return CHECK_LIB_AVAILABLE(&tiff_type, init_tiff_functions);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+  if (EQ (type, Qgif))
+    return CHECK_LIB_AVAILABLE(&gif_type, init_gif_functions);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+  if (EQ (type, Qpng))
+    return CHECK_LIB_AVAILABLE(&png_type, init_png_functions);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+  if (EQ (type, Qpostscript))
+    return CHECK_LIB_AVAILABLE(&gs_type, init_gs_functions);
+#endif
+
+  /* If the type is not recognized, avoid testing it ever again.  */
+  SET_IMAGE_TYPE_STATUS(type, Qnil);
+  return Qnil;
+}
+
 void
 syms_of_image ()
@@ -7958,4 +8047,5 @@
 #endif
 
+  defsubr (&Sinit_image_library);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,13 +8075,4 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
@@ -8000,35 +8081,6 @@
   Vimage_types = Qnil;
 
-  define_image_type (&xbm_type);
-  define_image_type (&pbm_type);
-
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
-
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
+  define_image_type (&xbm_type, TRUE);
+  define_image_type (&pbm_type, TRUE);
 
 #ifdef MAC_OS

Re: display word wrapping

[David Kastrup]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> On 08 Jun 2004 01:11:21 +0200, storm@cua.dk (Kim F. Storm) wrote:
> 
> > Looks really good now!!
> 
> Yeah, it looks ;)
> 
> Anyway, this is the latest revision, almost identical to the previous
> one, with two small changes:
> 
>   - Fixing a Lisp_Object/int mixup (I was returning an int from
>     define_image_type and passing it as is from init-image-library,
>     which returns a Lisp_Object).
> 
>   - Unknown image types (like 'zzz) are now added to Vimage_types, i.e.,
>     after (image-type-available-p 'zzz), image-types contains
>     '(zzz . nil).

If an image type is known, I hope that it is present as just its
name, and _not_ as a cons cell (jpeg . t) or so.

If not, we have a major incompatibility to the previous behavior.  It
is bad enough already what happens if somebody decides to cycle
through that variable just to display supported image types.

Can't we just keep the variable with the original meaning, and store
the lookup cache data somewhere else?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> Juanma Barranquero <lektu@mi.madritel.es> writes:
> 
> > On 08 Jun 2004 01:11:21 +0200, storm@cua.dk (Kim F. Storm) wrote:
> > 
> > > Looks really good now!!
> > 
> > Yeah, it looks ;)
> > 
> > Anyway, this is the latest revision, almost identical to the previous
> > one, with two small changes:
> > 
> >   - Fixing a Lisp_Object/int mixup (I was returning an int from
> >     define_image_type and passing it as is from init-image-library,
> >     which returns a Lisp_Object).
> > 
> >   - Unknown image types (like 'zzz) are now added to Vimage_types, i.e.,
> >     after (image-type-available-p 'zzz), image-types contains
> >     '(zzz . nil).
> 
> If an image type is known, I hope that it is present as just its
> name, and _not_ as a cons cell (jpeg . t) or so.
> 
> If not, we have a major incompatibility to the previous behavior.  

I would not call it a major incompatibility.  Code should not rely
on that variable -- they should call image-type-available-p.

>                                                                    It
> is bad enough already what happens if somebody decides to cycle
> through that variable just to display supported image types.

But with Juanma's delayed loading that variable does NOT provide a
complete list of supported types anyway -- only the types that 
are loaded so far.

> 
> Can't we just keep the variable with the original meaning, and store
> the lookup cache data somewhere else?

It could be done that way (the C code could use an internal
unsupported_image_types list which is NOT exported to lisp).

But I don't see a big reason to do so (since image-types is not
reliable anyway).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > If an image type is known, I hope that it is present as just its
> > name, and _not_ as a cons cell (jpeg . t) or so.
> > 
> > If not, we have a major incompatibility to the previous behavior.  
> 
> I would not call it a major incompatibility.  Code should not rely
> on that variable -- they should call image-type-available-p.

Says who?

> > Can't we just keep the variable with the original meaning, and store
> > the lookup cache data somewhere else?
> 
> It could be done that way (the C code could use an internal
> unsupported_image_types list which is NOT exported to lisp).
> 
> But I don't see a big reason to do so (since image-types is not
> reliable anyway).

Tell that to the Elisp manual.  Even in its current version it says:

    File: elisp,  Node: Images,  Next: Buttons,  Prev: Display Property,  Up: Display

    Images
    ======

    To display an image in an Emacs buffer, you must first create an image
    descriptor, then use it as a display specifier in the `display'
    property of text that is displayed (*note Display Property::).  Like the
    `display' property, this feature is available starting in Emacs 21.

       Emacs can display a number of different image formats; some of them
    are supported only if particular support libraries are installed on your
    machine.  The supported image formats include XBM, XPM (needing the
    libraries `libXpm' version 3.4k and `libz'), GIF (needing `libungif'
    4.1.0), Postscript, PBM, JPEG (needing the `libjpeg' library version
    v6a), TIFF (needing `libtiff' v3.4), and PNG (needing `libpng' 1.0.2).

       You specify one of these formats with an image type symbol.  The
    image type symbols are `xbm', `xpm', `gif', `postscript', `pbm',
    `jpeg', `tiff', and `png'.

     - Variable: image-types
         This variable contains a list of those image type symbols that are
         supported in the current configuration.


image-type-available-p is not even documented anywhere.  And of
course, stupid as I am, I have followed the Elisp manual's information
in a package I provide.

Since image-types has been the only documented way of accessing that
information so far, I think there is some justification in not
changing its meaning before any package provider has had a chance to
adapt.

I would prefer it if there was some time window of changing packages
and adapting to new conventions before breaking things.

BTW, do you have an idea since which Emacs version
image-type-available-p has been available?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > David Kastrup <dak@gnu.org> writes:
> > 
> > > If an image type is known, I hope that it is present as just its
> > > name, and _not_ as a cons cell (jpeg . t) or so.
> > > 
> > > If not, we have a major incompatibility to the previous behavior.  
> > 
> > I would not call it a major incompatibility.  Code should not rely
> > on that variable -- they should call image-type-available-p.
> 
> Says who?

I do :-)  

Because that's how the code in image.el checks it.


>      - Variable: image-types
>          This variable contains a list of those image type symbols that are
>          supported in the current configuration.
> 
> 
> image-type-available-p is not even documented anywhere.  And of
> course, stupid as I am, I have followed the Elisp manual's information
> in a package I provide.

We should definitely document image-type-available-p there.

If we change that, it is probably better NOT to maintain image-types
as a (potentially incomplete) list of supported image types.  Then
people will catch problems with the implementation sooner by not
relying on that variable anymore.

In any case, as Juanma also mentions, there is very little code
using image-types (your code being one) so it will not break a
lot of code.

> 
> Since image-types has been the only documented way of accessing that
> information so far, I think there is some justification in not
> changing its meaning before any package provider has had a chance to
> adapt.

True.

> BTW, do you have an idea since which Emacs version
> image-type-available-p has been available?

21.1 

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:

> > Since image-types has been the only documented way of accessing
> > that information so far, I think there is some justification in
> > not changing its meaning before any package provider has had a
> > chance to adapt.
> 
> True.
> 
> > BTW, do you have an idea since which Emacs version
> > image-type-available-p has been available?
> 
> 21.1

I just checked preview-latex.  During operation,
image-type-available-p is used.  Good.  During the installation,
however, image-types is checked to exist and be non-nil.  If it isn't,
installation is aborted with a notice that you need an Emacs version
supporting images.

Will this jibe with the current implementation?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 11:54:49 +0200
David Kastrup <dak@gnu.org> wrote:

> During the installation,
> however, image-types is checked to exist and be non-nil.  If it isn't,
> installation is aborted with a notice that you need an Emacs version
> supporting images.
> 
> Will this jibe with the current implementation?

On any Emacs supporting images, image-types is going to be always
non-nil, because it will at least containt '(pbm xbm), which are
prebuilt.

                                                                Juanma

Re: display word wrapping

[David Kastrup]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 08 Jun 2004 11:54:49 +0200
> David Kastrup <dak@gnu.org> wrote:
> 
> > During the installation,
> > however, image-types is checked to exist and be non-nil.  If it isn't,
> > installation is aborted with a notice that you need an Emacs version
> > supporting images.
> > 
> > Will this jibe with the current implementation?
> 
> On any Emacs supporting images, image-types is going to be always
> non-nil, because it will at least containt '(pbm xbm), which are
> prebuilt.

Ok, then there is no objection from the view of preview-latex.  But
the documentation in the Elisp manual needs to get changed.

And since the old behavior was already documented, perhaps we should
not change image-type to hold something entirely different.

How about having it be a constant containing all image types
_potentially_ supported by that compilation?  The complete actual set,
if it was needed, could then be gotten with

(delq nil (mapcar (lambda(x) (and (image-type-available-p x) x))
  image-types))

Then image-types at least has _some_ meaning similar to before.  But
adding alist elements into it completely changes the meaning.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 12:10:49 +0200
David Kastrup <dak@gnu.org> wrote:

> Ok, then there is no objection from the view of preview-latex.  But
> the documentation in the Elisp manual needs to get changed.

Of course.

> And since the old behavior was already documented, perhaps we should
> not change image-type to hold something entirely different.

No problem.

> How about having it be a constant containing all image types
> _potentially_ supported by that compilation?  The complete actual set,
> if it was needed, could then be gotten with
> 
> (delq nil (mapcar (lambda(x) (and (image-type-available-p x) x))
>   image-types))

It's OK to me.

                                                                Juanma

Re: display word wrapping

[David Kastrup]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 08 Jun 2004 12:10:49 +0200
> David Kastrup <dak@gnu.org> wrote:
> 
> > Ok, then there is no objection from the view of preview-latex.  But
> > the documentation in the Elisp manual needs to get changed.
> 
> Of course.
> 
> > And since the old behavior was already documented, perhaps we should
> > not change image-type to hold something entirely different.
> 
> No problem.
> 
> > How about having it be a constant containing all image types
> > _potentially_ supported by that compilation?  The complete actual set,
> > if it was needed, could then be gotten with
> > 
> > (delq nil (mapcar (lambda(x) (and (image-type-available-p x) x))
> >   image-types))
> 
> It's OK to me.

Anyway, what does image-type-available-p imply exactly?  Should it
perhaps get an &optional display argument like display-images-p does?

For example, postscript images are only supported on frames appearing
on X, and so on.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 12:43:18 +0200
David Kastrup <dak@gnu.org> wrote:

> Anyway, what does image-type-available-p imply exactly?  Should it
> perhaps get an &optional display argument like display-images-p does?

No, I don't think so.

(image-type-available-p 'jpeg)

implies that we've linked, either statically or dynamically, the
libraries needed to process jpeg images, and that we've set up pointers
to the relevant functions for loading them, etc.

> For example, postscript images are only supported on frames appearing
> on X, and so on.

Whether a frame is able to display a given image is not related to
image-type-available-p.  But you'll have to ask Kim.

                                                                Juanma

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> On 08 Jun 2004 12:43:18 +0200
> David Kastrup <dak@gnu.org> wrote:
> 
> > Anyway, what does image-type-available-p imply exactly?  Should it
> > perhaps get an &optional display argument like display-images-p does?
> 
> No, I don't think so.
> 
> (image-type-available-p 'jpeg)
> 
> implies that we've linked, either statically or dynamically, the
> libraries needed to process jpeg images, and that we've set up pointers
> to the relevant functions for loading them, etc.
> 
> > For example, postscript images are only supported on frames appearing
> > on X, and so on.
> 
> Whether a frame is able to display a given image is not related to
> image-type-available-p.  But you'll have to ask Kim.

I think it is a separate issue.  

If needed, it should be a separate function display-supports-image-type-p
(following the naming of display-supports-face-attributes-p).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Miles Bader]

On Tue, Jun 08, 2004 at 02:08:09PM +0200, Kim F. Storm wrote:
> If needed, it should be a separate function display-supports-image-type-p
> (following the naming of display-supports-face-attributes-p).

Note that `display-supports-face-attributes-p' is called that because it's a
per-display attribute, but it seems like the set of image-libraries loaded
is pretty much a global emacs thing.

[Whether or not a particular display supports images _at all_ is of course
display-dependent, and there's already a function for that --
display-images-p]

-Miles
-- 
We have met the enemy, and he is us.  -- Pogo

Re: display word wrapping

[David Kastrup]

Miles Bader <miles@gnu.org> writes:

> On Tue, Jun 08, 2004 at 02:08:09PM +0200, Kim F. Storm wrote:
> > If needed, it should be a separate function display-supports-image-type-p
> > (following the naming of display-supports-face-attributes-p).
> 
> Note that `display-supports-face-attributes-p' is called that because it's a
> per-display attribute, but it seems like the set of image-libraries loaded
> is pretty much a global emacs thing.
> 
> [Whether or not a particular display supports images _at all_ is of course
> display-dependent, and there's already a function for that --
> display-images-p]

PostScript images are only supported on X displays.  And I don't think
B&W displays can support much more than PBM, can they?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

OK, here's the patch with Vimage_types containing a list of potentially
supported image types, and an internal image_type_cache serving as a
cache alist.

Questions:

 - Should image_type_cache be exported to Lisp? OOH, it is an
   implementation detail; OTOH, exporting it would allow reseting an
   entry and trying a new library without restarting Emacs (if a load
   failed, of course; there's no support for unloading image libraries).

 - I'm initializing Vimage_types in syms_of_image() instead of
   init_image(), because I have the relevant #ifdef's already in place,
   but that's no a very good reason :)  Should I move it?

 - Is any way to do a defconst from C code? (There's no DEFCONST_LISP).
   Or is it irrelevant?

David, could you please try the patch and see if it addresses your
concerns?

                                                                Juanma




--- src/xdisp.c.orig	2004-06-08 13:20:45.000000000 +0200
+++ src/xdisp.c	2004-06-08 12:47:27.000000000 +0200
@@ -22262,5 +22262,6 @@
   DEFVAR_LISP ("image-types", &Vimage_types,
     doc: /* List of supported image types.
-Each element of the list is a symbol for a supported image type.  */);
+Each element of the list is a symbol for a potentially supported image type.
+To know whether it is really supported, you must use `image-type-available-p'.  */);
   Vimage_types = Qnil;
 
--- src/image.c.orig	2004-06-07 13:14:55.000000000 +0200
+++ src/image.c	2004-06-08 13:09:57.000000000 +0200
@@ -606,4 +606,8 @@
 static struct image_type *image_types;
 
+/* Cache for delayed-loading image types.  */
+
+Lisp_Object image_type_cache;
+
 /* The symbol `xbm' which is used as the type symbol for XBM images.  */
 
@@ -630,5 +634,5 @@
 /* Function prototypes.  */
 
-static void define_image_type P_ ((struct image_type *type));
+static Lisp_Object define_image_type P_ ((struct image_type *type, int loaded));
 static struct image_type *lookup_image_type P_ ((Lisp_Object symbol));
 static void image_error P_ ((char *format, Lisp_Object, Lisp_Object));
@@ -638,19 +642,35 @@
 				       Lisp_Object));
 
+#define CACHE_IMAGE_TYPE(type, status) \
+  do { image_type_cache = Fcons (Fcons (type, status), image_type_cache); } while (0)
+
+#define ADD_SUPPORTED_IMAGE_TYPE(type) \
+  do { Vimage_types = Fcons (type, Vimage_types); } while (0)
 
 /* Define a new image type from TYPE.  This adds a copy of TYPE to
-   image_types and adds the symbol *TYPE->type to Vimage_types.  */
+   image_types and adds (*TYPE->type, loaded) to image_type_cache.  */
 
-static void
-define_image_type (type)
+static Lisp_Object
+define_image_type (type, loaded)
      struct image_type *type;
+     int loaded;
 {
-  /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
-     The initialized data segment is read-only.  */
-  struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
-  bcopy (type, p, sizeof *p);
-  p->next = image_types;
-  image_types = p;
-  Vimage_types = Fcons (*p->type, Vimage_types);
+  Lisp_Object success;
+
+  if (!loaded)
+    success = Qnil;
+  else
+    {
+      /* Make a copy of TYPE to avoid a bus error in a dumped Emacs.
+         The initialized data segment is read-only.  */
+      struct image_type *p = (struct image_type *) xmalloc (sizeof *p);
+      bcopy (type, p, sizeof *p);
+      p->next = image_types;
+      image_types = p;
+      success = Qt;
+    }
+
+  CACHE_IMAGE_TYPE(*type->type, success);
+  return success;
 }
 
@@ -1789,4 +1809,31 @@
   }
 
+/* Load a DLL implementing an image type.
+   The `image-library-alist' variable associates a symbol,
+   identifying  an image type, to a list of possible filenames.
+   The function returns NULL if no library could be loaded for
+   the given image type, or if the library was previously loaded;
+   else the handle of the DLL.  */
+static HMODULE
+w32_dynaload (Lisp_Object libraries, Lisp_Object library_id)
+{
+  HMODULE library = NULL;
+
+  if (CONSP (libraries) && NILP (Fassq (library_id, image_type_cache)))
+    {
+      Lisp_Object dlls = Fassq (library_id, libraries);
+
+      if (CONSP (dlls))
+        for (dlls = XCDR (dlls); CONSP (dlls); dlls = XCDR (dlls))
+          {
+            CHECK_STRING_CAR (dlls);
+            if (library = LoadLibrary (SDATA (XCAR (dlls))))
+              break;
+          }
+    }
+
+  return library;
+}
+
 #endif /* HAVE_NTGUI */
 
@@ -3489,11 +3536,10 @@
 DEF_IMGLIB_FN (XImageFree);
 
-
 static int
-init_xpm_functions (void)
+init_xpm_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libXpm.dll")))
+  if (!(library = w32_dynaload (libraries, Qxpm)))
     return 0;
 
@@ -5589,19 +5635,10 @@
 
 static int
-init_png_functions (void)
+init_png_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  /* Ensure zlib is loaded.  Try debug version first.  */
-  if (!LoadLibrary ("zlibd.dll")
-      && !LoadLibrary ("zlib.dll"))
-    return 0;
-
   /* Try loading libpng under probable names.  */
-  if (!(library = LoadLibrary ("libpng13d.dll"))
-      && !(library = LoadLibrary ("libpng13.dll"))
-      && !(library = LoadLibrary ("libpng12d.dll"))
-      && !(library = LoadLibrary ("libpng12.dll"))
-      && !(library = LoadLibrary ("libpng.dll")))
+  if (!(library = w32_dynaload (libraries, Qpng)))
     return 0;
 
@@ -6247,11 +6284,9 @@
 
 static int
-init_jpeg_functions (void)
+init_jpeg_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libjpeg.dll"))
-      && !(library = LoadLibrary ("jpeg-62.dll"))
-      && !(library = LoadLibrary ("jpeg.dll")))
+  if (!(library = w32_dynaload (libraries, Qjpeg)))
     return 0;
 
@@ -6684,9 +6719,9 @@
 
 static int
-init_tiff_functions (void)
+init_tiff_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libtiff.dll")))
+  if (!(library = w32_dynaload (libraries, Qtiff)))
     return 0;
 
@@ -7104,9 +7139,9 @@
 
 static int
-init_gif_functions (void)
+init_gif_functions (Lisp_Object libraries)
 {
   HMODULE library;
 
-  if (!(library = LoadLibrary ("libungif.dll")))
+  if (!(library = w32_dynaload (libraries, Qgif)))
     return 0;
 
@@ -7881,7 +7916,70 @@
  ***********************************************************************/
 
+#ifdef HAVE_NTGUI
+/* Image types that rely on external libraries are loaded dynamically
+   if the library is available.  */
+#define CHECK_LIB_AVAILABLE(image_type, init_lib_fn) \
+  define_image_type (image_type, init_lib_fn (libraries))
+#else
+#define CHECK_LIB_AVAILABLE(image_type, init_lib_fn) \
+  define_image_type (image_type, TRUE)
+#endif /* HAVE_NTGUI */
+
+DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 2, 2, 0,
+       doc: /* Initialize image library implementing image type TYPE.
+Return non-nil if TYPE is a supported image type.
+
+Image types pbm and xbm are prebuilt; other types are loaded here.
+Libraries to load are specified in alist LIBRARIES (usually, the value
+of `image-library-alist', which see.  */)
+  (type, libraries)
+{
+  Lisp_Object tested;
+
+  /* Don't try to reload the library.  */
+  tested = Fassq (type, image_type_cache);
+  if (CONSP (tested))
+    return XCDR (tested);
+
+#if defined (HAVE_XPM) || defined (MAC_OS)
+  if (EQ (type, Qxpm))
+    return CHECK_LIB_AVAILABLE(&xpm_type, init_xpm_functions);
+#endif
+
+#if defined (HAVE_JPEG) || defined (MAC_OS)
+  if (EQ (type, Qjpeg))
+    return CHECK_LIB_AVAILABLE(&jpeg_type, init_jpeg_functions);
+#endif
+
+#if defined (HAVE_TIFF) || defined (MAC_OS)
+  if (EQ (type, Qtiff))
+    return CHECK_LIB_AVAILABLE(&tiff_type, init_tiff_functions);
+#endif
+
+#if defined (HAVE_GIF) || defined (MAC_OS)
+  if (EQ (type, Qgif))
+    return CHECK_LIB_AVAILABLE(&gif_type, init_gif_functions);
+#endif
+
+#if defined (HAVE_PNG) || defined (MAC_OS)
+  if (EQ (type, Qpng))
+    return CHECK_LIB_AVAILABLE(&png_type, init_png_functions);
+#endif
+
+#ifdef HAVE_GHOSTSCRIPT
+  if (EQ (type, Qpostscript))
+    return CHECK_LIB_AVAILABLE(&gs_type, init_gs_functions);
+#endif
+
+  /* If the type is not recognized, avoid testing it ever again.  */
+  CACHE_IMAGE_TYPE(type, Qnil);
+  return Qnil;
+}
+
 void
 syms_of_image ()
 {
+  Vimage_types = Qnil;
+
   QCascent = intern (":ascent");
   staticpro (&QCascent);
@@ -7917,4 +8015,5 @@
   staticpro (&Qpostscript);
 #ifdef HAVE_GHOSTSCRIPT
+  ADD_SUPPORTED_IMAGE_TYPE(&Qpostscript);
   QCloader = intern (":loader");
   staticpro (&QCloader);
@@ -7929,11 +8028,14 @@
   Qpbm = intern ("pbm");
   staticpro (&Qpbm);
+  ADD_SUPPORTED_IMAGE_TYPE(Qpbm);
 
   Qxbm = intern ("xbm");
   staticpro (&Qxbm);
+  ADD_SUPPORTED_IMAGE_TYPE(Qxbm);
 
 #if defined (HAVE_XPM) || defined (MAC_OS)
   Qxpm = intern ("xpm");
   staticpro (&Qxpm);
+  ADD_SUPPORTED_IMAGE_TYPE(Qxpm);
 #endif
 
@@ -7941,4 +8043,5 @@
   Qjpeg = intern ("jpeg");
   staticpro (&Qjpeg);
+  ADD_SUPPORTED_IMAGE_TYPE(Qjpeg);
 #endif
 
@@ -7946,4 +8049,5 @@
   Qtiff = intern ("tiff");
   staticpro (&Qtiff);
+  ADD_SUPPORTED_IMAGE_TYPE(Qtiff);
 #endif
 
@@ -7951,4 +8055,5 @@
   Qgif = intern ("gif");
   staticpro (&Qgif);
+  ADD_SUPPORTED_IMAGE_TYPE(Qgif);
 #endif
 
@@ -7956,6 +8061,8 @@
   Qpng = intern ("png");
   staticpro (&Qpng);
+  ADD_SUPPORTED_IMAGE_TYPE(Qpng);
 #endif
 
+  defsubr (&Sinit_image_library);
   defsubr (&Sclear_image_cache);
   defsubr (&Simage_size);
@@ -7985,50 +8092,12 @@
 }
 
-
-#ifdef HAVE_NTGUI
-/* Image types that rely on external libraries are loaded dynamically
-   if the library is available.  */
-#define IF_LIB_AVAILABLE(init_lib_fn)  if (init_lib_fn())
-#else
-#define IF_LIB_AVAILABLE(init_func)    /* Load unconditionally */
-#endif /* HAVE_NTGUI */
-
 void
 init_image ()
 {
   image_types = NULL;
-  Vimage_types = Qnil;
-
-  define_image_type (&xbm_type);
-  define_image_type (&pbm_type);
-
-#if defined (HAVE_XPM) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_xpm_functions)
-    define_image_type (&xpm_type);
-#endif
-
-#if defined (HAVE_JPEG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_jpeg_functions)
-    define_image_type (&jpeg_type);
-#endif
-
-#if defined (HAVE_TIFF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_tiff_functions)
-    define_image_type (&tiff_type);
-#endif
-
-#if defined (HAVE_GIF) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_gif_functions)
-    define_image_type (&gif_type);
-#endif
+  image_type_cache = Qnil;
 
-#if defined (HAVE_PNG) || defined (MAC_OS)
-  IF_LIB_AVAILABLE(init_png_functions)
-    define_image_type (&png_type);
-#endif
-
-#ifdef HAVE_GHOSTSCRIPT
-  define_image_type (&gs_type);
-#endif
+  define_image_type (&xbm_type, TRUE);
+  define_image_type (&pbm_type, TRUE);
 
 #ifdef MAC_OS
--- lisp/image.el.orig	2004-06-07 13:13:55.000000000 +0200
+++ lisp/image.el	2004-06-08 02:06:53.000000000 +0200
@@ -49,4 +49,15 @@
 a non-nil value, TYPE is the image's type.")
 
+;;;###autoload
+(defvar image-library-alist nil
+  "Alist of image types vs external libraries needed to display them.
+
+Each element is a list (IMAGE-TYPE LIBRARY...), where the car is a symbol
+representing a supported image type, and the rest are strings giving
+alternate filenames for the corresponding external libraries to load.
+They are tried in the order they appear on the list; if none of them can
+be loaded, the running session of Emacs won't display the image type.
+No entries are needed for pbm and xbm images; they're always supported.")
+;;;###autoload (put 'image-library-alist 'risky-local-variable t)
 
 (defun image-jpeg-p (data)
@@ -112,6 +123,6 @@
   "Value is non-nil if image type TYPE is available.
 Image types are symbols like `xbm' or `jpeg'."
-  (and (boundp 'image-types) (not (null (memq type image-types)))))
-
+  (and (fboundp 'init-image-library)
+       (init-image-library type image-library-alist)))
 
 ;;;###autoload
--- lisp/term/w32-win.el.orig	2004-06-07 13:16:33.000000000 +0200
+++ lisp/term/w32-win.el	2004-06-08 02:06:58.000000000 +0200
@@ -1262,4 +1262,12 @@
 	    (error "Font not found")))))
 
+;;; Set default known names for image libraries
+(setq image-library-alist
+      '((xpm "libXpm-nox4.dll" "libxpm.dll")
+        (png "libpng13d.dll" "libpng13.dll" "libpng12d.dll" "libpng12.dll" "libpng.dll")
+        (jpeg "jpeg62.dll" "libjpeg.dll" "jpeg-62.dll" "jpeg.dll")
+        (tiff "libtiff3.dll" "libtiff.dll")
+        (gif "libungif.dll")))
+
 ;;; arch-tag: 69fb1701-28c2-4890-b351-3d1fe4b4f166
 ;;; w32-win.el ends here

Re: display word wrapping

[David Kastrup]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> OK, here's the patch with Vimage_types containing a list of
> potentially supported image types, and an internal image_type_cache
> serving as a cache alist.
> 
> Questions:
> 
>  - Should image_type_cache be exported to Lisp? OOH, it is an
>    implementation detail; OTOH, exporting it would allow reseting an
>    entry and trying a new library without restarting Emacs (if a
>    load failed, of course; there's no support for unloading image
>    libraries).

Does not sound like it should be a variable for general use.  Whether
to export it into the Lisp name space at all for the sake of
debugging is a different question.

> David, could you please try the patch and see if it addresses your
> concerns?

Oh, but we already cleared that my concerns actually were not
reflected in any problems with the current preview-latex code base
(which incidentally would have worked), but rather were voiced on
theoretical grounds, like it not being nice to change documented
behavior from the Elisp manual in too surprising ways.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 13:39:52 +0200
David Kastrup <dak@gnu.org> wrote:

> Oh, but we already cleared that my concerns actually were not
> reflected in any problems with the current preview-latex code base
> (which incidentally would have worked), but rather were voiced on
> theoretical grounds,

Yeah, I know. I was trying to lure you into testing my patch ;)

                                                                Juanma

Re: display word wrapping

[Kim F. Storm]

Juanma Barranquero <jmbarranquero@wke.es> writes:

> OK, here's the patch with Vimage_types containing a list of potentially
> supported image types, and an internal image_type_cache serving as a
> cache alist.
> 
> Questions:
> 
>  - Should image_type_cache be exported to Lisp? OOH, it is an
>    implementation detail; OTOH, exporting it would allow reseting an
>    entry and trying a new library without restarting Emacs (if a load
>    failed, of course; there's no support for unloading image libraries).

IMO it is ok to have to restart emacs to try other libraries.
So it is not necessary to export it to lisp.

> 
>  - I'm initializing Vimage_types in syms_of_image() instead of
>    init_image(), because I have the relevant #ifdef's already in place,
>    but that's no a very good reason :)  Should I move it?

It is ok to do it in syms_of_image -- actually, while you are at it,
you should move Vimage_types and the associated DEFVAR_LISP from
xdisp.c to image.c; then you will declare and initialize the var
together as we normally do.

> 
>  - Is any way to do a defconst from C code? (There's no DEFCONST_LISP).
>    Or is it irrelevant?

For what purpose?

I suppose there is some flag you can set on the symbol to make it
constant -- cannot remember right now.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 14:05:12 +0200
storm@cua.dk (Kim F. Storm) wrote:

> IMO it is ok to have to restart emacs to try other libraries.
> So it is not necessary to export it to lisp.

Good.

> It is ok to do it in syms_of_image -- actually, while you are at it,
> you should move Vimage_types and the associated DEFVAR_LISP from
> xdisp.c to image.c;

Great! I was wondering why was it declared in xdisp.c...

> For what purpose?

Well, with the current impl, Vimage_types is a conceptually a constant;
it is initialized from data available at compilation and should never
change.

                                                                Juanma

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 11:33:55 +0200
storm@cua.dk (Kim F. Storm) wrote:

> David Kastrup <dak@gnu.org> writes:

> > Since image-types has been the only documented way of accessing that
> > information so far, I think there is some justification in not
> > changing its meaning before any package provider has had a chance to
> > adapt.
> 
> True.

What would that mean?

(make-obsolete-variable 'image-types
                        "use function `image-type-available-p'."
                        "22.1 (hopefully)")

Eh, perhaps we need a make-obsolete-future ;-)

But seriously, we're doing a feature-packed release, and we're talking
about a tiny change that will affect two or three packages out there in
very minor (and easily fixable, I think) ways.  Prereleases will take
months and months (based on previous experience)...

David, do you *really* feel it is a big problem? (You're the author of
one of these packages, after all, and I'm not.)

                                                                Juanma

Re: display word wrapping

[Juanma Barranquero]

On 08 Jun 2004 08:38:00 +0200
David Kastrup <dak@gnu.org> wrote:

> If an image type is known, I hope that it is present as just its
> name, and _not_ as a cons cell (jpeg . t) or so.
> 
> If not, we have a major incompatibility to the previous behavior.  It
> is bad enough already what happens if somebody decides to cycle
> through that variable just to display supported image types.

Yes, in my patch, image-types is an alist with (symbol . flag) items,
instead of a list of symbols.  It was suggested making it so on this
thread a few messages ago.

I'm not going to make a strong point on this; whatever is decided, I'll
implement.

However...

  - Kim *almost* suggested getting rid of image-types altogether,
    because image-types is an implementation detail.

  - No single .el, .c or .h file from the distribution uses image-types
    (other than image.c, and xdisp.c in which it is defined).

  - A Google search of "image-types" on gnu.emacs.* shows just one or
    two modules using image-types, in no particularly robust ways.

  - On delayed-loading environments (which could potentially be all, as
    delaying loading the libraries seems generally useful), image-types,
    even if a list as it was till now, can not be trusted to list
    supported image types, just supported types *already loaded*.

  - Delayed loading and image-types as it was till now, with the exact
    same semantics, are basically incompatible.  We could make:

    (defconst image-types '(jpeg tiff gif png postscript xpm pbm xbm))

    and use another variable as loaded-flags alist, but it's difficult
    to know what purpuse would it serve such image-types.  On Windows
    that wouldn't guarantee in any way that you can use png (for example),
    as libpng*.dll could be missing (that's the whole point of delayed
    loding).

  - You can easily fake an old style image-types with:

    (defvar my-image-types
     (let (result)
       (dolist (type '(xbm pbm xpm jpeg tiff gif png postcript) result)
         (when (image-type-available-p type)
           (push type result))))
     "Old style image-type.")

    and totally defeat delayed-loading.

> Can't we just keep the variable with the original meaning, and store
> the lookup cache data somewhere else?

We can keep the variable with a somewhat-similar meaning, but no with
the original one (unless we discard delayed-loading).  

                                                                Juanma

Re: display word wrapping

[Juanma Barranquero]

On Wed, 02 Jun 2004 23:17:21 +0100, Jason Rumney <jasonr@gnu.org> wrote:

BTW, I'm thinking of making this change to nt/INSTALL.

Comments?

                                                           /L/e/k/t/u



Index: INSTALL
===================================================================
RCS file: /cvsroot/emacs/emacs/nt/INSTALL,v
retrieving revision 1.20
diff -u -2 -r1.20 INSTALL
--- INSTALL	2 Sep 2003 22:36:30 -0000	1.20
+++ INSTALL	2 Jun 2004 23:42:06 -0000
@@ -1,6 +1,6 @@
 		      Building and Installing Emacs
-		on Windows NT/2000 and Windows 95/98/ME
+		on Windows NT/2K/XP and Windows 95/98/ME
 
-  Copyright (c) 2001 Free Software Foundation, Inc.
+  Copyright (c) 2001,2004 Free Software Foundation, Inc.
   See the end of the file for copying permissions.
 
@@ -32,5 +32,5 @@
   in the previous paragraph.
 
-  If you build Emacs on Windows 9X or ME, not on Windows 2000 or
+  If you build Emacs on Windows 9X or ME, not on Windows 2K/XP or
   Windows NT, we suggest to install the Cygwin port of Bash.
 
@@ -91,20 +91,35 @@
 * Optional image library support
 
-  To build Emacs with support for PNG images, the libpng and zlib
-  headers must be in the include path when the configure script is
-  run.  This can be setup using environment variables, or by
-  specifying --cflags -I...  options on the command-line to
-  configure.bat.  Similarly, the jpeg-6b, libXpm, tiff and libungif
-  headers need to be in the include path for support for those image
-  formats to work. The configure script will report whether it was
-  able to detect the headers.
-
-  To use the PNG support, zlib.dll (or zlibd.dll) and libpng.dll (or
-  libpng13.dll, or libpng13d.dll) must be on the PATH or in the same
-  directory as emacs.exe when Emacs is started. Similar instructions
-  apply for other image libraries. Note that tiff support depends on
-  the jpeg library. If you did not compile the libraries yourself, you
-  must make sure that the jpeg library you install is the same one
-  that the tiff library was compiled against.
+  In addition to its "native" image formats (pbm and xbm), Emacs can
+  support additional image types: xpm, tiff, gif, png and jpeg.  To build
+  Emacs with support for them, the corresponding headers must be in the
+  include path when the configure script is run.  This can be setup using
+  environment variables, or by specifying --cflags -I... options on the
+  command-line to configure.bat.  The configure script will report whether
+  it was able to detect the headers.
+
+  To use the external image support, the DLLs implementing the
+  functionality must be found when Emacs is started, either on the PATH, or
+  in the same directory as emacs.exe.  (You can also set an application
+  path on the Windows registry by adding an entry to
+  "HKCU\Software\Microsoft\Windows\CurrentVersion\App Paths", but that's
+  *not* recommended unless you know what you're doing.)  Failure to find a
+  library is not an error; the associated image format will simply be
+  unavailable.
+
+  Some image libraries have dependencies on one another, or from zlib.  For
+  example, tiff support depends on the jpeg library.  If you did not
+  compile the libraries yourself, you must make sure that any dependency is
+  in the PATH or otherwise accesible and that the binaries are compatible
+  (for example, that they were built with the same compiler).
+
+  Binaries for the image libraries (among many others) can be found at
+  GnuWin32 (http://gnuwin32.sourceforge.net).  These are built with MinGW
+  and work better with GCC/MinGW builds of Emacs, like the official binary
+  tarballs for Windows.  Compatibility with MSVC is still weak and should
+  not be trusted in production environments; if you really need an
+  MSVC-compiled Emacs with image support, you should try to build the
+  required libraries with the same compiler (though it can be extremely
+  non-trivial, and we'll be interested on hearing of any such effort).
 
 * Building

Re: display word wrapping

[Jason Rumney]

Juanma Barranquero <lektu@mi.madritel.es> writes:

> Comments?

Just two.

> +  (You can also set an application
> +  path on the Windows registry by adding an entry to
> +  "HKCU\Software\Microsoft\Windows\CurrentVersion\App Paths", but that's
> +  *not* recommended unless you know what you're doing.)

If it is not recommended, we should leave it out. If the user "knows
what they are doing", they will not need to be told this.

> +  Some image libraries have dependencies on one another, or from zlib.

on zlib.

Re: display word wrapping

[Juanma Barranquero]

On Thu, 03 Jun 2004 08:43:59 +0100
Jason Rumney <jasonr@gnu.org> wrote:

> If it is not recommended, we should leave it out.

OK, removed.

> > +  Some image libraries have dependencies on one another, or from zlib.
> 
> on zlib.

OK (pesky English grammar, of lack of it thereof... ;)

Tonight I'll commit it.  I'm also going to add a short entry to
etc/PROBLEMS explaining the current issues with mixed-compiler setups.

Thanks,

                                                                Juanma

Re: display word wrapping

[Benjamin Riefenstahl]

Hi Juanma,

Juanma Barranquero <lektu@mi.madritel.es> writes:
> +  GnuWin32 (http://gnuwin32.sourceforge.net).  These are built with MinGW
> +  and work better with GCC/MinGW builds of Emacs, like the official binary
> +  tarballs for Windows.  [...]

Do you mean "work better with GCC/MinGW builds of Emacs than the
official binary ..."?

benny

Re: display word wrapping

[Juanma Barranquero]

On Thu, 03 Jun 2004 11:37:26 +0200
Benjamin Riefenstahl <Benjamin.Riefenstahl@epost.de> wrote:

> > +  GnuWin32 (http://gnuwin32.sourceforge.net).  These are built with MinGW
> > +  and work better with GCC/MinGW builds of Emacs, like the official binary
> > +  tarballs for Windows.  [...]
> 
> Do you mean "work better with GCC/MinGW builds of Emacs than the
> official binary ..."?

No. I meant: "work better with GCC/MinGW builds of Emacs, like (for
example) the official binary builds, which are made with GCC."

Care to suggest a better wording?

Thanks,

                                                                Juanma

Re: display word wrapping

[Benjamin Riefenstahl]

Hi Juanma,


> Benjamin Riefenstahl <Benjamin.Riefenstahl@epost.de> wrote:
>> > + GnuWin32 (http://gnuwin32.sourceforge.net).  These are built
>> > + with MinGW and work better with GCC/MinGW builds of Emacs, like
>> > + the official binary tarballs for Windows.  [...]

Juanma Barranquero <jmbarranquero@wke.es> writes:
> No. I meant: "work better with GCC/MinGW builds of Emacs, like (for
> example) the official binary builds, which are made with GCC."
>
> Care to suggest a better wording?

Ah, ok, the "better" threw me off.  You mention no alternative that
this is "better than."  How about:

  These are built with MinGW and so they work better than other
  compiles with the official builds of Emacs, which are also made with
  MinGW.


benny

Re: display word wrapping

[Juanma Barranquero]

On Mon, 07 Jun 2004 13:13:36 +0200
Benjamin Riefenstahl <Benjamin.Riefenstahl@epost.de> wrote:

> How about:
> 
>   These are built with MinGW and so they work better than other
>   compiles with the official builds of Emacs, which are also made with
>   MinGW.

Er, I'm not sure that's an improvement ;)

I think this is clear enough:

  Binaries for the image libraries (among many others) can be found at
  GnuWin32 (http://gnuwin32.sourceforge.net).  These are built with
  MinGW, and so are very compatible with GCC/MinGW builds of Emacs (like
  the official binary tarballs for Windows).  Compatibility with MSVC,
  on the other hand, is still weak and should not be trusted in
  production environments; if you really need an MSVC-compiled Emacs
  with image support, you should try to build the required libraries
  with the same compiler (though it can be extremely non-trivial, and
  we'll be interested on hearing of any such effort).

                                                                Juanma

Re: display word wrapping

[Benjamin Riefenstahl]

Hi Juanma,

Juanma Barranquero <jmbarranquero@wke.es> writes:
> I think this is clear enough:
>
>   [...]

Sounds very good ;-)

benny

Re: display word wrapping

[Peter Lee]

>>>> Juanma Barranquero writes:

    >> liburt (needed by ungif) fdlibm (needed by ungif) libgw32c
    >> (needed by ungif)

    Juanma> Which ungif are you using? The one I've found
    Juanma> (libungif-4.1.0b1) does not need/use URT.

I'm using libungif-4.1.0b1.

The requirements section at
http://gnuwin32.sourceforge.net/packages/libungif.htm states:

-------------------------------------------------------------

Requirements

* All required packages from GnuWin32, i.e. excluding msvcrt.dll, perl,
etc, are included in the Setup program and the dependencies.

* MS-Windows 95 / 98 / ME / NT / 2000 / XP with msvcrt.dll. If
msvcrt.dll is not in your Windows/System folder, get it from Microsoft
or by installing Internet Explorer 4.0 or higher

* liburt

* fdlibm

* libgw32c (for developing with the LibUnGif library)

-------------------------------------------------------------

It does say that all dependencies are included in the setup, but when
I did my install I installed them (liburt, fdlibm, libgw32c)
seperately as I didn't see that.

It could be the requirements section is out of date, but I seem to
recall that gifs weren't working until I installed those seperate
packages.

Re: display word wrapping

[Eli Zaretskii]

> From: Peter Lee <pete_lee@swbell.net>
> Date: Tue, 01 Jun 2004 11:34:38 -0500
> 
> * liburt
> 
> * fdlibm
> 
> * libgw32c (for developing with the LibUnGif library)

fdlibm is free software, but I don't know about liburt.

As for libgw32c, the comment in parens sounds like we don't need it?

Re: display word wrapping

[Juanma Barranquero]

On Tue, 01 Jun 2004 11:34:38 -0500, Peter Lee <pete_lee@swbell.net> wrote:

> * liburt

> It could be the requirements section is out of date, but I seem to
> recall that gifs weren't working until I installed those seperate
> packages.

According to the sources, liburt is only needed to support files in URT
format. Perhaps libungif uses it if compiled to do so, but AFAICS, the
libungif.dll included in the binary package doesn't seem to use it:

C:\...\bin> tdump libungif.dll | grep "Imports"
Imports             00008000  00000254
Bound Imports       00000000  00000000
Delayed Imports     00000000  00000000
Imports from msvcrt.dll
Imports from msvcrt.dll

                                                           /L/e/k/t/u

Re: display word wrapping

[Richard Stallman]

	Juanma> with several names, hoping for the best (for example, we
	Juanma> try to load libjpeg.dll, jpeg-62.dll and jpeg.dll, in that
	Juanma> order). That seems fragile.

Are these libraries free software?  Or, do they normally come
with the Windows kernel or the Windows compiler?
The GPL requires asking this question.

Re: display word wrapping

[Juanma Barranquero]

On Sat, 29 May 2004 13:02:40 -0400, Richard Stallman <rms@gnu.org> wrote:

> 	Juanma> with several names, hoping for the best (for example, we
> 	Juanma> try to load libjpeg.dll, jpeg-62.dll and jpeg.dll, in that
> 	Juanma> order). That seems fragile.
> 
> Are these libraries free software?

All of them are distributed at least as open source. None of them is GPL
or LGPL, though.

I think they're the same libraries used on GNU/Linux and Unix builds of
Emacs, BTW.

> Or, do they normally come
> with the Windows kernel or the Windows compiler?

No, and no.

                                                           /L/e/k/t/u

Re: display word wrapping

[Richard Stallman]

    > Are these libraries free software?

    All of them are distributed at least as open source.

Unfortunately that does not answer the question of whether they are
free software.  The criteria for open source are not the same as those
for free software; a program can fit one category and not the other.

In the GNU Project, we advocate free software, not open source.  "Open
source" is the slogan of a movement that was formed specifically to
reject (and hush up) our philosophy, so we never use that term here
except to explain the difference between the two movements.  See
http://www.gnu.org/philosophy/free-software-for-freedom.html.

To know that a program meets the standards of the open source movement
does not give us any useful information, because we still have to
check whether it is free software before we can recommend it.

    I think they're the same libraries used on GNU/Linux and Unix builds of
    Emacs, BTW.

I think we have verified that all those libraries are free software.
Could someone positively verify that these libraries used on Windows
are indeed the same ones?

Re: display word wrapping

[Jason Rumney]

Richard Stallman <rms@gnu.org> writes:

> 	Juanma> with several names, hoping for the best (for example, we
> 	Juanma> try to load libjpeg.dll, jpeg-62.dll and jpeg.dll, in that
> 	Juanma> order). That seems fragile.
>
> Are these libraries free software?  Or, do they normally come
> with the Windows kernel or the Windows compiler?
> The GPL requires asking this question.

They are all Free Software. They are the same image libraries that
the X version of Emacs links against, and you looked over all the
licenses several months ago and confirmed that they were all GPL
compatible.

Re: display word wrapping

[Eli Zaretskii]

> From: Jason Rumney <jasonr@gnu.org>
> Date: Mon, 31 May 2004 19:42:12 +0100
> >
> > Are these libraries free software?  Or, do they normally come
> > with the Windows kernel or the Windows compiler?
> > The GPL requires asking this question.
> 
> They are all Free Software. They are the same image libraries that
> the X version of Emacs links against, and you looked over all the
> licenses several months ago and confirmed that they were all GPL
> compatible.

That's true, but when I had a quick look at the libungif download page
for Windows, it seemed to be saying that 2 auxiliary libraries are
required, whose status I'm not sure about.  So perhaps there's still
something to check here.

Re: display word wrapping

[Juanma Barranquero]

On Tue, 01 Jun 2004 06:51:21 +0200
"Eli Zaretskii" <eliz@gnu.org> wrote:

> That's true, but when I had a quick look at the libungif download page
> for Windows, it seemed to be saying that 2 auxiliary libraries are
> required, whose status I'm not sure about.  So perhaps there's still
> something to check here.

I didn't download any of these and I'm able to compile Emacs and see
GIFs just fine.

                                                                Juanma

Re: display word wrapping

[Eli Zaretskii]

> Date: Tue, 01 Jun 2004 09:18:54 +0200
> From: Juanma Barranquero <jmbarranquero@wke.es>
> 
> I didn't download any of these and I'm able to compile Emacs and see
> GIFs just fine.

I take it that you've checked that your system didn't already have
those libraries, yes?

Even so, I think it could be useful to find out under what
circumstances those additional libraries are needed.  Perhaps there's
some rare set of conditions when Emacs does need them.

Re: display word wrapping

[Juanma Barranquero]

On Tue, 01 Jun 2004 22:40:32 +0200, "Eli Zaretskii" <eliz@gnu.org> wrote:

> I take it that you've checked that your system didn't already have
> those libraries, yes?

Yes.

> Even so, I think it could be useful to find out under what
> circumstances those additional libraries are needed.  Perhaps there's
> some rare set of conditions when Emacs does need them.

Theoretically, libungif should need them when showing Utah RLE images. 

                                                           /L/e/k/t/u

Re: display word wrapping

[Richard Stallman]

    That's true, but when I had a quick look at the libungif download page
    for Windows, it seemed to be saying that 2 auxiliary libraries are
    required, whose status I'm not sure about.  So perhaps there's still
    something to check here.

Could you check those two libraries, please?

Re: display word wrapping

[Kai Grossjohann]

storm@cua.dk (Kim F. Storm) writes:

> - Tramp reentrancy rework
>
>   This is really a bug-fix, but seems to require major changes.

Indeed.

>   Maybe we don't really need to do this for 21.4, if there is a
>   simpler way to simply avoid reentrancy (e.g. disable auto-revert in
>   remote buffers).

Yes, disabling auto-revert in remote buffers seems the way to go.

I wonder, how does Ange-FTP deal with this?  I can't remember any
code for queueing requests, so in theory it might suffer from similar
problems as Tramp.  One difference between Tramp and Ange-FTP is that
Tramp often invokes long-running shell commands (for inline
transfer), whereas Ange-FTP just sends a simple put/get for the file
transfer, and the transfer itself happens out of band.

Kai

Re: display word wrapping

[Luc Teirlinck]

Kai Grossjohann wrote:

   Yes, disabling auto-revert in remote buffers seems the way to go.

Since there appears to be consensus over this, I have disabled
auto-reverting of remote files.

Sincerely,

Luc.

Re: display word wrapping

[Richard Stallman]

    Since there appears to be consensus over this, I have disabled
    auto-reverting of remote files.

Thank you.

Could you please update the documentation of auto-revert at all levels?
I am not sure whether the manual mentions it.

Re: display word wrapping

[Luc Teirlinck]

Richard Stallman wrote:

   Could you please update the documentation of auto-revert at all levels?
   I am not sure whether the manual mentions it.

I believe that the only places that need to be (slightly) updated are
the ";;; Commentary" section of autorevert.el and man/files.texi.  I
can commit the following patches, if they look OK.

===File ~/files.texi-diff===================================
*** files.texi	02 Apr 2004 21:27:43 -0600	1.88
--- files.texi	28 May 2004 17:06:20 -0500	
***************
*** 894,901 ****
  Auto-Revert mode, Emacs periodically checks all file buffers and
  reverts any when the corresponding file has changed.  The local
  variant, Auto-Revert mode, applies only to buffers in which it was
! activated.  Checking the files is done at intervals determined by the
! variable @code{auto-revert-interval}.
  
  @node Auto Save
  @section Auto-Saving: Protection Against Disasters
--- 894,901 ----
  Auto-Revert mode, Emacs periodically checks all file buffers and
  reverts any when the corresponding file has changed.  The local
  variant, Auto-Revert mode, applies only to buffers in which it was
! activated.  Neither mode reverts remote files.  Checking the files is
! done at intervals determined by the variable @code{auto-revert-interval}.
  
  @node Auto Save
  @section Auto-Saving: Protection Against Disasters
============================================================

===File ~/autorevert-diff===================================
*** autorevert.el	27 May 2004 17:44:05 -0500	1.30
--- autorevert.el	28 May 2004 17:07:57 -0500	
***************
*** 36,43 ****
  ;; Auto-Revert Mode.  Both modes automatically revert buffers
  ;; whenever the corresponding files have been changed on disk.
  ;;
! ;; Auto-Revert Mode can be activated for individual buffers.
! ;; Global Auto-Revert Mode applies to all file buffers.
  ;;
  ;; Both modes operate by checking the time stamp of all files at
  ;; intervals of `auto-revert-interval'.  The default is every five
--- 36,46 ----
  ;; Auto-Revert Mode.  Both modes automatically revert buffers
  ;; whenever the corresponding files have been changed on disk.
  ;;
! ;; Auto-Revert Mode can be activated for individual buffers.  Global
! ;; Auto-Revert Mode applies to all file buffers. (If the user option
! ;; `global-auto-revert-non-file-buffers' is non-nil, it also applies
! ;; to some non-file buffers.  This option is disabled by default.)
! ;; Neither mode reverts remote files.
  ;;
  ;; Both modes operate by checking the time stamp of all files at
  ;; intervals of `auto-revert-interval'.  The default is every five
============================================================

Re: display word wrapping

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: 26 May 2004 16:50:24 +0200
> 
> But I think it would probably be shortsighted to tackle automatic
> word wrap before the Bidi branch has been merged as well: that one
> has a lot of potential for interference with word wrapping.

Out of curiosity: why do you think so?  That is, what aspects of
bidirectional support did you have in mind when you wrote that?

Re: display word wrapping

[David Kastrup]

"Eli Zaretskii" <eliz@gnu.org> writes:

> > From: David Kastrup <dak@gnu.org>
> > Date: 26 May 2004 16:50:24 +0200
> > 
> > But I think it would probably be shortsighted to tackle automatic
> > word wrap before the Bidi branch has been merged as well: that one
> > has a lot of potential for interference with word wrapping.
> 
> Out of curiosity: why do you think so?  That is, what aspects of
> bidirectional support did you have in mind when you wrote that?

Well, if you change writing direction in the line, then soft-wrapping
a line pushes out words from the _middle_ of the line into the next
line.  Or something.  I don't want to think too much about this or
I'll get a headache.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: 26 May 2004 21:39:25 +0200
> 
> Well, if you change writing direction in the line, then soft-wrapping
> a line pushes out words from the _middle_ of the line into the next
> line.

Well, actually this problem should not happen at all, since the
display engine (in its bidi variation) first reorders the characters,
i.e. it fetches characters in _visual_ order, and only after that
decides whether to wrap.

Anyway, I agree it's not useful to discuss that too much at this time.

Re: display word wrapping

[Henrik Enberg]

Miles Bader <miles@lsi.nec.co.jp> writes:

>   * A way to define how words are divided for the magic wrapping.  I
>     suppose it should use syntax tables in some way; I'm not sure are
>     they easy to use or not from the display engine's point of view?

This is teoretically language dependent.  e.g. Swedish and English don't
break words the same way.  Page-setting programs have tables for each
supported language.

Re: display word wrapping

[David Kastrup]

Henrik Enberg <henrik.enberg@telia.com> writes:

> Miles Bader <miles@lsi.nec.co.jp> writes:
> 
> >   * A way to define how words are divided for the magic wrapping.  I
> >     suppose it should use syntax tables in some way; I'm not sure are
> >     they easy to use or not from the display engine's point of view?
> 
> This is teoretically language dependent.  e.g. Swedish and English don't
> break words the same way.  Page-setting programs have tables for each
> supported language.

Miles is not talking about word breaking but word wrapping.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: display word wrapping

[Richard Stallman]

    I've always thought that since the display engine does `proper' wrapping
    for variable-width characters already (variable-width fonts, tabs, etc),
    which really seems to work awful darn well, maybe it wouldn't be too
    hard to trick it into treating words in the buffer as `characters' for
    wrapping purposes.

I am not sure what those words mean.  Do you mean word-wrapping at
redisplay time without use of real newline characters?

This causes problems, not just for C-n, but for all editing
facilities that have something to do with newlines.  So I think
we should try a different approach.

I think we should do this by extending auto-fill instead, making it
seamless, so that the buffer has real text with real newlines.

Re: display word wrapping

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     I've always thought that since the display engine does `proper' wrapping
>     for variable-width characters already (variable-width fonts, tabs, etc),
>     which really seems to work awful darn well, maybe it wouldn't be too
>     hard to trick it into treating words in the buffer as `characters' for
>     wrapping purposes.
> 
> I am not sure what those words mean.  Do you mean word-wrapping at
> redisplay time without use of real newline characters?

Yes.  Even window's notepad can do word wrapping without modifying the
buffer text...

> 
> This causes problems, not just for C-n, but for all editing
> facilities that have something to do with newlines.  So I think
> we should try a different approach.

It may cause (a lot of) problems when EDITING such text, but it makes perfect sense to have automatic word wrapping when you VIEW text that is a lot wider than your current window.  By default, redisplay already wraps long lines at character boundaries -- my idea was that it should be possible (and not too difficult) to let redisplay backtrack at the right edge (or left with bidi) to find a whitespace character, and do the automatic wrapping there instead.  One way to do it could be to put a virtual "space" display property on that space character with a width exactly equal to the number of pixels needed to reach the right edge of the window.


The same text again with wrapping at 80 columns:

It may cause (a lot of) problems when EDITING such text, but it makes perfect se
nse to have automatic word wrapping when you VIEW text that is a lot wider than
your current window.  By default, redisplay already wraps long lines at characte
r boundaries -- my idea was that it should be possible (and not too difficult) t
o let redisplay backtrack at the right edge (or left with bidi) to find a whites
pace character, and do the automatic wrapping there instead.  One way to do it c
ould be to put a virtual "space" display property on that space character with a
width exactly equal to the number of pixels needed to reach the right edge of th
e window.


And now with the proposed (virtual) word wrapping:

It may cause (a lot of) problems when EDITING such text, but it makes perfect
sense to have automatic word wrapping when you VIEW text that is a lot wider
than your current window.  By default, redisplay already wraps long lines at
character boundaries -- my idea was that it should be possible (and not too
difficult) to let redisplay backtrack at the right edge (or left with bidi) to
find a whitespace character, and do the automatic wrapping there instead.  One
way to do it could be to put a virtual "space" display property on that space
character with a width exactly equal to the number of pixels needed to reach the
right edge of the window.


> 
> I think we should do this by extending auto-fill instead, making it
> seamless, so that the buffer has real text with real newlines.

That is ok for editing, but for reading (e.g. this mail), virtual word
wrapping would be much simpler -- and problem-less.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: display word wrapping

[Miles Bader]

storm@cua.dk (Kim F. Storm) writes:
>> I am not sure what those words mean.  Do you mean word-wrapping at
>> redisplay time without use of real newline characters?
...
>> This causes problems, not just for C-n, but for all editing
>> facilities that have something to do with newlines.  So I think we
>> should try a different approach.
>
> It may cause (a lot of) problems when EDITING such text

_Will_ it actually cause problems when editing text?  Really the _only_
commands I can think of that would need adjusting would be C-n / C-p,
and they're not particularly hard.  Most columnar commands won't be a
problem, because users don't _expect_ them to somehow work except on
paragraph boundaries when editting word-wrapped text.

I think trying to make buffer-filled text `seamless' sounds _much_
harder than just doing it in redisplay and tweaking things like C-n.
The redisplay code can already more or less do the display operations
that are needed (andn unlike the old redisplay code, even really, really
long wrapped lines don't seem to have speed problems until you get to
absurd sizes that no real user text would ever approach).

-Miles
-- 
Come now, if we were really planning to harm you, would we be waiting here, 
 beside the path, in the very darkest part of the forest?

Re: display word wrapping

[Kai Grossjohann]

Miles Bader <miles@lsi.nec.co.jp> writes:

> _Will_ it actually cause problems when editing text?  Really the _only_
> commands I can think of that would need adjusting would be C-n / C-p,
> and they're not particularly hard.  Most columnar commands won't be a
> problem, because users don't _expect_ them to somehow work except on
> paragraph boundaries when editting word-wrapped text.

I think most users will expect C-a and C-e to be visual, too.

But beyond these, I also can't think of any.

Kai

Re: display word wrapping

[Richard Stallman]

    Yes.  Even window's notepad can do word wrapping without modifying the
    buffer text...

What happens in Windows (aside from trying to read the files it
produces) is not a major design criterion for Emacs or other GNU
packages.

    That is ok for editing, but for reading (e.g. this mail), virtual word
    wrapping would be much simpler -- and problem-less.

I agree that word-wrapping in display would be useful for viewing
unfilled files.  However, that raises the question of how we design
Emacs with both line-breaking and word-wrapping without making that
somewhat confusing.

Perhaps we can have one command which enables word-wrapping
in a read-only buffer and enables filling in a writable buffer.

Re: display word wrapping

[Karl Eichwalder]

Richard Stallman <rms@gnu.org> writes:

> I agree that word-wrapping in display would be useful for viewing
> unfilled files.

Gnus comes with various so called washing functions.  But even for
editing word-wrapping is important

> However, that raises the question of how we design Emacs with both
> line-breaking and word-wrapping without making that somewhat
> confusing.

I think the issue must be checked case by case.  Editing wikipedia
articles is an interesting case.  A mode for editing the Emacs wiki is
already available, but last time I tried (approx. 6 months ago) this
mode didn't work that good for wikipedia.org articles.

I know several people who are interested in using Emacs as an editor
for wikipedia.org.  accessing the article over http isn't that
important - you can use Emacs as an external editor for webbrowsers -
but dealing with the articles' source texts is.

Such an article looks as follows
( http://en.wikipedia.org/wiki/Central_Park ):

<table align = right width = 400><tr><td>[[image:Centralpark.jpg]]</table>
[[de:Central Park]] [[sv:Central Park]]


'''Central Park''' is a very large [[park]] (843 acres = 3.4 km², a rectangle of 2.5 miles/4 km by one-half mile/800 m) in [[Manhattan]], [[New York, New York|New York]], [[New York]], [[United States]].  An oasis for Manhattanites escaping from their [[skyscraper]]s, the park is well-known worldwide after its appearance in many movies and television shows.


The park was designed by [[Frederick Law Olmsted]] and [[Calvert Vaux]], who later created [[Brooklyn, New York|Brooklyn]]'s [[Prospect Park]]. While much of the park looks natural, it is in fact highly landscaped. It contains several artificial [[lake]]s, extensive [[walking track]]s, two [[ice-skating]] rinks, a wildlife sanctuary, and [[pasture|grass]]y areas used for myriad [[sport]]ing pursuits, as well as [[playground]]s for [[child]]ren. The park is a popular oasis for migrating [[bird]]s, and thus is popular with [[bird watcher]]s. The 9.7 km road circling the park is popular with joggers, bicyclists and [[inline skate]]rs, especially on weekends when automobile traffic is banned.

[...]
-=-=-=-=-=-=-=-=-=-=-=-=-=- cut here -=-=-=-=-=-=-=-=-=-=-=-=-=-

Of course, more contructs are allowed; e.g., lists:

*point 1
*point 2
*point 3

and, the horror, tables:

{|
| entry 1 | entry 2|
|-
| entry 3 | entry 4|
|}

-- 
                                                         |      ,__o
                                                         |    _-\_<,
http://www.gnu.franken.de/ke/                            |   (*)/'(*)

Re: display word wrapping

[Richard Stallman]

    > I agree that word-wrapping in display would be useful for viewing
    > unfilled files.

    Gnus comes with various so called washing functions.

Sorry, I don't understand.  I don't use Gnus.  Could you say
(generally) what these functions do, and also say explicitly the
conclusion we should draw from them?

Re: display word wrapping

[Kai Grossjohann]

Richard Stallman <rms@gnu.org> writes:

> Sorry, I don't understand.  I don't use Gnus.  Could you say
> (generally) what these functions do, and also say explicitly the
> conclusion we should draw from them?

Washing in Gnus means to change how the message is displayed by
editing the buffer displaying it.  The message itself is not changed,
only the buffer displaying it.  One of the washing functions affects
long lines: You receive a message which has long lines, you then
invoke the washing function, and Gnus will display the message with
newlines in the right places.  There are other washing functions, but
those are not relevant in this context.

So M-x view-file RET could, in principle, offer similar functions.

However, IMVHO, this is not a substitute for display word wrapping.

Kai

Re: display word wrapping

[Richard Stallman]

      One of the washing functions affects
    long lines: You receive a message which has long lines, you then
    invoke the washing function, and Gnus will display the message with
    newlines in the right places.  There are other washing functions, but
    those are not relevant in this context.

    So M-x view-file RET could, in principle, offer similar functions.

    However, IMVHO, this is not a substitute for display word wrapping.

Why not?

Re: display word wrapping

[Jason Rumney]

Richard Stallman <rms@gnu.org> writes:

>     Yes.  Even window's notepad can do word wrapping without modifying the
>     buffer text...
>
> What happens in Windows (aside from trying to read the files it
> produces) is not a major design criterion for Emacs or other GNU
> packages.

Windows Notepad is often held up as an example of the most
featureless text editor. I don't think Kim was saying Emacs should
copy notepad, the emphasis is on the "even".

Re: display word wrapping

[Stefan Monnier]

> Windows Notepad is often held up as an example of the most
> featureless text editor.

It's all relative.  From what I heard it has fairly good unicode support by
Emacs standards (such as bidi).

In any case, I think it would be good to have word-wrapping in the display.
But I also think we should postpone discussion to after-21.4.


        Stefan

Re: display word wrapping

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: 29 May 2004 23:11:59 -0400
> 
> > Windows Notepad is often held up as an example of the most
> > featureless text editor.
> 
> It's all relative.  From what I heard it has fairly good unicode support by
> Emacs standards (such as bidi).

Actually, the bidi support in Notepad is quite awful.  Of course,
it's much better than what Emacs currently has...

> In any case, I think it would be good to have word-wrapping in the display.
> But I also think we should postpone discussion to after-21.4.

Agreed on both counts.

Re: display word wrapping

[Richard Stallman]

    Windows Notepad is often held up as an example of the most
    featureless text editor. I don't think Kim was saying Emacs should
    copy notepad, the emphasis is on the "even".

That Windows Notepad does it this way
doesn't mean it is the right way to do things in Emacs.

Re: display word wrapping

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Thu, 27 May 2004 19:54:38 -0400
> 
> This causes problems, not just for C-n, but for all editing
> facilities that have something to do with newlines.

We could introduce a set of different commands that move by display
lines, and have an option to bind C-n etc. to those new commands.

Re: display word wrapping

[Richard Stallman]

    We could introduce a set of different commands that move by display
    lines, and have an option to bind C-n etc. to those new commands.

We could dothis, but this is a drastic change in what a text file is
supposed to be.  We would have to replace all the Emacs facilities
that deal with lines.  And in many programs (such as older versions of
Emacs) the files would be a pain to read.

So I think we should do word-wrapping by inserting newlines at the
right places.  That is the modular way to do it.  Everything else will
still work.