GNOME Bugzilla – Bug 125141
Deprecate gimpmisc and gimpmiscui
Last modified: 2004-12-22 21:47:04 UTC
libgimpmisc has helped rationalise some of the shared code in plug-ins, but it also contains quite a few things of questionable value. Before 2.0, we should hide the libgimpmisc API from 3rd party plug-in designers, and anticipate removing it completely between now and (say) 3.0. To do this, we need to not install it or its headers, and links statically for the core plug-ins that use it. Since this is an API change this should be done before the first 2.0 prerelease. The alternative is to live with this for at least two release cycles. Cheers, Dave.
Afaik there is no such thing as a libgimpmisc. The code that is in the files gimpmisc.c and gimpmiscui.c is added to the existing libraries (libgimp and libgimpui). I do agree however that the API should not be visisble to 3d party developers, especially since some of the code is hopefully going to dissapear soon (like GimpFixmePreview stuff) and for other code the API isn't stable enough. The alternative (the one I prefer) for static libraries is to go the same road as GTK does for it's deprecated widgets: you disable the stuff, unless someone explicitely wants to use it: #undef GIMP_DISABLE_USE_AT_YOUR_OWN_RISK ;) And of we don't have to distribute these header files.
Sven's comments in a mail to the list: Someone should check what functions are in libgimpmisc.h and libgimpmiscui.h. Everything that seems very useful and has a clean API should be moved to a final place (and be documented). The rest of the functions can stay. We then change the Makefile.am not to install these headers and link the functionality directly into the plug-ins (see how it is done for libgck). This also means that the plug-ins using these functions will have to include the header files from the source tree explicitely.
I believe that David Odin is looking into this. David, could you add a comment here to let people know what you're doing? Cheers, Dave.
Changing subject to reflect reality. Dave.
I'm working right now on merging the gimp_pixel_fetcher_get_pixel and gimp_pixel_fetcher_get_pixel2 routine. This will become one routine with the following API: void gimp_pixel_fetcher_get_pixel (GimpPixelFetcher *pf, gint x, gint y, guchar *pixel); The `wrapmode' has to be set by a new function call: void gimp_pixel_fetcher_set_wrapmode (GimpPixelFetcher *pf, GimpPixelFetcherWrapMode wrapmode); typedef enum (PIXEL_NONE, PIXEL_WRAP, PIXEL_SMEAR, PIXEL_BLACK} GimpPixelFetcherWrapMode; The default value will be PIXEL_NONE
Please make sure that the enum values live in the GIMP namespace. I would suggest to use GIMP_PIXEL_FETCHER_WRAP_NONE, GIMP_PIXEL_FETCHER_WRAP_AROUND, GIMP_PIXEL_FETCHER_WRAP_SMEAR, GIMP_PIXEL_FETCHER_WRAP_BLACK Or perhaps rename to GimpPixelFetcherEdgeMode and go for GIMP_PIXEL_FETCHER_EDGE_NONE, GIMP_PIXEL_FETCHER_EDGE_WRAP, GIMP_PIXEL_FETCHER_EDGE_SMEAR, GIMP_PIXEL_FETCHER_EDGE_BLACK
I agree that EdgeMode is a far better name.
Can I get a status update on this, please? Maurits, David - what's the state of play? Dave.
Dave, I've made a few changes so the gimp_pixel_fetcher routines are now as described above. I also changed the plug-ins accordingly and will commit this stuff soon. I think the API for this set of routines is now pretty clean so I will start to document it. On very short notice, however, we should go with the solution as proposed by Sven: put this stuff asap in a libgck like library, so we can finally get our pre-release out. Note: I haven't looked at how to do this yet. Too busy with buying presents and writing poems for `Sinterklaas' ;)
It doesn't make any sense to do move useful routines into an internal library only for the purpose of a pre-release. There must not be an API change after the prerelease. Please move the pixel fetcher routines to new files before you start to document them. And please attach a header file for review to this report.
I'll take care of the gimpmiscui part. This means: <ul> <li>remove gimp_parameter_settings_new() and fix the plug-ins which use it.</li> <li>remove gimp_plug_in_get_path(), and fix the plug-ins which use it.</li> <li>Rename gimpfixmepreview and put it in a better place, since it looks we can keep it anyway.</li>
gimp_parameter_settings_new() has been removed (mostly by reveting the commit that introduced it. gimp_fixme_preview has been renamed gimp_old_preview and now live in its own, not installed, library in /plug-ins/libgimpoldpreview. I have yet to understand the rational behind gimp_plug_in_get_path(). To me this function is only a wrapper around gimp_gimprc_query() with a message displayed if the path is NULL, and nothing more. I think we should simply fix gimp_gimprc_query() so it displays the erreur message itself and remove completly gimp_plug_in_get_path(). Anyway, only 3 plug-ins use it for now.
IMO gimprc_query() should stay as it is. It's a PDB procedure and the C wrapper shouldn't add any functionality that is not in the PDB. I vote for moving this code back to the 3 plug-ins that use it. BTW: Very nice job on libgimpmisc(ui)! Your help is very much appreciated.
OK. I've done what Sven has been suggesting. So the libgimp/gimpmiscui.[ch] files have been removed from the CVS tree. This bug is now partly fixed. I guess Maurits will go on with the pixel fetchers part.
Hi, Maurits just sent me a mail about this - he's been very busy recently, and was just on his way out the dorr for the holidays when he replied. He thinks that he's more or less done, but he'd like someone else to check over what he's done before committing. David, do you think you could have a look? He sent me all the files he changed (unfortunately not a diff), so I'll attach them all here. Cheers, Dave.
Created attachment 22678 [details] gimpmisc.c
Created attachment 22679 [details] gimpmisc.h
Created attachment 22680 [details] curve bend
Created attachment 22681 [details] displace
Created attachment 22682 [details] edge
Created attachment 22683 [details] gflare
Created attachment 22684 [details] illusion
Created attachment 22685 [details] shift
To sum up: The files above were changed my Maurits in line with the comments earlier on in the report. All that remains is to move the remaining functions to another file with a better name. The files changed, with their paths, are: libgimpwidgets/gimpmisc.[ch] plug-ins/common/curve_bend.c plug-ins/common/displace.c plug-ins/common/edge.c plug-ins/common/illusion.c plug-ins/common/shift.c plug-ins/gflare/gflare.c Somewhere in the middle of all that, I realised it would have been quicker to just over-write the local files here and do a CVS diff. I'm not sure how old Maurits' CVS sandbox is, though - there may be conflicts to resolve in some of the plug-ins. Cheers, Dave.
We need a diff attached to this report so it can be sanely reviewed. If someone wants to help with this report, please create such a diff and attach it.
Created attachment 22710 [details] [review] diff of Maurits' changes.
2003-12-26 Sven Neumann <sven@gimp.org> * libgimp/gimpmisc.[ch]: applied a modified version of a patch from Maurits Rijk that cleans up the remaining API (bug #125141). * plug-ins/common/curve_bend.c * plug-ins/common/displace.c * plug-ins/common/edge.c * plug-ins/common/illusion.c * plug-ins/common/shift.c * plug-ins/gflare/gflare.c: changed accordingly. This leaves the following to do: (1) The functions should be moved to a different file. (2) The functions must be documented using inline comments. (3) The API reference manual needs to include the new file(s). Bumping to 2.0 since this isn't a prerequisite for the pre-release.
libgimp/gimpmisc.[ch] have been split into libgimp/gimppixelfetcher.[ch] and libgimp/gimpregioniterator.[ch]. We still need to documents all these functions.
Missing documentation applies to quite a few libgimp functions. It's a bug, but a different issue. The gimpmisc one is FIXED: 2004-01-12 DindinX <david@dindinx.org> * libgimp/gimpmisc.[ch]: remove these files * libgimp/gimppixelfetcher.c: * libgimp/gimppixelfetcher.h: * libgimp/gimpregioniterator.c: * libgimp/gimpregioniterator.h: and created these ones. Regarding #125141, gimpmisc was split into gimppixelfetcher and gimpregioniterator. * libgimp/Makefile.am: * libgimp/gimp.h: modified accordingly.