Re: [Gimp-developer] N-Point Image Deformation queries


We somewhat assumed that you weren't here, because we couldn't easily
get a hold of you. Apparently, we didn't try hard enough. Thanks for
sticking around.

Anyway, I began reviewing your code. I made some changes to it to try
and make it a bit cleaner, so we could merge it into the main gegl
tree. You can see all of my changes here:

The main complaints we have about your code are:

  * The GEGL operation shouldn't contain any Cairo code in it. UI
stuff like that should only be in either your library or your GIMP
tool. Of course, you have it set up so that this should be easy to

  * Generally, when you write an API, you want to expose as little as
possible. Only the bare minimum number of functions should be included
in the public/installed header files, and typically no structures at
all should appear there. I've begun fixing this by converting some of
your functions to static functions where possible.

  * Some of the functions like add_point_to_suitable_cluster do
bizarre things like convert floats to strings just so they can be used
as hash table keys. A lot of this code wasn't actually used, so I
commented it out for now.

I would feel a lot better if the npd library didn't include its own
interpolation code. You mentioned outputting float coordinates to an
image and then using map-absolute, but I'd prefer if the gegl
operation found the color values directly with a GeglSampler object.
This gives you access to Gegl's built-in interpolation methods.

I'm not sure if you want me to continue working on npd (I can find
other stuff to do, if you'd prefer to make these changes yourself.)

Anyway, these are the main things that are stopping us from merging
the gegl npd branch right now. I haven't actually looked at the gimp
npd branch, but mitch sounded generally happy with the code there.

If you want to talk to me on irc, my nick is "drawoc".

Anyway, thanks for the hard work!

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]