Rotate images on Manage images tab

As Node Gallery already depends on imagecache, which in turn depends on ImageAPI, this would theoretically be possible (with only few lines of code). If Justin gives his blessing for this, I'll implement it.

There are two things this will not be able to solve:
- image API does not support lossless jpeg transformations, thus every rotation could lead to degradation
- your browser will cache the file, and thus only display the rotated image after cache clearing

We cannot simply rename the image, because the files are managed by Drupal. We don't know which other modules are accessing them, what modifications they do to the image (say filefield_paths, etc). I don't think this is an option. Cache controls or query strings are too far away from the purpose of this module, so they are no option either.
I think the point is: if we want this feature we have to live with the cache and degradation issue, otherwise, this feature cannot get in.

I was curios how much code this feature would require, so I implemented a PoC. Note that my point in #1 still stands, specifically that I'll wait for Justins call on this.

This was brought up over at #870552: Ability to rotate during upload would be handy too.

I'm not wholeheartedly against it, I just don't want it to become a time-sink.

@scroogie, I'm pretty sure that the caching issue will only be an issue in the case where Apache has the mod_expires module loaded. Since you're modifying the file on the filesystem, Apache will note the modified time, and clients will get the updates. Only when mod_expires is loaded does the caching become to the point where the client doesn't even check with the server for 2 weeks on any file not a .php file.

I've got two issues with the POC. First, I know the next issue after committing this will be: "My users aren't smart enough to know the direction and degrees to rotate the image - can we do some sort of jquery preview before saving the changes?"

My other major problem is the loss of quality - my main use of NG is for a photographer, and the loss of quality would be an issue.

I didn't even apply it, but this patch looks *very* interesting: http://drupal.org/node/173498#comment-2126640. It looks like it may address both issues. scroogie, do you think it would work, and can you can port it?

The last minor issue I have, is that it have a checkbox in the relationship settings so that admins can turn it off if they want. That is easy to remedy.

The javascript in there uses an external library, which in turn uses the browser canvas to rotate the image. I doubt that this provides better quality. I'd actually presume it provides much worse results. The server side code they use is nearly the same, except that they temporarily change the quality options. Perhaps thats worth a though, and I'll investigate this. Please note, however, that the quality loss is implicit when working with JPEGs, as JPEG uses a quantized DCT-based approximation. We would have to rely on special tools such as jpegtran to do a *real* lossless rotation. On most linux systems, this should even be available, but this would mean some more custom code to call the binary.

Okay, here it is, setting the backend quality to 100 for jpegs.

I'll think about the preview issue tomorrow. I'm not sure how we could implement the interface on the manage images page. Do you have an idea for the UI?

I somehow overread the admin setting. I'll add that tomorrow.

Perhaps just two very small images below the image, one showing a rotate right, the other a rotate left? A quick images.google.com search gives http://www.orthotemplate.com/Icons/RotateRight.png - way too big, but you get the point.

Mhm, yeah, but I want it to behave similar to the other operations on manage images, so it shouldn't rotate instantly but only after submit. The problem is, if it's a simple press button, how do I visually keep track of the selected rotation.

A last word on the quality loss. If you want to make sure not to loose quality, you can use PNG, which uses Lempel-Ziv compression (basically gzip) and thus in contrast to jpeg won't loose quality no matter how often you recode it.

Here, I think this is a good tradeoff. It still displays radios, but it updates the preview picture on client-side. Relationship setting is also implemented. We can vary the UI a bit, e.g. using a seperate thumbnail and/or displaying arrows instead.

Technically, I used CSS transforms, because 1) in this case it's much easier to circumvent CSS differences in browsers than JS differences and 2) once CSS3 is more common, we can easily remove the workarounds, which would only leave 1 directive.

Note: this adds a new file, so beware the usual related CVS quirks.

Reroll for latest cvs.

Attaching a re-roll for latest HEAD.

In reviewing this, there's one serious issue, and one issue that I don't know about for sure.

The more serious issue is that on my dev server, if I attempt to rotate just one image (it's a large 1.5MB image), I get PHP timeouts. After digging for a bit, I hit the status report, and found:

GD Image Rotation Low Quality / Poor Performance
The installed version of PHP GD does not support image rotations. It was probably compiled using the official GD libraries from http://www.libgd.org instead of the GD library bundled with PHP. You should recompile PHP --with-gd using the bundled GD library. See PHP Manual for more information. A slower implementation of imagerotate() written in PHP will used in the interim.

Poor performance indeed - more than 30 seconds to rotate one large image! So, at a minimum, we should probably disable the feature when PHP GD rotations are not available. My next concern, is how much time does it take to rotate, say, 5 large images when PHP GD rotations are available?

Second, I just don't like the UI yet. It's certainly functional, but a lot of users may not know what CW and CCW mean. I just hit flickr to see what they do, and it's pretty impressive. What if you had a link on each row that says simply "rotate", and when clicked, it presented a popup div that showed a preview thumbnail of all three possible rotations, allowing the user to select which they want?

That sounds like a lot of work to me, but maybe it's not. Thoughts?

Did you also make sure that you're *clients* cache is cleared? Try pressing F5 or Ctrl+F5 a couple of times.

Weird, I guess there must be an Image API problem on your configuration then. Is there something in /admin/reports/dblog ?

Re #13:

The fallback is provided by the Image API module. I didn't know that it was *that* slow. We can check this at runtime indeed.

A preview dialog as you describe would not be that much work. I'd like to rely on jquery UI then, though, because using a seperate window is always messy when it comes to writing back the return value. With jquery UI I could easily use a dialog. If I find the time I will implement a PoC. I will most probably need help with the styling.

That is a bug in ImageAPI then. Which version do you use?

Here is the next patch that addresses #13:
- added a check for imagerotate if GD is the ImageAPI backend
- added a Jquery UI dialog with thumbnail preview for easier selection

Cheers

Forgot to address this: "My next concern, is how much time does it take to rotate, say, 5 large images when PHP GD rotations are available?"

I tried this on my local install. I rotated 20 hubble images of 1280x1280. I'd say it took around 1.5 seconds per four images.

The problem seems to be the background color. According to ImageAPI documentation, this parameter is optional and should default to transparency. Please enable debugging and have a look at the execute commandline which will be displayed after an operation.

Revert the old patch (patch -p0 -R < patch_file) and try this one.

Mhm, I just noticed that I included unrelated changes. Have to check my patches of today. Here is the right one.

Mhm, the command line looks good now though. Do you still get the "ImageMagick reported error code 1" in the dblog?

Yes, this is the browser cache. I don't think this is actually bad, though. It will most probably just hit the adminstrators. We could show a message to point that out.
Of course we could also try some fancy stuff like unique query strings or cache headers, but I don't see a convincing solution right now.

igorik says a couple of times that he's clearing *drupal* cache. igorik, is it the drupal cache you're having to clear, or your browser's cache?

Here's my thoughts on this feature. Andre, if you can find and fix the cache bug in the next day or so, I think we can slip it in before alpha2. I'm debating on whether or not to ship it enabled or disabled - mainly because I don't know how much browser compatibility testing has been done. I'm thinking we can ship alpha2 with it enabled, so that users will test it for us. If it becomes a large headache with multiple browsers, we can still ship it with 3.0, but just have it ship disabled (with hook_help() on compatibility issues).

Is that agreeable to everyone?

No time to look into code, but I did have time to test a bit. I can vouch for what igorik is saying, but for me, instead of clearing Drupal caches, I was having to flush my imagecache presets to be able to see my changes. A quick glance at the code shows that it's calling imagecache_image_flush(), so it may be something with my install.

I tried all sorts of things now, but I don't see any cache bugs on my installations. A Ctrl+F5 always fixes this for me, no matter what I do.

Yes, that sounds about right. I played around with query strings, but it makes no sense, as we don't know which visitor has which version cached. I now believe the only solution is to rename the image. I'm not exactly sure about the consequences though, especially in combination with the gazillion other modules out there. Justin, what do you think about it?

No time right now, but here is what we talked about on IRC. To get in this would need testing, especially with other file modules, the jquery_ui dependency must be optional, and the UI needs to be discussed.

Just a quick update to subscribers on this issue - this feature won't make it in for alpha2. We'll plan on doing an alpha3 shortly after that will incorporate this feature and any bugfixes found from alpha2. With any luck, beta1 will come after alpha3.

Great to see this is happening. Nice work.

Reroll for latest head and making additional dialog conditional on existance of jquery_ui.

- Needs testing with file modules
- Would be nice if someone good at theming could beautify the dialog or give mockups for a better UI

works!
could rotation be when viewing an image instead in the manage tab?

Committed to dev.

After speaking with @scroogie on this, we agree that the UI needs *something*, but we're not able to determine exactly what to do. However, the backend code is solid and tested, therefore this particular feature request is fixed.

I committed this, and by default the feature is enabled. This was intentional, so that we can get some eyes from the community looking at this. Please post any recommendations on what to do with the UI or any other problems in a new issue. Minor fixes to existing features won't hold up the beta cycle.

Thanks @scroogie for all your hard work on this - I believe we're the only gallery solution that offers integrated image rotation now!

This is so sick. Awesome work.

I agree that the UI seems to be not yet perfect but I cannot point a finger to it.

edit: How do we disable this feature? Did I miss a setting or is such a setting not yet included?

There should be a setting in the manage images settings: "Enable the rotation of images on the manage images page".

Indeed. I simply missed it. I have to say it again: Great stuff here.

This feature has caused a significant bug: #1026998: site design without style on ie7/8. node_gallery.css validation. Please see my comment on that issue for a fix.