MapObject and Lighting have been fixed not to use XPM icons. I think
we should get this uglyness removed from the remaining 3 plug-ins
for 2.2. Perhaps this comment brings the issue back to some hackers'
minds...

It seems that the following plug-ins are still using XPM files:

 Fractal Explorer
 ImageMap
 PageCurl
 Rotate Colormap

Would be nice to get this done for 2.2, but it is of course not a blocker.
Should be 4 simple fixes, any volunteers?

Imagemap is almost finished. I volunteer to do the other plug-ins as well.

Shouldn't block the 2.2 release. If someone wants to do it nevertheless, it
could still be done...

Imagemap was indeed fixed when I looked at it.
Here are .png files for the 3 plug-ins left, tared as pluginname/images/pngfiles.
Please tell me if I forgot something.

Created attachment 33756 [details]
FractalExplorer png icons

Created attachment 33757 [details]
pagecurl png icons

Created attachment 33758 [details]
rcm png icons

Created attachment 33759 [details] [review]
Adds 'images' to SUBDIRS

Are these icons just the XPM files converted to PNG or did you draw new ones (or
pick suitable ones from a stock icon collection)?

What I had in mind for Pagecurl was to use a single icon and rotate/flip it
accordingly. That will however have to wait for the next development cycle since
it needs GdkPixbuf routines that have only just been added. As soon as we depend
on GTK+-2.6, we should do this change.

At least for FractalExplorer, replacing the XPM icons with PNG ones is probably
not the right approach here. What should be done here instead is:

(1) Remove the About dialog (that would remove the need for logo.h as well).
(2) Either replace the remaining XPM icons with stock icons from the GTK+ and
    GIMP stock icon collections. There should suitable icons available.

However I just played a bit with FractalExplorer and this part of the GUI seems
to be rather. What is needed here is review this part of the user interface.
There should probably be a separate bug report opened for this. I don't think
it makes sense to deal with the icons until the list interface has been
improved. Since there's a GtkList being used here, the first thing to do would
be to port to GtkTreeView.

  
                

Ooops, I edited this text after finishing it and obviously I managed to mess it
up quite a bit. That should have read:

(1) Remove the About dialog (that would remove the need for logo.h as well).
(2) Replace the remaining XPM icons with stock icons from the GTK+ or
    GIMP stock icon collections. There should be suitable icons available.

However I just played a bit with FractalExplorer and this part of the GUI seems
to be rather broken. [...]

Sven: I converted the files. I tried to draw a simple one (mini_cross from
Fractal Explorer) and saw the result was the same (looking at it, I did not
compare the files).

I can redraw them if it can lead to better results, but someone will have to
explain to me:
- how do I pick a color like #96588E38AEBA (is that #968EAE, #5838BA, something
else? or can I work in the gimp with 16bit colors?)
- what options do I choose when saving the .png file, to be consistent with
other plug-in icons?

Btw: I used convert (from ImageMagick 5.5.7) file.xpm file.png, with no options.

Since I see that you are working on this stuff and to get you going, I've now
done the following change to CVS:

2004-11-14  Sven Neumann  <sven@gimp.org>

	* plug-ins/gimpressionist/Makefile.am: fixed typo.

	* plug-ins/pagecurl/Makefile.am
	* plug-ins/pagecurl/curl[0-7].png: added PNG versions of the XPM
	icons used by the PageCurl plug-in. Added rules to build a header
	file that can be used to get rid of the XPM files (bug #140202).

Getting this Makefile stuff right is definitely not trivial, so I guessed you
could need some help here. This change allows you to port Pagecurl from
GimpPixmap to GdkPixbuf/GtkImage. What you will need to do is to include the
created header file, add code that creates pixbufs (gdk_pixbuf_new_from_inline)
and replace the GimpPixmap with a GtkImage that renders the pixbufs.

Let's ignore FractalExplorer for now. It doesn't make sense to work on it as
long as it still uses a GtkList. When it is ported to GtkTreeView, stock icons
can be used and the XPM files can just be removed.

As soon as Pagecurl is done, we can have a look at RCM. I guess we will need to
add a stock icon factory there so that we can use gtk_button_new_from_stock().

Created attachment 33763 [details] [review]
Ports Pagecurl from GimpPixmap to GdkPixbuf/GtkImage.

Thanks for that patch. I've done a couple of smaller changes, for example
unref'ing the pixbufs so we don't leak them...

2004-11-14  Sven Neumann  <sven@gimp.org>

	* plug-ins/pagecurl/pagecurl.c: applied a patch from Karine Proot
	that replaces the XPM icons with pixbufs (bug #140202).

	* plug-ins/pagecurl/curl[0-7].xpm: removed.

I suggest that we fix RCM, open a new bug report for FractalExplorer and close
this one. This report starts to become confusing. It deals with too many things.

Preparing the changes to RCM:

2004-11-14  Sven Neumann  <sven@gimp.org>

	* configure.in
	* plug-ins/rcm/Makefile.am
	* plug-ins/rcm/images/Makefile.am
	* plug-ins/rcm/images/rcm-360.png
	* plug-ins/rcm/images/rcm-a-b.png
	* plug-ins/rcm/images/rcm-ccw.png
	* plug-ins/rcm/images/rcm-cw.png: added PNG versions of the XPM
	icons used by the RCM plug-in. Added rules to build a header file
	that can be used to get rid of the XPM files (bug #140202).

rcm-360 should probably have better been renamed to rcm-all and rcm-a-b would
better be called rcm-swap. Will change that as soon as the changes to the code
have been done...

I guess the pagecurl plug-in should actually have a preview widget instead of
the icons.

Here comes the rcm patch, thanks to Sven's help.

Two new files are needed : rcm_stock.[c,h]
They can be found at http://edhel.nerim.net/gimp/

Note: There is no Gtk-CRITICAL anymore.

Created attachment 33874 [details] [review]
Makes rcm plug-in use a stock icon factory.

Very nice, thank you. Applied with some minor modifications (mainly added
missing i18n to the stock items).

2004-11-17  Sven Neumann  <sven@gimp.org>

	* plug-ins/rcm/Makefile.am
	* plug-ins/rcm/rcm_callback.c
	* plug-ins/rcm/rcm_dialog.c
	* plug-ins/rcm/rcm_stock.[ch]: applied a patch from Karine Proot
	that replaces the XPM icons with stock icons (bug #140202).

	* plug-ins/rcm/pixmaps/*.xpm: removed.

Pygimp is also using a "xpm" icon  - actually, the logo data is hard-coded 
into the source file, and gdk XPM-dealing api is used. 
 
I just switched it to an external .png 

Created attachment 33877 [details] [review]
pygimp - removes inline logo and changes code to load external .png

Yosh made some commits tonight that are not in anoncvs yet - patch to gimpfu.py
may need to be tunned to apply cleanly.

Created attachment 33878 [details]
pygimp: gimpfu logo

Created attachment 33927 [details] [review]
Updated patch removing inline pygimp logo.

The pygimp patches are obsolete, the code in CVS uses a new logo, in an external
png.

So the only plug-in that's left to deal with is FractalExplorer.

There should definitely be no XPMs in 2.4

There is currently a problem with FractalExplorer, as it segfaults (see bug
#172347). I tried to track the bug down without success.

Sven suggested it would be a good idea to port FractalExplorer to GtkTreeView,
and that would probably lead to rewrite the part that segfaults.

Since the segfault has been fixed, it would be a good time now to finally port
this beast to GtkTreeView and get rid of the remaining XPM icons.

Created attachment 48061 [details] [review]
Uses stock icons instead of xpm ones

The patch is big because it ports FractalExplorer from GtkList to GtkTreeView.

I also changed the behavior of the plug-in when you remove all fractals :
previously it was adding a new fractal so the list would never get empty, now
it lets you empty the list but you still have the last one deleted in the
preview (and in current_obj) to be used if necessary.

That's why I removed so many functions - they were all there for that trick, if
I'm not mistaking...

The stock icons used are GTK_STOCK_NO and GTK_STOCK_YES to notice read-only
fractals from others, that's the closest I found. The third xpm icon was used
for the "list never empty trick" and thus has not been replaced.

Yay! Finally no more XPMs!

2005-06-21  Sven Neumann  <sven@gimp.org>

	* plug-ins/FractalExplorer/FractalExplorer.c: applied a patch from
	Karine Delvare that ports the list to GtkTreeView and replaces the
	XPM icons with stock icons. Fixes bug #140202.

	* plug-ins/FractalExplorer/Makefile.am
	* plug-ins/FractalExplorer/pix_data.h: removed this file.

Thanks a lot for this patch! There are a few minor issues with the new UI. Ask
me on IRC if you are interested to do further work on this plug-in...

Created attachment 48132 [details] [review]
UI improvements

Removed About dialog.
The list will now scroll when you browse it.
Removed stock icons that were confusing.

2005-06-22  Sven Neumann  <sven@gimp.org>

        * plug-ins/FractalExplorer/Dialogs.c
        * plug-ins/FractalExplorer/FractalExplorer.[ch]: merged in a patch
        from Karine Delvare with further UI improvements (see bug #140202).

One thing still jumped at my attention. The title of the list asks the user to
double-click a fractal. That isn't quite right. Perhaps that frame should be
removed entirely.

Sorry, I forgot to mention that logo.h should be removed now the About gialog is
gone.

Created attachment 48174 [details] [review]
Removes unwanted frame.

2005-06-22  Sven Neumann  <sven@gimp.org>

	* plug-ins/FractalExplorer/FractalExplorer.c: applied a patch from
	Karine Delvare that removes an obsolete frame.

	* plug-ins/FractalExplorer/Makefile.am
	* plug-ins/FractalExplorer/logo.h: removed logo now that the About
	dialog is gone.

There's still a problem with the new treeview. If you switch to the notebook
showing the list of fractals, the first fractal in the list is being selected,
thus changing the currently setup fractal. This needs to be avoided somehow. The
old code seemed to require the user to double-click to actually activate a
fractal. There are probably other ways to solve this. The current fractal could
for example be in the list and selected.

Created attachment 48310 [details] [review]
Activate fractal on click rather than select.

Fiddling with button events is not a good idea. Why don't you connect to
"row-activated" instead?

"row-activated" needs a double-click to activate a fractal, which is harder to
discover (unless I bring the frame back with its title). That is indeed much
simpler and what I had done first, but I was under the impression this was not
good enough.

Double-click is the generally accepted metaphor for activating something
in a list where you can also select things. IMHO it's absolutely unacceptable
that selecting (single clicking) an item with the mouse behaves different
that selecting it with the keyboard.

Please use "row-activated", everything else would be inconsistent from
most other lists on the desktop.

Created attachment 48317 [details] [review]
Activates fractal on double-click rather than select

Do we need the tip about double-click back?

Comment on attachment 48317 [details] [review]
Activates fractal on double-click rather than select

Looks fine. Please commit.

Created attachment 48337 [details] [review]
Add an apply button and remove the frame around the scrolled window.