Re: [Gimp-developer] Getting new layer modes fit for inclusion
On 08/24/2010 02:52 AM, Rupert Weber 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) Hi Rupert Thanks a lot for your hard work, it is much appreciated. Personally I am fine with not having your new conversion routines ported to babl yet, we can do that later. It is my firm belief however that it is too early to apply the patch. There are usability aspects that needs to be taken care of first, that has been mentioned before. 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) Disclaimer: I haven't reviewed your latest patch so there might be things we need to address there too. Best 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
[Gimp-developer] Getting new layer modes fit for inclusion
On Tue, Aug 24, 2010 at 10:22 AM, Rupert Weber 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) 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 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). GEGL modes are something else. I believe what Sven was suggesting is to implement your conversion code in a BABL extension, and use that to do color conversion in the layer modes; Not to use GEGL for that whole thing. That said... > 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 absolutely agree that this patch should be applied promptly. It implements much-wanted functionality in an effective an
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] Alignment-Tool && Guides
On Mon, 2010-08-23 at 09:56 +0200, oli...@first.in-berlin.de wrote: > On Sun, Aug 22, 2010 at 06:23:37PM -0700, Bill Skaggs wrote: > [...] > > > > My recollection is that layer border are already magnetic, too. > > New feature? Since when? > I have 2.6.7 here. Canvas borders are magnetic, but layer borders aren't (yet). Sven ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
[Gimp-developer] Measurement-Tool cleared when taking Guides
Hello, when I use the measurement tool, and then try to pick a guide, the measurement tool is blown away from the screen. I can't see that this is a feature. If I measure one distance, I know it. The next step might be: measure the same distance somewhere else, and place a guide at the new point. How to circumvent this behaviour of clearing the measurement tool? Ciao, Oliver ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Alignment-Tool && Guides
On 8/23/10, oliver wrote: >> My recollection is that layer border are already magnetic, too. > > New feature? Since when? > I have 2.6.7 here. > > Can it be disabled? Snapping options are in the lower half of the View menu. Alexandre Prokoudine http://libregraphicsworld.org ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Alignment-Tool && Guides
On Sun, Aug 22, 2010 at 06:23:37PM -0700, Bill Skaggs wrote: [...] > > My recollection is that layer border are already magnetic, too. New feature? Since when? I have 2.6.7 here. Can it be disabled? Ciao, Oliver ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
Re: [Gimp-developer] Alignment-Tool && Guides
On Sun, Aug 22, 2010 at 06:23:37PM -0700, Bill Skaggs wrote: > That's actually supposed to be supported already. You should be able to use > a guide > with the alignment tool exactly as though it is an infinitely long layer of > zero width. [...] As primary object I have the choice to select: - picture - selection - active layer - active channel - active patth There is no "active guide", as all guides are active at the same time, so I never heard of that. And there is no option "guides" at all. So I can't center an object around a guide. (Or did I miss some hidden behaviour of the tool?) But you are right for the other way around: a guide can be postioned relative to the other objects. Ciao, Oliver ___ Gimp-developer mailing list Gimp-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
