Re: [Gegl-developer] babl docs
On Fri, 2010-10-01 at 08:20 +0200, Martin Nordholts wrote: What specific C99 features are we talking about at all? Is there a specific feature that you guys would like to use? I like in particular * designated initializers Nice to have Indeed. * // comments Using /* */ for documentation and // for comments increases code readability since different comment semantics gets different syntax // comments are just plain ugly. I don't follow you on this one, but I could probably live with it. * mixed declarations and code Because variables can be declared closer to where they are used That leads to very ugly code. You can always open a new scope if you want to declare local variables. And perhaps you should split your code into smaller functions. I am strongly against this and think we should definitely continue to compile with '-Wdeclaration-after-statement' even if we decide that C99 code should in general be allowed. * new block scopes for selection and iteration statements So one can have for (int i = 0; i N; i++) i.e. not having to declare 'i' beforehand Yes, that is a nice feature. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] babl docs
On Thu, 2010-09-30 at 07:36 +0200, Martin Nordholts wrote: GIMP tends to claim C89 compatibility; we shouldnt assume more modern language standard than GIMP does. Hi Øyvind What problems do you expect us to get if we begin to rely on an 11 year old standard instead of a 21 year old standard? At what time has enough time passed for us to feel comfortable using the improvements in C99? As soon as enough compilers support it so that we don't loose platform compatibility. What specific C99 features are we talking about at all? Is there a specific feature that you guys would like to use? Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Small patch
Hi, some comments on your patch: +#include glib.h #include glib-object.h That's redundant as glib-object.h already includes this file for you. +inline gint get_band_size (gint size); Splitting the code into smaller functions is cool. But please consider to name the functions according to the file it lives in. So this would become gegl_processor_get_band_size(). This makes it easier to interpret stack traces and to discuss the code as it is clear where the function is located. +inline gint +get_band_size ( gint size ) Please format this as get_band_size (gint size). Also you will want to declare this function as static. And as far as I can see it should also be labeled G_GNUC_CONST as it doesn't examine any values except its parameters, and has no effects except its return value. I wouldn't declare it inline though. The compiler can better decide if it makes sense to inline this function. I am not happy about the way you reformatted the code in the class_init method: - g_object_class_install_property (gobject_class, PROP_NODE, - g_param_spec_object (node, -GeglNode, -The GeglNode to process (will saturate the provider's cache if the provided node is a sink node), -GEGL_TYPE_NODE, - G_PARAM_WRITABLE | - G_PARAM_CONSTRUCT)); + g_object_class_install_property ( +gobject_class, +PROP_NODE, +g_param_spec_object ( node, +GeglNode, +The GeglNode to process (will saturate the provider's +cache if the provided node is a sink node), +GEGL_TYPE_NODE, +G_PARAM_WRITABLE | +G_PARAM_CONSTRUCT)); Can we please keep the formatting here? If you want to reduce the code to fewer columns, you could break the blurb into multiple lines. Other than that, the patch looks good to me. If you incorporate my suggestions I will take care of getting the next version into trunk. Nice to see some work being done here. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] babl portability patches, and a test failure
Hi, On Tue, 2009-02-24 at 03:22 +, Gary V. Vaughan wrote: * for gnulib and autoconf to work properly, every .c file needs to #include config.h before anything else! Yes, definitely! And I even kept that in a separate patch! :) I have committed that part to trunk now. Actually, I changed it to include config.h as that's more correct than config.h. After all we are not including a system header here but a local one. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Question about the use of C99/gcc built-in math intrinsics within GEGL on gfloats
Hi, On Sun, 2008-09-14 at 11:30 +0200, Geert Jordaens wrote: Introducing the qualifier restrict will have some more checks to be done by the programmer and enabling the *-fstrict-aliasing* flag and the warning *-Wstrict-aliasing *would be advisable. I don't think enabling the strict-aliasing rules is advisable in general. It is too easy to break the rules and this might lead to bugs. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] #defining the restrict keyword
Hi, On Sat, 2008-09-13 at 11:21 -0400, Nicolas Robidoux wrote: Given that the restrict keyword may be defined by default for other compilers than gcc (it is in gcc http://developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Other-Builtins.html wouldn't the following be better? #ifndef restrict #ifndef G_GNUC_RESTRICT #if defined (__GNUC__) (__GNUC__ = 4) #define G_GNUC_RESTRICT __restrict__ #else #define G_GNUC_RESTRICT #endif #define restrict G_GNUC_RESTRICT #endif #endif Well, using plain restrict probably only works if C99 is enabled. And if C99 is enabled, then there's no need for this macro. So the easiest solution would certainly be to switch to C99 for GEGL. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Question about the use of C99/gcc built-in math intrinsics within GEGL on gfloats
Hi, On Fri, 2008-09-12 at 19:45 -0400, Nicolas Robidoux wrote: There is another C99/gcc built-in with the potential to speed up code a lot: the restrict keyword. See: http://www.cellperformance.com/mike_acton/2006/05/demystifying_the_restrict_keyw.html It looks like the restrict keyword could be easily wrapped into a macro that evaluates to restrict on compilers that support it and to on compilers where support for it is missing. So if we should decide that it is too early for using C99 features, we could still use restrict. We just need to add a configure check for it. We could even suggest that it is added to GLib as G_GNUC_RESTRICT. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Question about the use of C99/gcc built-in math intrinsics within GEGL on gfloats
Hi, regarding the use of C99 features, this is a pointer to the last time this question came up among the GLib developers: http://mail.gnome.org/archives/gtk-devel-list/2008-June/msg00020.html The thread linked from this mail might have some interesting arguments that we should consider. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Question about the use of C99/gcc built-in math intrinsics within GEGL on gfloats
Hi, I've filed an enhancement request for G_GNUC_RESTRICT: http://bugzilla.gnome.org/show_bug.cgi?id=552098 We should however not wait for this to be included in GLib. As GLib 2.18 has just been released, it will take a while before 2.20 hits the road. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Patch: GIO dependency made optionnal
Hi, On Sat, 2008-05-17 at 21:38 +0100, Øyvind Kolås wrote: On Sat, May 17, 2008 at 9:22 PM, Hubert Figuiere [EMAIL PROTECTED] wrote: Here is my initial patch for GEGL to make gio an optional dependency for those who run an already obsolete version of glib. I don't think this should be applied. An optional dependency in the GEGL core will cause code duplication and it bears the risk that versions of GEGL compiled against different versions of glib will behave differently. I don't see the point in doing this. This isn't only about obsolete versions of glib since there is no version having a GIO supporting all the features GeglBuffer would want it to, see bug #533547. Thus unless this gets resolved in glib we might have to fall back to regular posix calls anyways. If that happens, that is fine. But it should IMO not be optional. et's not make the code more complex for no good reason. At the time that GEGL will be useful enough to be actually used, glib with GIO will be available everywhere. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] speeding up GEGL operations in GIMP
Hi, On Thu, 2008-05-15 at 10:06 +0200, Sven Neumann wrote: Eventually he gimp-8bit extension should provide shortcuts for both gamma-corrected and linear 8bit - double conversions. 8bit - float conversions, of course. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
[Gegl-developer] speeding up GEGL operations in GIMP
Hi, currently the GEGL operations in GIMP are very slow. I have done some basic profiling yesterday and it appears that the main problem is the conversion from floating point to 8bit. Here is where the most time is being spent. So it doesn't really make much sense to optimize the operations in GIMP or the point filters in GEGL. We need to look at the babl conversions first. Here's an interesting paper that outlines how to do conversion from linear light floating point image data to 8-bit sRGB using a relatively small lookup table: http://mysite.verizon.net/spitzak/conversion/sketches_0265.pdf That is exactly the conversion that the tile sink executes. It would help us a lot if someone could implement this. The file extensions/gimp-8bit.c would probably be the right place to put this code. I am afraid I am not going to find time for this. Is anyone else interested in working on this? Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] BABL constructors speedup patch
Hi, On Mon, 2008-03-31 at 00:29 +0200, Jan Heller wrote: That is correct. I will definitely port the rest of the code to the new API, I'm just contemplating the best course of action. There are other related changes I would want to make and I was thinking I'd make them all at once. The less changes in one step, the better. This will make it easier for us to review the changes. Also in case that something breaks, it makes it a lot easier to isolate the change that caused the problem. So, if possible, please try to create many small patches instead of one large one. Thanks. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] BABL constructors speedup patch
Hi, On Sat, 2008-03-29 at 17:57 +0100, Jan Heller wrote: Here is an another patch I came up with while playing with BABL code. It concerns constructors of the BABL classes. The original code prepares the whole instance of a class before trying to insert it into the BABL database. The insertion might fail in case there is an instance of the same name already present in the database. This is a bit wasteful, so the patch tries to test the preexistent instance as soon as possible. Very nice. I have committed this change to SVN. BTW, wasn't there some additional changes needed after your last patch? If I remember correctly, some lists still need to be ported to the new list API. Is that correct? Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Hi, need any help?
Hi, On Mon, 2008-03-10 at 00:35 +0100, Jan Heller wrote: Glad to hear it :) So here is the patch. Essentially, it implements coalesced hashing as BablHashTable structure and uses it as the babl database data structure. The code is not long and is quite self-explanatory I believe, although I wouldn't mind adding a few comments if needed. Thanks for the patch. Here are some comments that I collected when reading through the patch: I would prefer if this functionality could be added in new files babl-list.[ch] and babl-hash-table.[ch]. Also the hash-table functions should be prefixed with babl_hash_table_ instead of just babl_hash_ as we should keep the babl_hash prefix for hash functions. Then I don't understand the purpose of babl_list_each_NEW(). This is just temporary, isn't it? Oh, and if Pippin wouldn't object against making babl depend on glib, we wouldn't have to duplicate all this code... Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] gegl ops CVS
Hi, On Fri, 2006-08-04 at 21:22 -0500, Cody Russell wrote: I was talking to pipping tonight because I made a gegl op for outputting into a GdkPixbuf and wanted to import it into cvs. He mentioned that we should import gegl-operations into its own module in cvs and it needs to be organized in a meaningful way, so I volunteered to post on the list and get some discussion on this going. The module could be organized into different subdirectories: gegl-operations/ input/ output/ filters/ Does it really make sense to have a separate CVS module for this? After all the two modules will have to be in sync anyway so it would probably be easier to keep the operators in the gegl CVS module. We could still release two distinct tarballs from this source if that is desired. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] error: 'GeglOp' undeclared
Hi, Florent Monnier [EMAIL PROTECTED] writes: Here the exemple 'test-01.c' I'm trying to compile: What code is that? Why do you think it would work? Because I found it in the documentation (the reference). In the outdated reference that is not any longer online or in the current one? I explained earlier that the API is subject to changes. We have done a major code cleanup this summer. None of the old example files will work any longer. The source code as found in CVS has working test files (which aren't really meant as examples though). Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] ./configure fatal warning
Hi, Florent Monnier [EMAIL PROTECTED] writes: Where should I search for intltool.m4? I just figured that we are not currently doing any internationalization. So intltool and glib-gettextize are actually not needed at the moment. I have changed autogen.sh accordingly. Sven ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
