Autorotate does not reset EXIF orientation tag

My 1st impressions after a short look at it:
- this function only exists if imagemagick advanced module is installed and enabled.
- that module defines this action as a separate effect.
- so we could say this is superfluous, people can add it themselves if they want to. But that probably is not a good idea as they probably don't know that different browsers will act differently.
- it introduces toolkit specific code without calling image_toolkit_invoke().
- so, I guess we would better call this code by calling image_effect_apply().
- is there no option in imagemagick to just change the rotation info?

- or we just add the -strip to this effect as well. What about other effects like e.g. rotate?

EDIT:
see http://www.imagemagick.org/Usage/photos/#orient for some more background info. I guess we should not use image_rotate() at all when working with imagemagick. Moreover the exif extension dependency can be removed as well in this case. So I propose to make imagecache_autorotate_image() toolkit dependant and introduce 2 new functions image_gd_autorotate() and image_imagemagick_autorotate().

Fixed and committed. The actual fix is like in the Imagemagick advanced module, but I also refactored the other code of this effect (regarding e.g. resulting dimensions and image_toolkit_invoke).

I tested a simple image (now added as test image) with both GD and imagemagick. Looks good.

@dman: could you (as native English speaking person) check the extended/rewritten on-form documentation for correctness and comprehensibility? Thanks in advance.

Good spotting.
I had actually anticipated that when this first went in, but being mostly concerned with GD, and not knowing any browser (at the time) that would produce this double-effect - didn't follow that up. :-B
Out of interest, how DID you spot this?

Actually, I like EXIF, and the EXIF-safe feature is one of the main benefits for using imagemagick at all (I've got a large module 'meta' that deals with exif tag-to-field mappings

Sooo.... I'm really not keen on bluntly stripping all metadata.
Can we just unset the orientation flag instead? I'd normally use exiftool on the commandline, but don't want to add that as a requirement.

.. loks like it may be possible with mogrify http://stackoverflow.com/questions/229446/how-do-i-add-exif-data-to-an-i...
Or

convert image -format "%[exif:orientation]" info:

According to http://www.wizards-toolkit.org/discourse-server/viewtopic.php?f=1&t=16784

...
or, well, it seems that if using imagemagick ... we may as well have just used the --auto-orient flag, huh?

Thanks for the clean patch, but maybe there are better solutions?
Also, you are relying on the image_imagemagick_strip function, which looks like it comes from imagemagick_advanced.module.
Given the triviality of the code, that dependency is not really worth it.

I'd be open to replacing the imagemagick_autorotate callback with --auto-orient fully though - if we can also test it.

wow - that was a quick double-post!

Timestamps don't show it, but I swear I opened this issue, looked at the imagemagick docs and replied within ten minutes. My post above was in direct response to the OP issue.

Hm. I'm confused now.
Looking at that commit I don't think we are doing the same thing at all here..

I'm really confused - looks like my tracker just showed me the OP, not any of the later fix.
Please ignore this - I had an early start today and my brain iz fizzy.

I had an early start today and my brain iz fizzy.

Hmmm, I'm not sure anymore that you are the right person to ask to check mine English texts :)

In fact, we'redoing basically the same, but as we also have to support GD, and thus have to distinguish between those 2. and want to be brave citizens that update the image dimensions, our code has some additional lines.

Back to fixed?

back to fixed.