Page 1 of 1

New function (Cylinderize)

Posted: 2012-01-10T12:22:56-07:00
by RNHurt
Hey guys, my team and I are working up a new function to emulate Fred's Cylinderize function (http://www.fmwconcepts.com/imagemagick/ ... /index.php). Like the Polaroid function, I think a Cylinderize function would be well received by other IM users. We didn't want to use the script because we have a very high performance use case and shelling out to a script is not really an option.

We do have working code and I would like to know the best way to submit patches to be included in the mainline branch of ImageMagick. I couldn't find specific coding standards but we tried to follow the general coding standards of the rest of the IM code. My plan going forward is to:
  • base our changes on the latest dev branch of IM
  • add tests for new functionality
  • update the command line arguments
  • update the documentation
Are there any other things we should keep in mind? How do you accept patches? Email, SVN, etc.? Currently, our code is on GitHub (https://github.com/CafepressDev/ImageMagick) but be warned that it is against the v6.7.2-9 code base. And while the base code works (fx.c) we haven't had a chance to add tests, command line args, etc. :(

Re: New function (Cylinderize)

Posted: 2012-01-10T12:47:23-07:00
by magick
Are you willing to release the code under the ImageMagick license? Is the method completed or is there more work? Once you're ready, let us know here and we will get your patches from Github and commit it to the ImageMagick 6.7.4 branch and the trunk. Thanks.

Re: New function (Cylinderize)

Posted: 2012-01-10T12:57:01-07:00
by fmw42
And while the base code works (fx.c) we haven't had a chance to add tests, command line args, etc.
It would be nice to see my script productized within IM. However, I would suggest you look further into the arguments limitations within IM or discuss what you are planning with Magick and/or Anthony. My cylinderize script has more arguments than the basic (4?) IM allows unless you do something like Anthony has done with some of the newer distort functions.

Re: New function (Cylinderize)

Posted: 2012-01-10T13:04:42-07:00
by magick
You can encode up to 5 fields into a command line argument of either style: 1x2+3+4+5 or 1,2,3,4,5. Any parameters beyond 5 requires a define. The path forward would be to use the 5 most widely used parameters for the -cylinderize option and then use -define cylinderize:6=value thru -define cylinderize:n=value for less frequently used parameters.

Re: New function (Cylinderize)

Posted: 2012-01-10T17:11:02-07:00
by fmw42
magick wrote:You can encode up to 5 fields into a command line argument of either style: 1x2+3+4+5 or 1,2,3,4,5. Any parameters beyond 5 requires a define. The path forward would be to use the 5 most widely used parameters for the -cylinderize option and then use -define cylinderize:6=value thru -define cylinderize:n=value for less frequently used parameters.

Doesn't Anthony's -distort functions or -function allow more (or any number of) arguments or are they still limited to five?

For example, -function polynomial seems to imply any order and the corresponding number of arguments.

The downside is that these arguments are just a list of value in some specified order. So there is no parameter name or abbreviation to identify the parameters.

Thus the -define approach might be better, though a bit long to specify every parameter as a define.


Fred

Re: New function (Cylinderize)

Posted: 2012-01-11T21:37:27-07:00
by anthony
There are four back types of arguments.
See http://www.imagemagick.org/Usage/basics/#arguments

Most common types is either 'string' or 'geometry' that later being limited to 5 numbers plus various flags.

distort allows more arguments as they are processed specially as a floating point list (array)
sparse-color is similar but allo the use of colors instead of some floating point values in ceratin location.

It all comes down to the API interface that is implemented.

Re: New function (Cylinderize)

Posted: 2012-01-12T08:08:51-07:00
by RNHurt
magick wrote:Are you willing to release the code under the ImageMagick license? Is the method completed or is there more work? Once you're ready, let us know here and we will get your patches from Github and commit it to the ImageMagick 6.7.4 branch and the trunk. Thanks.
Yes, we are willing to release the code under the ImageMagick license. The code isn't complete yet and there are some weird bugs dealing with transparency and virtual_pixels, but it does generate cylindrical images. :)

The main developer on this project (Sara) is looking at the API comments above and is re-evaluating how our code is structured. We might come back with some more questions on how to proceed.

Re: New function (Cylinderize)

Posted: 2012-01-12T08:38:33-07:00
by sara_shafaei
Our function basic idea is from Fred's scrip for Cylinderize. It is currently taking 7 parameters. 6 parameters are in double type and the one is MagickBooleanType. What do you recommend for this?
I was also wondering what is your standard to check the parameter to be in the specific rang inside the function. Is it common for IM. I mean inside the function not command line. I am currently doing it myself and print the error message not using assert. Is this acceptable to IM.

I also have a question from Cylinderize script. When we set the virtualpixcelMethod to transparent. Result is not very smooth at the edge like when we have a background. Is this solvable?

Thank you!
Sara

Re: New function (Cylinderize)

Posted: 2012-01-12T11:30:19-07:00
by fmw42
sara_shafaei wrote:Our function basic idea is from Fred's scrip for Cylinderize. It is currently taking 7 parameters. 6 parameters are in double type and the one is MagickBooleanType. What do you recommend for this?
I was also wondering what is your standard to check the parameter to be in the specific rang inside the function. Is it common for IM. I mean inside the function not command line. I am currently doing it myself and print the error message not using assert. Is this acceptable to IM.

I also have a question from Cylinderize script. When we set the virtualpixcelMethod to transparent. Result is not very smooth at the edge like when we have a background. Is this solvable?

Thank you!
Sara

Feel free to contact me offline if you want with further clarification of the transparency issue and perhaps an example result from your work compared to my script result? Are you using the most current version of my cylinderize script? It is dated 10/5/2011 and has 14 arguments and allows an optional background image and better top and bottom curvature. You can contact me at fmw at alink dot net. Unfortunately, the IM developers will have to answer your coding questions as I only do scripting.

Fred

Re: New function (Cylinderize)

Posted: 2012-01-13T07:53:30-07:00
by sara_shafaei
Thank you so much Fred for your fast reply. I found out what was the problem. I was doing something wrong and it is working great now. I may contact you if I have a question later on.
Thanks again!
~Sara

Re: New function (Cylinderize)

Posted: 2012-01-17T15:44:35-07:00
by sara_shafaei
Hi guys,

I have a question for Anthony or other imagemagick guys here. Our Cylinderize function is almost done. We need to check input parameters to make sure they are in the certain range. right now I am doing something like this:
assert(radius>0);
assert((wrap>=10)&&(wrap<=100));

It is working, but I want to make sure that is acceptable. If it is not, could you please give me example in the imagemagick code. I couldn't find any.

We used Floating Point Lists for the command line input parameters, since we have 6 double parameters.

Thank you,
~Sara

Re: New function (Cylinderize)

Posted: 2012-01-19T13:37:27-07:00
by RNHurt
magick wrote:Are you willing to release the code under the ImageMagick license? Is the method completed or is there more work? Once you're ready, let us know here and we will get your patches from Github and commit it to the ImageMagick 6.7.4 branch and the trunk. Thanks.
Unfortunately, I didn't plan things very well on my side and I'm not sure how to pull a valid diff from Github. :( I didn't add any tags and we've started over quite a few times trying to get XCode to cooperate.

The good news is that I have built a patch against the 6.7.4-7 codebase. Is there somewhere that I can upload the patch file?

Re: New function (Cylinderize)

Posted: 2012-01-19T17:47:00-07:00
by magick
You can e-mail the patch file to patches @ imagemagick dot org.