Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 09/01/2010 12:12 PM, Øyvind Kolås wrote: On Wed, Sep 1, 2010 at 6:35 AM, Martin Nordholtsense...@gmail.com wrote: My point was that we either decide that GEGL should render just like legacy, in which case we would need to implement both kinds of color layer modes in GEGL, or we decide that some incompatibilities are OK, in which case we don't need the legacy color modes in GIMP. -- Right now, the Overlay layer mode actually renders differently in GEGL than in legacy, so we would also have to introduce an (obsolete) version of Overlay. (The legacy Overlay is exactly identical to legacy Soft Light, but the GEGL Overlay is different from the GEGL Soft Light.) GEGL will do compositing in linear light RGB, while GIMP will do compositing in sRGB Yes you're right, and personally I think this is fine, as long as we do full and proper color management in GIMP 3.0. / Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/30/2010 09:34 PM, Martin Nordholts wrote: Is it decided that GEGL will not support the legacy modes, or should they be implemented in GEGL as well to retain backward compatability? There is one big issue left to settle: whether e.g. a Screen pixel composited onto nothing should result in the pixel or 'nothing'. Right the result is the unmodified pixel, as in the SVG 1.2 compositing model. In legacy GIMP compositing, the result is 'nothing' (except when the layer is the bottom-most layer...). I'm fine with using our legacy compositing model, as long as there exists a consistent compositing model without special cases to describe it. I will try to come up with such a compositing model unless someone else gives it a go before me. I hadn't noticed that yet: All non-GEGL modes now actually use deleted content for compositing, so that could probably be called broken. (Paint something on bottom layer that has effect on top layer's compositing, then delete content in bottom layer to get checkerboard. There are changes in alpha, but the deleted drawing still shows through) But if what you wrote was an answer to my questions, it was much more than I bargained for. ;) I just meant if GEGL should also implement the current HSV-based color layer modes for (then) old files, or if GEGL will stay with just supporting the LCH modes. Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 09/01/2010 02:37 AM, Rupert Weber wrote: On 08/30/2010 09:34 PM, Martin Nordholts wrote: Is it decided that GEGL will not support the legacy modes, or should they be implemented in GEGL as well to retain backward compatability? There is one big issue left to settle I just meant if GEGL should also implement the current HSV-based color layer modes for (then) old files, or if GEGL will stay with just supporting the LCH modes. Sorry, I got carried away :) My point was that we either decide that GEGL should render just like legacy, in which case we would need to implement both kinds of color layer modes in GEGL, or we decide that some incompatibilities are OK, in which case we don't need the legacy color modes in GIMP. If we end up using the SVG 1.2 compositing model, we have decided we won't be compatible. But if we change to use our legacy compositing model in GEGL too, then for simplicity we should also make sure to render legacy files with GEGL like the legacy rendering engine does it, i.e. implement all layer modes. Right now, the Overlay layer mode actually renders differently in GEGL than in legacy, so we would also have to introduce an (obsolete) version of Overlay. (The legacy Overlay is exactly identical to legacy Soft Light, but the GEGL Overlay is different from the GEGL Soft Light.) / Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
so I've played around with babl a bit, and quite a few questions came up. Is there a separate babl mailing list? Getting the conversions symmetric is possible, but at a cost. Without any other changes, increasing the bits from 16 to 20 gives perfect round-trip accuracy, but it also becomes about 4x slower; probably due to the increased size (x32) of lookup tables. So that would definitely need more work. Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On Mon, 2010-08-30 at 20:25 +0200, Rupert Weber wrote: so I've played around with babl a bit, and quite a few questions came up. Is there a separate babl mailing list? gegl-developer is the place for discussing babl things. Sven ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/30/2010 08:19 PM, Rupert Weber wrote: On 08/24/2010 07:46 AM, Martin Nordholts wrote: * Only show the legacy color modes when an image that already uses them is the active image (we either show all four, even if an image only uses one). Haven't got around doing that, yet, but have question. Deciding when to activate the legacy modes is easy: Whenever an image is loaded that uses them. But when to deactivate? - As soon as there is no layer left using legacy modes? (I.e. you couldn't switch modes back and forth. Once switched away from legacy modes, the menu item would be gone.) - When closing the image or switching to another image that doesn't have legacy modes. I.e. changing the active image will change the dropdown. - When closing the last image that uses legacy modes. I.e. you could activate legacy modes for all images by keeping a foot-in-the-door image open. - Only when closing GIMP. While I could think of use-cases for all of them, I think it boils down to how strongly we would want to discourage from using the legacy modes. I think you could add a gboolean was_loaded_with_obsolete_modes to GimpImage, set it when an image is loaded, and then check that boolean in gimp_layer_tree_view_set_image(). So whenever an image that was loaded with obsolete layer modes present becomes the active image, we show the obsolete modes in the menu. Slightly different from my initial solution proposal in other words. / Martin Is it decided that GEGL will not support the legacy modes, or should they be implemented in GEGL as well to retain backward compatability? There is one big issue left to settle: whether e.g. a Screen pixel composited onto nothing should result in the pixel or 'nothing'. Right the result is the unmodified pixel, as in the SVG 1.2 compositing model. In legacy GIMP compositing, the result is 'nothing' (except when the layer is the bottom-most layer...). I'm fine with using our legacy compositing model, as long as there exists a consistent compositing model without special cases to describe it. I will try to come up with such a compositing model unless someone else gives it a go before me. / Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 24-08-10 11:51, Øyvind Kolås wrote: On Tue, Aug 24, 2010 at 9:58 AM, Rupert Weberg...@leguanease.org wrote: On 08/24/2010 04:20 AM, David Gowers wrote: I hope you're not associating the quite suboptimal way in which GIMP currently uses GEGL, with BABL's speed or lack of speed. BABL just processes raw pixel buffers. A converter function just accepts a source and a destination pointer, along with a pixel count, and should convert that number of pixels. It doesn't have any heavy architecture or processing, aside from what may be in each individual converter function looking at your code in gimpcolorspace.c, making that code work with BABL looks like it's pretty much a case of cut+paste, modify the way the input is referred to, add some registration code. (getting your layer mode code to USE that, is a different issue, and I agree that would be non-trivial, although you might get significant speed gains from it because of greatly reduced function call overhead -- probably about as much as you describe for inlining below.) As I indicated, I'll be happy to make the babl integration of those conversions my next little project, but I also expect a bunch of questions to come up in the process (color management comes to mind). Color management does not have much to do with this, since the pixels are already assumed to be in a well defined pixel format and color space. Babl is nothing more than a collection of conversion functions, meta data about the color spaces and data types used and the ability to assess the conversion speed and quality of these conversions at runtime. Thus as long as the claim as to what pixel format a buffer is in is correct, use of babl is correctly color managed. /Øyvind K. Well how well defined is the format that babl receives and sends back. According to the web page at : http://www.gimp.org/release-notes/gimp-2.4-cm.html. The intent is to work in Gimp using the sRGB color space, however one can ignore this. On the other side there babl is supposed to work in scRGB (http://www.gegl.org/babl). Maybe I didn't look hard enough but I didn't find a real conversion from sRGB(8bit) to scRGB(16bit), neigther in GIMP nor in babl as described in *http://www.colour.org/tc8-05/Docs/colorspace/61966-2-2NPa.pdf.* In the babl extensions a CIE model is present, it takes as e reference white D65 while more common would be the reference white E (http://www.brucelindbloom.com). I assume that the sRGB/scRGB have reference white D50. Has anyone an idea what is the reference white for the RGB color model in BABL or GIMP? Are the reference white and the chromaticity coordinates not missing in the BABL model? Geert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 11:11 PM, Sven Neumann wrote: And no, I didn't ask you to work on the GEGL modes, I just ask you (or actually anyone) to use and improve babl when it comes to pixel format conversions. Maybe I'm just paranoid, but I'm afraid that might turn out to be equivalent. Using existing babl is out of the question because it is just way too slow. Leaves improving babl, which I will be glad to do (or attempt) next. I just don't expect that to be a quickie. Some questions already came up and I'm sure there will be more. The algorithms will probably have to be twiddled with, maybe just a few more bits precision, maybe some floating point/integer/LUT combination -- I just don't know yet. Once that's done, all current Lab conversions in GIMP can easily be ported to babl. If it turns out I'm wrong and it's much easier and quicker than I anticipate -- fantastic. Throw those conversion right back out and it's babl all the way. If I'm not, then it's a feature that could have been. Regards Rupert PS: As David already noted, the last patch doesn't have the conversions in libgimpcolor anymore -- so no additional API. ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 07:46 AM, Martin Nordholts wrote: With your patch applied, there are two variants of the color-related layer modes. Legacy and obsolete broken variants that new images don't need, and your correct useful new variants. We are working hard on improving the UI, and having two variants of the same layer mode always available, where one is broken and one works, is simply not good enough. I suggest we: * Only show the legacy color modes when an image that already uses them is the active image (we either show all four, even if an image only uses one). * Add an (obsolete) suffix to the legacy ones (only shown in the UI, not in the API) * Remove the (LCH) suffix in your new layer modes (only in the UI, not in the API) Yes, I almost forgot about that. I always considered the names so far as working titles only. What you suggest seems reasonable to me. (And the current GEGL modes suggest that dropping the old modes is the intended route, anyway). If that's how we do it, I'll need a little help, as I have no UI/Gtk experience. Or maybe at least a pointer where to look at / which way to go. I could imagine having two prebuilt menus which are swapped vs. dynamically adding removing items. -- The toolbox dropdown for the painting mode should probably never change. (And if someone says, hey that's easy, I'll do that in a couple of minutes, please, don't let me stop you :) ) Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 04:20 AM, David Gowers wrote: I hope you're not associating the quite suboptimal way in which GIMP currently uses GEGL, with BABL's speed or lack of speed. BABL just processes raw pixel buffers. A converter function just accepts a source and a destination pointer, along with a pixel count, and should convert that number of pixels. It doesn't have any heavy architecture or processing, aside from what may be in each individual converter function looking at your code in gimpcolorspace.c, making that code work with BABL looks like it's pretty much a case of cut+paste, modify the way the input is referred to, add some registration code. (getting your layer mode code to USE that, is a different issue, and I agree that would be non-trivial, although you might get significant speed gains from it because of greatly reduced function call overhead -- probably about as much as you describe for inlining below.) As I indicated, I'll be happy to make the babl integration of those conversions my next little project, but I also expect a bunch of questions to come up in the process (color management comes to mind). On the other hand I see bablification of gimp_composite_* as part of a general overhaul of that code, not limited to the LCH layer modes. And that raises the question if that makes sense or if that time wouldn't be better spent on the GEGL modes. I only have one thing to criticize about this patch: It adds a single whitespace error :) Ha, I believe 'single' is a new positive record for me... :o) Regards Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On Tue, Aug 24, 2010 at 9:58 AM, Rupert Weber g...@leguanease.org wrote: On 08/24/2010 04:20 AM, David Gowers wrote: I hope you're not associating the quite suboptimal way in which GIMP currently uses GEGL, with BABL's speed or lack of speed. BABL just processes raw pixel buffers. A converter function just accepts a source and a destination pointer, along with a pixel count, and should convert that number of pixels. It doesn't have any heavy architecture or processing, aside from what may be in each individual converter function looking at your code in gimpcolorspace.c, making that code work with BABL looks like it's pretty much a case of cut+paste, modify the way the input is referred to, add some registration code. (getting your layer mode code to USE that, is a different issue, and I agree that would be non-trivial, although you might get significant speed gains from it because of greatly reduced function call overhead -- probably about as much as you describe for inlining below.) As I indicated, I'll be happy to make the babl integration of those conversions my next little project, but I also expect a bunch of questions to come up in the process (color management comes to mind). Color management does not have much to do with this, since the pixels are already assumed to be in a well defined pixel format and color space. Babl is nothing more than a collection of conversion functions, meta data about the color spaces and data types used and the ability to assess the conversion speed and quality of these conversions at runtime. Thus as long as the claim as to what pixel format a buffer is in is correct, use of babl is correctly color managed. /Øyvind K. -- «The future is already here. It's just not very evenly distributed» -- William Gibson http://pippin.gimp.org/ http://ffii.org/ ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On Tue, Aug 24, 2010 at 1:52 AM, Rupert Weber g...@leguanease.org wrote: On 08/22/2010 02:45 PM, Sven Neumann wrote: New code in GIMP should use babl for pixel format conversion. There's no need to introduce new API for that as we have babl which is available to the core and plug-ins and provides a much superior API. The short answer is: No. I won't do that. For the long answer see further down below. (Sorry if this post becomes a bit longish) First about the current state of affairs: I posted the last update to the patch to http://bugzilla.gnome.org/attachment.cgi?bugid=325564 From my point of view, it is now done. Some performance numbers, comparing redraw times of legacy modes to the new LCH modes and to current GEGL LCH modes: (Tested with a very large picture to get measurable numbers; still these are ca. numbers, obtained with a stop watch) Mode | Legacy | New LCH | GEGL/babl -++-+-- Hue | 3.6 | 6.4 | 396 Saturation | 4.6 | 6.2 | 405 Color | 4.7 | 4.1 | 431 Value/Lightn.| 3.5 | 4.1 | 416 So I'm in the ball park of the legacy modes, Color is even a little faster. Compared to current GEGL/babl the new modes are about 60 to 100 times faster. (Yes, no typo) Due to the hacky way it is implemented, to enable using the GEGL code paths alongside the legacy code paths without breaking the legacy code paths. Additional conversions back and forth between 8bit and 32bit float are needed, and the processing is not done in non-optimal amounts. As to accuracy, these are the round-trip pixel errors for Lab and LCH conversions: Error after round-trip [in 8bit RGB space]: L*a*b* L*C*H* 0: 16561783 ( 98.716%) 0: 16527659 ( 98.513%) 1: 214941 ( 1.281%) 1: 248244 ( 1.480%) 2: 492 ( 0.003%) 2: 1313 ( 0.008%) 3: 0 ( 0.000%) 3: 0 ( 0.000%) The worst we get are off-by-two errors. You won't notice without diff'ing two layers. If you don't just stack no-op layers on top of each other, out-of-gamut errors will be *far* greater than these. Such an error should be unacceptable, the conversion code for CIE Lab in babl are symmetric. So, as I already said, I consider the patch done now. And then there is babl. I feel very bad about that request. Because I expect it to be the first step in a relatively short sequence of if-we-do-that-why-don't-we's that will end with these modes not getting in but rather be added to the GEGL agenda. As that is effectively what you are asking me to do: work on the GEGL modes instead (or duplicate them, which would be even sillier). But that is not what I signed up for. The idea was to get something done and usable now. Not something that will be great in some uncertain future. When this is done I will be glad to take a look at babl and see if the conversions can somehow be integrated. But I don't expect that to be a trivial task. The patch is here. Now, and it works. The conversions add 17k of code. Once GEGL takes over, they'll simply removed again. No one gets hurt. I suspect that the code is already triply duplicated now then, the original GIMP CIE Lab code in app/base/cpercep.c , it's copy in babl/ectensions/CIE.c and your conversion code. If you add your conversion code to babl as well it will be picked instead of the existing conversion, if it is both symmetric and faster than the existing implementations. /Øyvind K. -- «The future is already here. It's just not very evenly distributed» -- William Gibson http://pippin.gimp.org/ http://ffii.org/ ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 12:21 PM, Øyvind Kolås wrote: Such an error should be unacceptable, the conversion code for CIE Lab in babl are symmetric. and the problems begin... that's what I meant I suspect that the code is already triply duplicated now then, the original GIMP CIE Lab code in app/base/cpercep.c , it's copy in babl/ectensions/CIE.c and your conversion code. I'm surprised to find that code in app/base. As you say, it's the same as in babl. But I don't see that code being called from anywhere within GIMP. Why is it even there? It seems we can reduce by one copy then. ;) If you add your conversion code to babl as well it will be picked instead of the existing conversion, if it is both symmetric and faster than the existing implementations. But it isn't fully symmetric. For the intended purpose, i.e. conversion from/to 8bit RGB, that's not a problem. For a more general purpose routine, it might be. So while the integer conversions might be portable to babl in some way shape or form, it won't be a simple 1:1 copy/paste. ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 10:09 AM, Rupert Weber wrote: On 08/24/2010 07:46 AM, Martin Nordholts wrote: I suggest we: * Only show the legacy color modes when an image that already uses them is the active image (we either show all four, even if an image only uses one). If that's how we do it, I'll need a little help, as I have no UI/Gtk experience. I imagine the solution to be along the lines of: 1. Adding a new function to app/core/gimpimage.c called gimp_image_uses_obsolete_layer_modes() 2. Add a with_obsolete_modes boolean parameter to gimp_paint_mode_menu_new() 3. Create two variants of the paint mode menu, one with and one without the obsolete modes, in gimp_layer_tree_view_init() 4. Show the correct version in of the paint mode menu in gimp_layer_tree_view_set_image() based on return result of gimp_image_uses_obsolete_layer_modes() An additonal comment: * Add the obsolete layer modes after the new layer modes in the paint mode menu, your latest patch has the old ones before the new ones, but the new ones are the most relevant / Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 04:20 AM, David Gowers wrote: I hope you're not associating the quite suboptimal way in which GIMP currently uses GEGL, with BABL's speed or lack of speed. Just did a quick test: 1Mio random pixels passed to babl as one buffer (=one function call) vs. passing the same buffer pixel by pixel (=1Mio calls) to the integer conversions. babl still comes out 35x slower. ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 05:21 PM, Martin Nordholts wrote: I imagine the solution to be along the lines of: [...] Cool, thanks. That gives me a good headstart. But I probably won't be able to dive in there before the weekend. * Add the obsolete layer modes after the new layer modes in the paint mode menu, your latest patch has the old ones before the new ones, but the new ones are the most relevant Agreed. Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On Tue, Aug 24, 2010 at 4:30 PM, Rupert Weber g...@leguanease.org wrote: On 08/24/2010 04:20 AM, David Gowers wrote: I hope you're not associating the quite suboptimal way in which GIMP currently uses GEGL, with BABL's speed or lack of speed. Just did a quick test: 1Mio random pixels passed to babl as one buffer (=one function call) vs. passing the same buffer pixel by pixel (=1Mio calls) to the integer conversions. babl still comes out 35x slower. This is expected since there are no additional fast paths in babl for the specific conversion you probably used. At the moment babl will have to do quite a bit of internal work going via double precision floats and manually permuting components. The purpose of babl is to provide an API that provides correct color conversions and allows accelerating these with extensions that are conformance tested and performance ranked at runtime. Your integer code could be added as such an extension and will be chosen automatically if it is good enough (or if the environment variable BABL_TOLERANCE is set to for instance 0.02 or higher, indicating that babl shouldnt be as strict about which fast paths are permitted.) Fast conversions added to babl do not only benefit the conversion of the implemented source/target pixelformats since babl can also string together such conversions in a chain). /Øyvind K. -- ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On Tue, 2010-08-24 at 17:06 +0200, Rupert Weber wrote: I suspect that the code is already triply duplicated now then, the original GIMP CIE Lab code in app/base/cpercep.c , it's copy in babl/ectensions/CIE.c and your conversion code. I'm surprised to find that code in app/base. As you say, it's the same as in babl. But I don't see that code being called from anywhere within GIMP. Why is it even there? It is used by the RGB-Indexed conversion in core/gimpimage-convert.c Sven ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
Hi, On Tue, 2010-08-24 at 02:52 +0200, Rupert Weber wrote: The patch is here. Now, and it works. The conversions add 17k of code. Once GEGL takes over, they'll simply removed again. No one gets hurt. Well, the problem with your patch is that it adds new public API to libgimpcolor. And that will hurt us pretty badly. We will have to maintain that API for a couple of releases and I would really like to avoid that. Plug-ins will start to use it and that code will pretty soon have to be converted to use babl then. Instead of adding more pixel conversion routines to libgimpcolor, all of libgimpcolor should be deprecated as soon as possible and all users should be converted to use babl instead. Otherwise we will stick with our legacy APIs forever. Unfortunately I can't access bugzilla.gnome.org at the moment (seems to be a temporary server problem), so I can't review your patch in detail. I really don't want to stand in the way of this patch, but I strongly suggest that we go the road of deprecating libgimpcolor asap instead of extending it further. And no, I didn't ask you to work on the GEGL modes, I just ask you (or actually anyone) to use and improve babl when it comes to pixel format conversions. Sven ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/22/2010 02:45 PM, Sven Neumann wrote: New code in GIMP should use babl for pixel format conversion. There's no need to introduce new API for that as we have babl which is available to the core and plug-ins and provides a much superior API. The short answer is: No. I won't do that. For the long answer see further down below. (Sorry if this post becomes a bit longish) First about the current state of affairs: I posted the last update to the patch to http://bugzilla.gnome.org/attachment.cgi?bugid=325564 From my point of view, it is now done. Some performance numbers, comparing redraw times of legacy modes to the new LCH modes and to current GEGL LCH modes: (Tested with a very large picture to get measurable numbers; still these are ca. numbers, obtained with a stop watch) Mode | Legacy | New LCH | GEGL/babl -++-+-- Hue | 3.6 | 6.4 | 396 Saturation | 4.6 | 6.2 | 405 Color| 4.7 | 4.1 | 431 Value/Lightn.| 3.5 | 4.1 | 416 So I'm in the ball park of the legacy modes, Color is even a little faster. Compared to current GEGL/babl the new modes are about 60 to 100 times faster. (Yes, no typo) As to accuracy, these are the round-trip pixel errors for Lab and LCH conversions: Error after round-trip [in 8bit RGB space]: L*a*b* L*C*H* 0: 16561783 ( 98.716%) 0: 16527659 ( 98.513%) 1: 214941 ( 1.281%) 1: 248244 ( 1.480%) 2:492 ( 0.003%) 2: 1313 ( 0.008%) 3: 0 ( 0.000%) 3: 0 ( 0.000%) The worst we get are off-by-two errors. You won't notice without diff'ing two layers. If you don't just stack no-op layers on top of each other, out-of-gamut errors will be *far* greater than these. So, as I already said, I consider the patch done now. Things I will still be glad to change: - Location/name of new file, name of exported functions, etc. - Any bug fixes, of course. - The open issues I had mentioned earlier (file formats, GIMP_COMPOSITE_BLEND et al.) Things I won't change: - Optimization. I currently see no further optimization potential without uglifying the code. - As to putting the conversion outside the loop: Yes it can be done, but even if it is done, it doesn't belong in this patch. The current implementation in gimp_composite_generic.c is symmetric to the existing layer modes. So any such un-looping would be a general change to that file, not specific to the new layer modes. - Inlining: Brutally inlining everything can be done and gives some 12%-15% performance increase. -- But I don't want to get my hands dirty with that.. :o) And then there is babl. I feel very bad about that request. Because I expect it to be the first step in a relatively short sequence of if-we-do-that-why-don't-we's that will end with these modes not getting in but rather be added to the GEGL agenda. As that is effectively what you are asking me to do: work on the GEGL modes instead (or duplicate them, which would be even sillier). But that is not what I signed up for. The idea was to get something done and usable now. Not something that will be great in some uncertain future. When this is done I will be glad to take a look at babl and see if the conversions can somehow be integrated. But I don't expect that to be a trivial task. The patch is here. Now, and it works. The conversions add 17k of code. Once GEGL takes over, they'll simply removed again. No one gets hurt. Regards Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On Mon, 2010-08-09 at 15:42 +0200, Rupert Weber wrote: (2) Separate out the actual conversion routines in gimplibcolor as a standalone patch. Maybe change the Decompose plug-in to use those, so we at least have something to make use of the new conversions. (as far as I can tell, the current Decompose Lab functions are broken, anyway). (3) Once gimplibcolor updates are all settled, go about adding new layer modes which make use of them. (a) decide, which modes. Be conservative, don't clutter the menu too much with modes that nobody needs. Once they are out, we can't take them back. (b) decide how to name them. (c) decide how to deal with storing them (XCF version++) New code in GIMP should use babl for pixel format conversion. There's no need to introduce new API for that as we have babl which is available to the core and plug-ins and provides a much superior API. If your optimized routines turn out to outperform the CIE Lab - sRGB conversions that are in babl right now, then your code should be added to babl. Sven ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/09/2010 10:32 PM, Rupert Weber wrote: On 08/09/2010 07:42 PM, Martin Nordholts wrote: As long as you are available to respond to feedback about the patch, it will be included into 2.8, don't worry. It's just that it might take a while before anyone gets around to review and test it properly. Oh, cool. Just, before anyone actually dives in that code, please ask me for the most recent version, as I don't want to spam the bugzilla page with daily updates. I suggest you update the patch on, say, a weekly basis. No open source project minds being spammed with patches :) (a) decide, which modes. Be conservative, don't clutter the menu CIE LCH based Hue, Saturation, Color and Lightness, right? Well, yes. But what about e.g. Dodge, Burn, or Soft Light? D/B are easy to do and make sense. SL is not so easy, don't know if it even makes sense. But I'm also ok with sticking to those 4 obvious ones. And D/B ;=) I don't get, we already have working Dodge, Burn and Soft Light, don't we? / Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
From Rupert: (as far as I can tell, the current Decompose Lab functions are broken, anyway). What makes you say that? And how severe is the problem? Right this minute I'm writing a script with LAB Decompose... Charlie ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/09/2010 04:16 PM, Charlie De wrote: From Rupert: (as far as I can tell, the current Decompose Lab functions are broken, anyway). What makes you say that? And how severe is the problem? Right this minute I'm writing a script with LAB Decompose... Well, 'different' might have been nicer than 'broken' - The conversion doesn't account for gamma: http://www.brucelindbloom.com/index.html?Eqn_RGB_to_XYZ.html As a result, the L* channel is very light (check the histogram). I have no clue how that influences a* and b*. Also, it uses the conversion matrix for PAL/SECAM RGB. (Maybe PAL/SECAM has gamma 1.0? Then the missing gamma correction would be correct.) http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html Anyway, unless your image is in PAL/SECAM RGB, the conversion in Decompose is incorrect. ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
Actually, I see it, now that the script is done and the tests can be performed without bruising my knuckles. It isn't as bad as the Color mode, but it's there. Decomposing and recomposing an unaltered layer copy of an image changes the colours a little. Having two layers, top blurred, and bringing into it the L component of the lower layer, accentuates noise somewhat, not as badly as the Color mode. Iterating this procedure a number of times on the blurred layer, however, soon breaks down the image, with severe colour artifacts. Good news: the YCbCr_ITU_R709 decomposition works much better. Iteration tests brought no breakdown. The test on an unaltered layer copy of original also doesn't alter colours. A commenter in the original bug report about the Color mode did say the R709 worked well in his tests. Charlie - Original Message From: Rupert Weber g...@leguanease.org To: gimp-developer@lists.XCF.Berkeley.EDU Sent: Mon, August 9, 2010 4:45:00 PM Subject: Re: [Gimp-developer] Getting new layer modes fit for inclusion On 08/09/2010 04:16 PM, Charlie De wrote: From Rupert: (as far as I can tell, the current Decompose Lab functions are broken, anyway). What makes you say that? And how severe is the problem? Right this minute I'm writing a script with LAB Decompose... Well, 'different' might have been nicer than 'broken' - The conversion doesn't account for gamma: http://www.brucelindbloom.com/index.html?Eqn_RGB_to_XYZ.html As a result, the L* channel is very light (check the histogram). I have no clue how that influences a* and b*. Also, it uses the conversion matrix for PAL/SECAM RGB. (Maybe PAL/SECAM has gamma 1.0? Then the missing gamma correction would be correct.) http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html Anyway, unless your image is in PAL/SECAM RGB, the conversion in Decompose is incorrect. ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
Given that decisions made about layer modes will have consequences for years to come, it strikes me that there is an urgent need for a formal specification of what is to be done. A natural place to write one would be the wiki, but since that isn't functioning, maybe it would be possible to create a collaborative Google Docs document and use it to work out a spec? -- Bill ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/09/2010 03:42 PM, Rupert Weber wrote: Hi, while the last patch I posted* to http://bugzilla.gnome.org/show_bug.cgi?id=325564 is of course outdated already, I think it's time to think about how to ever get this included. Hi, As long as you are available to respond to feedback about the patch, it will be included into 2.8, don't worry. It's just that it might take a while before anyone gets around to review and test it properly. My suggestion: (1) Completely drop the xcf enum conversions (xcf-util.*) I introduced. They are not needed anymore and, quite frankly, pointless right now. This cuts down the patch a bit and means no new files added. Sounds good. (2) Separate out the actual conversion routines in gimplibcolor as a standalone patch. Maybe change the Decompose plug-in to use those, so we at least have something to make use of the new conversions. (as far as I can tell, the current Decompose Lab functions are broken, anyway). Having conversion routines in a separate patch is reasonable. If Decompose is broken, a separate patch to fix it would also be good. (3) Once gimplibcolor updates are all settled, go about adding new layer modes which make use of them. (a) decide, which modes. Be conservative, don't clutter the menu too much with modes that nobody needs. Once they are out, we can't take them back. CIE LCH based Hue, Saturation, Color and Lightness, right? (b) decide how to name them. I think the displayed names should be like above, and the API names should have a LCH prefix or suffix. I think the old ones should have display names with (compatibility) appended, and only be shown when an XCF that uses these modes have been loaded. The API names should remain as is. (c) decide how to deal with storing them (XCF version++) Didn't we already say that the XCF version should be bumped? Regards, Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
From Martin: I think the displayed names should be like above, and the API names should have a LCH prefix or suffix. I think the old ones should have display names with (compatibility) appended, and only be shown when an XCF that uses these modes have been loaded. The API names should remain as is. (compatibiity) makes sense but might be a bit too long. Wouldn't (Old) or (HSB) be better? Charlie ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/09/2010 09:08 PM, Charlie De wrote: From Martin: I think the displayed names should be like above, and the API names should have a LCH prefix or suffix. I think the old ones should have display names with (compatibility) appended, and only be shown when an XCF that uses these modes have been loaded. The API names should remain as is. (compatibiity) makes sense but might be a bit too long. Wouldn't (Old) or (HSB) be better? I think the names needs to suggest that new files should avoid using those layer modes. Other alternatives: (compat.) (legacy) (obsolete) (broken) (old) / Martin -- My GIMP Blog: http://www.chromecode.com/ Automatic tab style and removed tab title bar ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
From Martin: I think the names needs to suggest that new files should avoid using those layer modes. Other alternatives: (compat.) (legacy) (obsolete) (broken) (old) Good point! Then (obsolete) is best. The abbreviation of compat. to be avoided; legacy is too figurative, has slightly wrong connotations; broken, though technically correct, is too negative to be in the interface; old doesn't suggest enough that the old modes are obsolete. Since these names will only appear when old files are loaded, the more technical nature of obsolete is better. Can we all agree on (Obsolete)? Charlie ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/09/2010 07:42 PM, Martin Nordholts wrote: As long as you are available to respond to feedback about the patch, it will be included into 2.8, don't worry. It's just that it might take a while before anyone gets around to review and test it properly. Oh, cool. Just, before anyone actually dives in that code, please ask me for the most recent version, as I don't want to spam the bugzilla page with daily updates. (a) decide, which modes. Be conservative, don't clutter the menu CIE LCH based Hue, Saturation, Color and Lightness, right? Well, yes. But what about e.g. Dodge, Burn, or Soft Light? D/B are easy to do and make sense. SL is not so easy, don't know if it even makes sense. But I'm also ok with sticking to those 4 obvious ones. And D/B ;=) (c) decide how to deal with storing them (XCF version++) Didn't we already say that the XCF version should be bumped? yes, I just wasn't sure how final that was. Good. Rupert ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
