search

Serabe / RMagick4J

RMagick for JRuby.
Other
29 stars 13 forks source link

Geometry refactoring, CMYK JPEGs and other stuff #7

Closed palmskog closed 8 years ago

palmskog commented 14 years ago

Here are the commits for the fixes I described.

Being new to commiting with git, I accidentally used the auto-generated email address "palmskog@kokytos.(none)" for the first two commits. If you can change this to "palmskog@kth.se" when you pull, that would be much appreciated.

Most of the stuff added has specs, but not the CMYK part, since that requires finding suitable public domain images or similar to add to the repository. I have however tested it quite extensively by hand on a set of images I had lying around. Also, one thing I did not mention earlier is the added support for "jpg:/path/to/img" syntax in the write method of MagickImage.

The refactoring of the Geometry hierarchy is a straightforward application of the Visitor pattern to handle the cases when either of the height/width components of a geometry string are missing. This avoids virtually all of the if-then-else mess that ImageMagick itself uses. Most of the Height/Width classes are symmetrical, but some are not, notably the area ones, which motivates the split.

Serabe commented 13 years ago

I've heard that git lets you rewrite history, though I've never needed to. Let me one or two days (sorry for the current delay) and, if I find nothing for changing the mail address, I'll go with the default, ok?

donv commented 11 years ago

Hi @palmskog !

I am reviving this project. Would you still like this merged?

palmskog commented 10 years ago

Hi @donv

Merge away, I think the features I added should be useful, especially for resizing/cropping images. (The "wrong email address" thing I mentioned in the original request doesn't matter much, so just go as is.)

donv commented 8 years ago

Hi @palmskog !

Only two years before responding! Sorry about that! 😄 I would still like to have this merged, but it needs a rebase which I do not have the time to do.

Please let me know if you still would like this merged and if you have the time to rebase against master.

palmskog commented 8 years ago

Hi @donv ,

This PR was really a "fire and forget" for me. Looking now in detail at the commit history for the Serabe:master branch, it seems the commits in this PR were already added by @Serabe ("Commits on Sep 13, 2010"), possibly by pulling them directly from my fork outside of the context of the PR. Hence, to the best of my knowledge, this PR can be closed.

donv commented 8 years ago

@palmskog Thanks for the quick reply. Closing.