I suggest to read the book "Beginning Gimp" written by Akkana Peck. Data structures aren't cleared automatically. You have to clear them yourself. Otherwise you have random garbage in your picture. This can happen but it mustn't happen always.

On Tuesday 06 March 2007 18:16, Luis A. Florit wrote:

Pals,

I made my first plugin in C for gimp, based on the one here:

http://developer.gimp.org/writing-a-plug-in/3/index.html

Unfortunately, this plugin has several bugs, that translated to my own, and I am not being able to detect them (is there any other source like the avove to get some base plugin to work on?) Sorry for the not very objetive question:

The preview window updates when I change the parameters, but only in the top-left corner, where the preview appears. In the rest, it updates with some previous data, or some garbage. When I run the plugin for the second time, it is ok. That is, there is some buffer that is not being freed... Should I clean the region after a gimp_pixel_rgn_init call? Sometimes this also occurs on the main window.

Thanks,

Luis

Hi,

On Tue, 2007-03-06 at 14:16 -0300, Luis A. Florit wrote:

http://developer.gimp.org/writing-a-plug-in/3/index.html

Unfortunately, this plugin has several bugs

If you can point out bugs in the tutorial, we would love to hear about them so that they can be corrected. What exactly do you think is incorrect?

Sven

Hi,

On Tue, 2007-03-06 at 23:04 -0300, Luis A. Florit wrote:

These are the bugs I can remember now. Probably some of these are not bugs... apologies then.

1) The bottom bar of the main window becomes blue when the dialog appears.

Looks like progress handling needs to be improved in the plug-in. It shouldn't call gimp_progress_set_value() when drawing a preview.

2) The function process_row does not use the last parameter i.

How is that a problem?

3) Ok, this is the serious one: The whole blurred region is shifted 1 pixel down. To check this, create a new white image with a single black pixel in the middle, and blur it with radius=1. Go back and forth and you will see the 3x3 gray square shifting down.

Again, how is that a problem? This example code is meant to illustrate some concepts of GIMP plug-in development, in particular how to add a preview to your plug-in window. It is not meant to perform any useful image manipulation.

It would be nice though if someone could have a look at the tutorial and update the example code. The user interface looks somewhat outdated as the code originates back to gimp 2.0. We could probably improve the look and feel of third-party plug-ins if we made sure that the example code on developer.gimp.org serves as a good example.

Sven

On Wed, 07 Mar 2007 08:53:34 +0100, Sven Neumann wrote:

3) Ok, this is the serious one: The whole blurred region is shifted 1 pixel down. To check this, create a new white image with a single black pixel in the middle, and blur it with radius=1. Go back and forth and you will see the 3x3 gray square shifting down.

Again, how is that a problem? This example code is meant to illustrate some concepts of GIMP plug-in development, in particular how to add a preview to your plug-in window. It is not meant to perform any useful image manipulation.

How is it a problem that there are these small and varying offsets all the way through gimp?

I suspect this is not so much a bug in the example code, which as you say is just an example, but in gimp itself.

I have already submitted bugs comments indicatiing significant drift in scaling with NONE interpolation, there is residual drift in lanczos that I dont believe comes from the scaling code , there are fiddle factors to correct offsets in a number of areas.

Rather than dismissing this report, it may serve as a useful means of triangulating to find the root cause of some of these drifts.

If scaling, rotation and now plugins show drift it should help eliminate the superficial code and enable tracking this back to the problems in the core.

This is only gut feeling on my part but it seems that the evidence is mount to confirm what I suggested six mounths ago when I first started looking at lanczos.

Thanks to Luis for taking the time to notify the apparent error.

gg

Hi,

On Wed, 2007-03-07 at 09:55 +0100, gg@catking.net wrote:

How is it a problem that there are these small and varying offsets all the way through gimp?

I suspect this is not so much a bug in the example code, which as you say is just an example, but in gimp itself.

If you can point out how this can be a bug in gimp itself, I would love to hear about it. We have several hundreds of plug-ins that work correctly so it is rather unlikely that there's a bug in the core or libgimp that would be responsible for this offset.

The fact that there's an offset here and a drift in the transform code in the core is pure coincidence. I wonder what makes you think that there's a relation. Off-by-one and rounding errors are not uncommon and they happen all over the place.

Sven

On Wed, 07 Mar 2007 08:53:34 +0100, Sven Neumann wrote:

3) Ok, this is the serious one: The whole blurred region is shifted 1 pixel down. To check this, create a new white image with a single black pixel in the middle, and blur it with radius=1. Go back and forth and you will see the 3x3 gray square shifting down.

Again, how is that a problem? This example code is meant to illustrate some concepts of GIMP plug-in development, in particular how to add a preview to your plug-in window. It is not meant to perform any useful image manipulation.

PS.

Blur off the menu does not seem to reproduce this. So either its a buggy example as Svan suggests or a drift in the plugin interface.

A quick check on rotation shows 1px drift in x and y. Create a 20x20 and a 21x21 canvas put a pencil tool (11 circle) spot and rotate 90 degrees about the centre of the dot. Preview gets it right but it moves when committed. Also odd effects around borders

The offsets seem to be independant of interp method.

A non-exhaustive check on on scaling seems not to show this offset but like I said I found I had to add fiddle factors to achieve this. I added some comments to code to attempt to explain what was happening but it was a bit hand wavy. My belief is that I am applying a correction to an offset coming from outside the scaling code.

gg.

On Wed, 07 Mar 2007 10:02:38 +0100, Sven Neumann wrote:

Hi,

On Wed, 2007-03-07 at 09:55 +0100, gg@catking.net wrote:

How is it a problem that there are these small and varying offsets all the
way through gimp?

I suspect this is not so much a bug in the example code, which as you say
is just an example, but in gimp itself.

If you can point out how this can be a bug in gimp itself, I would love to hear about it. We have several hundreds of plug-ins that work correctly so it is rather unlikely that there's a bug in the core or libgimp that would be responsible for this offset.

OK, then you're probably right , it's a buggy eg. and it would be best to give examples the work correcly to get ppl off to a good start.

The fact that there's an offset here and a drift in the transform code in the core is pure coincidence. I wonder what makes you think that there's a relation. Off-by-one and rounding errors are not uncommon and they happen all over the place.

Sven

Obviously rounding errors are unavoidable when doing everything in integer arithmatic although I think quite a bit could be done to reduce the accumulation of errors.

However I was seeing a more significant offset on NONE that I detailed when I split the "lancos offset" bug (principally for this reason).

gg

Hi,

On Wed, 2007-03-07 at 11:03 +0100, gg@catking.net wrote:

OK, then you're probably right , it's a buggy eg. and it would be best to give examples the work correcly to get ppl off to a good start.

I already fixed the wrong use of gimp_progress_set_value() this morning. If someone wants to look into fixing or improving the examples and/or the tutorial, please do so. The code is found in the gimp-web-devel SVN module.

Sven

Hi,

On Wed, 2007-03-07 at 10:52 +0100, gg@catking.net wrote:

A quick check on rotation shows 1px drift in x and y. Create a 20x20 and a 21x21 canvas put a pencil tool (11 circle) spot and rotate 90 degrees about the centre of the dot. Preview gets it right but it moves when committed. Also odd effects around borders

The offsets seem to be independant of interp method.

If you rotate by exactly 90 degrees, this is always done with INTERPOLATION_NONE, no matter what you select in the tool options.

Sven

Hi,

On Wed, 2007-03-07 at 22:18 -0300, Luis A. Florit wrote:

My plugin is working fine, after I found my bug and the one in the blur example. It's just annoying.

Then please explain the bug in the example code and send us a patch (preferably against SVN) that we can apply so that others won't run into the problem again.

http://svn.gnome.org/viewcvs/gimp-web-devel/trunk/writing-a-plug-in/

You should realize that this is all just volunteers effort. Someone wrote this tutorial years ago and another volunteer added it to the gimp-developer web site. It will take another volunteer to improve it and to fix the example code. And yet even more volunteers will have to show up in the future to keep the tutorial uptodate.

Sven

Hi,

If you rotate by exactly 90 degrees, this is always done with INTERPOLATION_NONE, no matter what you select in the tool options.

Perhaps this is the culprit? An offset seems unavoidable if the transformation is performed without interpolation. So perhaps all we need to do is to remove this optimization (which is supposed to speed up rotations by multiple of 90 degrees)?

Peter, can you try to remove that code in app/core/gimp-transform-region.c (line 208 and 209) and check if that fixes your test case?

Sven

On 3/8/07, Sven Neumann wrote:

Hi,

If you rotate by exactly 90 degrees, this is always done with INTERPOLATION_NONE, no matter what you select in the tool options.

Perhaps this is the culprit? An offset seems unavoidable if the transformation is performed without interpolation. So perhaps all we need to do is to remove this optimization (which is supposed to speed up rotations by multiple of 90 degrees)?

BTW, do you think the rotation centre should be snapped to 0.5 pixel increments when interpolation is NONE? It doesn't make sense to have any more precision at that point (and can introduce glitches -- for instance, try floating a rectangular region, then dragging the rotation centre to the top left as precisely as you can, setting it to rotate 90 degrees, and performing the rotation... -- compared to inputting the coordinates of the top left yourself and then performing the 90 degree rotation. In the first case, even fractional imprecision means the result is not even in the right place.)

Anyway, in the case of a 90 degree rotation, it seems unlikely that the user would want it misaligned with the pixels -- in which case no interpolation is needed and the result should be exactly right.

Simple test case that uniformly fails, currently:

* Select a rectangular region of the picture. * Float it
* Rotate it. Set the rotation centre to the exact top left (by first positioning the centre near it, then editing the coordinates in the rotation dialog to make them exact). Set the angle to 90 degrees and the interpolation to NONE. Supersampling option appears to have no effect in this case.
* The result may be offset by 1 pixel in X and/or Y axis; It is also missing one line of pixels (which line is omitted varies.)

On Thu, 08 Mar 2007 11:54:25 +0100, David Gowers wrote:

On 3/8/07, Sven Neumann wrote:

Hi,

If you rotate by exactly 90 degrees, this is always done with INTERPOLATION_NONE, no matter what you select in the tool options.

Perhaps this is the culprit? An offset seems unavoidable if the transformation is performed without interpolation. So perhaps all we need to do is to remove this optimization (which is supposed to speed up rotations by multiple of 90 degrees)?

BTW, do you think the rotation centre should be snapped to 0.5 pixel increments when interpolation is NONE? It doesn't make sense to have any more precision at that point (and can introduce glitches -- for instance, try floating a rectangular region, then dragging the rotation centre to the
top left as precisely as you can, setting it to rotate 90 degrees, and performing the rotation... -- compared to inputting the coordinates of the
top left yourself and then performing the 90 degree rotation. In the first
case, even fractional imprecision means the result is not even in the right
place.)

Anyway, in the case of a 90 degree rotation, it seems unlikely that the user
would want it misaligned with the pixels -- in which case no interpolation
is needed and the result should be exactly right.

Simple test case that uniformly fails, currently:

* Select a rectangular region of the picture. * Float it
* Rotate it. Set the rotation centre to the exact top left (by first positioning the centre near it, then editing the coordinates in the rotation
dialog to make them exact). Set the angle to 90 degrees and the interpolation to NONE. Supersampling option appears to have no effect in this case.
* The result may be offset by 1 pixel in X and/or Y axis; It is also missing
one line of pixels (which line is omitted varies.)

I think you can achieve the same results without floating. Just try a rotate.

gg

Quoting "Luis A. Florit" :

http://w3.impa.br/~luis/fotos/lixo/grid_blurred.jpg

You can see three problems: 1) The old 1-pixel-down shift.
2) The two dark bands (20 lines each) at top and bottom of the image. 3) Even much intriguing is the grey band right below the top one, also of 20 lines! And it is homogeneous (no likes inside)...

My 'solution' (change 'x1, y1,' in line 241 by 'x1, y1+1,') took care of the shift, but not of the banding.

The dark bands can be attributed to the fact that the averaged region near the borders is "filled out" by effectively duplicating the row or column at the edges. The algorithm may not be optimal, but it works as one should expect.

The average is currently produced by a formula similar to:

sum (v-2, v-1, v, v+1, v+2) / 5

Near the borders, when there is no value "v+2" available, for example, you end up with:

sum (v-2, v-1, v, v+1, v+1) / 5

And the next pixel will be:

sum (v-2, v-1, v, v, v) / 5

If you wish to improve the algorithm, I would suggest that you need to discard terms and adjust the denominator accordingly -- the last example shown should produce "sum (v-2, v-1, v) / 3".

At least, that is my recommendation. I am not a mathematician.

On Fri, 09 Mar 2007 05:31:26 +0100, wrote:

Quoting "Luis A. Florit" :

http://w3.impa.br/~luis/fotos/lixo/grid_blurred.jpg

You can see three problems: 1) The old 1-pixel-down shift.
2) The two dark bands (20 lines each) at top and bottom of the image. 3) Even much intriguing is the grey band right below the top one, also of 20 lines! And it is homogeneous (no likes inside)...

My 'solution' (change 'x1, y1,' in line 241 by 'x1, y1+1,') took care of the shift, but not of the banding.

The dark bands can be attributed to the fact that the averaged region near the borders is "filled out" by effectively duplicating the row or column at the edges. The algorithm may not be optimal, but it works as one should expect.

The average is currently produced by a formula similar to:

sum (v-2, v-1, v, v+1, v+2) / 5

Near the borders, when there is no value "v+2" available, for example, you end up with:

sum (v-2, v-1, v, v+1, v+1) / 5

And the next pixel will be:

sum (v-2, v-1, v, v, v) / 5

If you wish to improve the algorithm, I would suggest that you need to discard terms and adjust the denominator accordingly -- the last example shown should produce "sum (v-2, v-1, v) / 3".

At least, that is my recommendation. I am not a mathematician.

On Thu, 08 Mar 2007 23:13:08 +0100, Luis A. Florit wrote:

My 'solution' (change 'x1, y1,' in line 241 by 'x1, y1+1,') took care of the shift, but not of the banding. The homogeneous grey band just moved to the bottom... (you cannot imagine the mess this does in my code!)

This is what's known in the trade as a frig factor!

You are arbitarily making an adjustment to correct one subjective error. You will likely find that on a different image it bahaves differently unless you understand why this adjustment is necessary and that +1 is a generally valid fix.

I cant comment on whether this is a bug in the tile code of the example or elsewhere. I suggest you look at some of the built in plugins.

I think most of the effects on the filters menu are implemented as plugins that are part of the gimp distribution. There are some differences in integrating external plugins but it's pretty much the same thing in the plugin code so you may be able get some clues on what you are trying to do from the existing plugins.

Be warned there are bugs in there two ;)

I did some revectoring and tidy up work on the convolution plugin last year and got told off for not testing it and introducing bugs. It turned out the bugs have been there since 1997. My objective was to make the code more readable and maintainable and to generalise the matrix routines to handle larger dimensions. I did not attempt to fix these anomolies.

My initial idea was to write a new plugin but I gave up since the documentation was totally inadequate, out of date and sometimes inaccurate. I decided it would take more time than I had available.

I hope you have more luck.

gg

Hi,

On Fri, 2007-03-09 at 08:08 +0100, gg@catking.net wrote:

I did some revectoring and tidy up work on the convolution plugin last year and got told off for not testing it and introducing bugs. It turned out the bugs have been there since 1997. My objective was to make the code more readable and maintainable and to generalise the matrix routines to handle larger dimensions. I did not attempt to fix these anomolies.

Huh? Your changes got committed, if I remember correctly.

GG, can you perhaps consider to take your bashing off-list? We all know very well that the GIMP code is far from perfect. A lot of plug-ins and also some code in the core is old and the people who wrote it weren't experts in computer graphics. The algorithms being used are sometimes badly chosen and sometimes even incorrectly implemented. There is a lot to do in this respect and we hope that the switch to GEGL will force us to review all this code. The concepts of GEGL and babl should also allow us to get to cleaner and more readable code. Please do not discourage us and potential contributors by continously pointing out that some of the code is crap. We all know that.

Sven

Hi,

On Thu, 2007-03-08 at 19:13 -0300, Luis A. Florit wrote:

Unfortunately, the code has more that one bug, or the correction I made didn't solve the issue completely.

I think we should then replace the example by something simpler. The point of this tutorial is not to show how to implement a blur. It is supposed to show some basic concepts of plug-in development. The code that is used as an example should be simple and bug-free. The current blur code doesn't seem to fulfill these requirements. So perhaps it should be replaced by a more local operation.

Another option would be to add a comment to the tutorial that points out that, for the sake of simplicity, the blur code is simpler than it would have to be and that it introduces a shift for that reason.

Now: Could anyone please show me a SIMPLE code where the tile handling is properly implemented, without bugs? I mean, the same blurring plugin would be just perfect, but without bugs... From this buggy example, I just don't understand how they work, so I am not able to find the bug.

The GIMP source code includes more than hundred plug-ins. I am sure you will find one or two that can serve as examples.

Sven

Hi,

On Thu, 2007-03-08 at 09:45 +0100, peter@piments.com wrote:

I have limitted time right now but I'll try to have a look in the next few days, though I dont follow your logic.

If you have a look at it now, be warned that there's a bug in the pixel-surround routines which I introduced two days ago. I am aware of the bug and I will either fix it over the next days or revert some optimizations. So please don't wonder about artefacts at the bottom of the source area, these are due to this bug.

The reason this is optimised like this is because there is NO NEED for any fancy techniques since it's a one-to-one mapping.

Is it really always a one-to-one mapping? I might be wrong but this seems to be dependant on the choice of the center of rotation and I am not sure if correctly check for this.

I did start to look into this a while back but ran out of time. One source of error I noted was that the code splits all transforms into an origin and offset coordinates for just about all operations. Many of these are done with the origin at the image centre. This means two truncations , two rounding errors.

Sounds like we should investigate this further.

Sven

On Fri, 09 Mar 2007 08:36:46 +0100, Sven Neumann wrote:

Hi,

On Fri, 2007-03-09 at 08:08 +0100, gg@catking.net wrote:

I did some revectoring and tidy up work on the convolution plugin last year and got told off for not testing it and introducing bugs. It turned out the bugs have been there since 1997. My objective was to make the code
more readable and maintainable and to generalise the matrix routines to handle larger dimensions. I did not attempt to fix these anomolies.

Huh? Your changes got committed, if I remember correctly.

Yes the readability and cleanup that I did got committed, it is now more readable and maintainable. I did not attempt to fix a couple of oddities that I noticed (and that Bill initially thought I had introduced).

GG, can you perhaps consider to take your bashing off-list? We all know very well that the GIMP code is far from perfect. A lot of plug-ins and also some code in the core is old and the people who wrote it weren't experts in computer graphics. The algorithms being used are sometimes badly chosen and sometimes even incorrectly implemented. There is a lot to do in this respect and we hope that the switch to GEGL will force us to review all this code. The concepts of GEGL and babl should also allow us to get to cleaner and more readable code. Please do not discourage us and potential contributors by continously pointing out that some of the code is crap. We all know that.

Sven

Well it seems it was you who got Luis back up as soon as he came in but thanks for having a more positive to things generally in your recent posts.

I was aiming to forewarn him about the bugs and the state of the code I was refering him to, to prevent him wasting further time and getting more frustrated.

In the past I have seen too many attempts to improve things dismissed out of hand as either not being a problem or being a waste of time.

If we take the more realistic approach you outline above things will certainly move forwards and ppl will know where they are.

gg

Concerning the plug-in example, if you want to see the proper way to do it, you can look at the real "blur" plug-in, in plug-ins/common/blur.c. However, as Sven suggests, that plug-in only does a 3x3 blur, and a proper implementation for NxN, as in the example, would be pretty hard to read -- too hard to make a good example.

-- Bill

______________ ______________ ______________ ______________ Sent via the CNPRC Email system at primate.ucdavis.edu

Hi,

On Fri, 2007-03-09 at 09:07 +0100, Sven Neumann wrote:

If you have a look at it now, be warned that there's a bug in the pixel-surround routines which I introduced two days ago. I am aware of the bug and I will either fix it over the next days or revert some optimizations. So please don't wonder about artefacts at the bottom of the source area, these are due to this bug.

Just FYI: That problem should be fixed now.

Sven

On Sat, 10 Mar 2007 21:24:13 +0100, Sven Neumann wrote:

Hi,

On Fri, 2007-03-09 at 09:07 +0100, Sven Neumann wrote:

If you have a look at it now, be warned that there's a bug in the pixel-surround routines which I introduced two days ago. I am aware of the bug and I will either fix it over the next days or revert some optimizations. So please don't wonder about artefacts at the bottom of the source area, these are due to this bug.

Just FYI: That problem should be fixed now.

Sven

thx for the update.
gg

On Fri, 09 Mar 2007 09:07:48 +0100, Sven Neumann wrote:

The reason this is optimised like this is because there is NO NEED for any fancy techniques since it's a one-to-one mapping.

Is it really always a one-to-one mapping? I might be wrong but this seems to be dependant on the choice of the center of rotation and I am not sure if correctly check for this.

yes , you may be right. I was resting on a preconception of the centre of rot. being integer or half-integer as it is by default. But I think the recent fp input allows 0.01 increments. Maybe the desirablility of that needs to be checked.

0.5 did solve some drift but finer offsets may produce other errors or need some follow through.

there is also the question of differing x and y resolution.

you are probably right that the optimisation is inappropriately tested but it was not the cause of the test case I ran that was symetrical and integer centred. The drift remains when the lines you indicated are removed.

I did start to look into this a while back but ran out of time. One source of error I noted was that the code splits all transforms into an origin and offset coordinates for just about all operations. Many of these are done with the origin at the image centre. This means two truncations , two rounding errors.

Sounds like we should investigate this further.

Yes, I dont know if I'll have time to have a serious look at this but I think the offset needs to remain fp as long as possible , not get truncated, and all gint() casts need to be checked.

These drifts may sound trivial but can lead to noticable noise after very few operations. Take a small, simple geometric shape , rotate L 15, rotate R 15, the ammount of non-symetric noise is not good.

Thanks for taking the comments on board, I think it's important that the gimp does not degrade image quality more than is mathematically unavoidable.

gg.