straighten photo
Submitted by Adam Dingle
Assigned to cli..@..ba.org
Link to original bug (#715290)
Description
---- Reported by adam@yorba.org 2009-03-11 10:46:00 -0700 ----
Original Redmine bug id: 61
Original URL: http://redmine.yorba.org/issues/61
Searchable id: yorba-bug-61
Original author: Adam Dingle
Original description:
straighten photo
Related issues:
- related to shotwell - 4481: [straighten] Unitize EditingTools (Fixed)
- related to shotwell - 4483: [straighten] Make rotate_arb maintain the original aspect... (Fixed)
- related to shotwell - Task #4474 (closed): [straighten] Icon required for the straightening tool's t... (Fixed)
- related to shotwell - Task #4521 (closed): [straighten] Notify the rest of the app the effective siz... (Fixed)
- related to shotwell - 4663: [straighten] Center of rotation is placed inappropriately... (Fixed)
- related to shotwell - 4612: [straighten] crop tool does not keep crop rectangle in im... (Fixed)
- related to shotwell - Task #4471 (closed): [straighten] Skeletal UI for straighten tool (Fixed)
- related to shotwell - Task #4484 (closed): [straighten] Rework derotate_point() to work with the new... (Fixed)
- related to shotwell - 4473: [straighten] Implement angle awareness for crop tool (Fixed)
- related to shotwell - Task #4472 (closed): [straighten] Implement rotozooming (Fixed)
- related to shotwell - 4616: [straighten] straighten preview image is unacceptably blu... (Fixed)
- duplicated by shotwell - 4084: straighten photos by drawing a line (Duplicate)
- blocked by shotwell - 3887: Redeye: Significant graphical corruption occurs if the ma... (Fixed)
---- Additional Comments From shotwell-maint@gnome.bugs 2013-05-01 11:42:00 -0700 ----
History
Comment 1
Updated by Adam Dingle over 3 years ago
-
Target version deleted (
<strike>
__</strike>
)
Comment 2
Updated by Mirek Rewak over 3 years ago
I've just added my patch to have a straighten tool. This patch should be reviewed by some Shotwell developers because I was just playing with Shotwell code and Vala language and its bindings and it can be completly wrong :). It uses Cairo for rotating.
Comment 3
Updated by Jim Nelson over 3 years ago
- Status changed from Open to Review
- Assignee changed from Anonymous to Mirek Rewak
- Priority set to High
mrewak,
This is an impressive patch. We're quite pleased to see how you've modelled the tool like the others -- making an Undo/Redo command, persisting the transformation properly in the database, and placing it in the pipeline. Great work!
Some obvious things I'm seeing in the code you should be aware of:
- The patch doesn't compile with trunk as of today. There's been a couple of major check-ins recently that cause the problem. Specifically, the “altered†signal now specifies what was altered so observers can decide whether or not to act on it. In TransformablePhoto.set_raw_straighten you should do this:
notify_altered(new Alteration("image", "straighten"));
- I'm seeing hard tabs throughout the code. Our coding conventions are to use soft tabs. Also, we put open braces on the same line as the keyword.
We need more time to look over your work and we'll get back to you soon.
-- Jim
Comment 4
Updated by Mirek Rewak over 3 years ago
Nice to hear that my patch was not blamed :) I just looked how other tools were coded.
The patch is now updated to match the trunk version. And I fixed the code following your code convention and make some minor changes. The patch is quite big because I also stripped trailing spaces in the files.
Comment 5
Updated by Adam Dingle over 3 years ago
Mirek,
thanks for your updated patch. We've now had a chance to try it out more. As Jim said this looks quite promising. Unforfunately your patch is not completely sound when the user both crops and straightens. To see an example, open a photo and first straighten it about 30 degrees to the right. Now open the crop tool and crop out a small rectangle near the center right edge of the photo. After you close the tool, Shotwell will display a image different from the area you selected.
The combination of cropping and straightening is a little tricky, and we just had a discussion here at Yorba about how to handle this. We also looked at several commercial programs (iPhoto, Aperture, Lightroom) to see how they handle cropping and straightening. Here's what we've decided.
- We'd like to modify the crop tool so that it displays the entire image rotated at the current straightening angle. The user should be able to move the crop rectangle anywhere inside this rotated image. (This is similar to how cropping works in iPhoto and Lightroom.)
- When the user invokes the straighten tool, we'd like to show only the current cropped area, and straightening should rotate around the center of the cropped area. This is exactly how your patch works today. As the user straightens, we never want to see blank empty triangles rotate into the corners of the image.
- As you know, when an image is straightened it may also need to be cropped a bit so that enough pixels are available for rotating. When the user straightens an image, we'd like the straighten tool to actually adjust the image's crop rectangle as appropriate. In other words, if the user straightens an unedited image, then invokes the crop tool, the crop tool will display a crop rectangle which was calculated by the straighten tool so that it fits entirely inside the source image.
- In the pipeline, we think that cropping and straightening should be combined into a single pipeline stage. That stage will grab a crop area which is itself rotated at the straightening angle.
- Your current patch allows straightening by up to 90 degrees in increments of a single degree. We this this is more straightening than is necessary, and is too sensitive to mouse movements. Instead, we think we should allow straightening by up to about 20 degrees in increments of 0.1 degree.
- Please make sure that the red eye tool works properly in the presence of both cropping and straightening. You may need to rotate red eye coordinates in some cases.
I hope this is all clear. I'll also send you a screenshot by email to further illustrate what the crop tool might now look like. If you have any more questions feel free to ask. Thanks again for your help on this patch!
By the way, we're planning a string freeze for 0.7 in just a couple of weeks, on Thursday July 29. Your change will introduce at least a couple of new strings (“Straightenâ€, “Angleâ€) so it should be committed at least in some form by that date if you'd like it be in 0.7. There's no big hurry, though; we can always target 0.8 if this doesn't make 0.7.
Comment 6
Updated by Adam Dingle over 3 years ago
- Subject changed from straighten photo to [strings] straighten photo
Comment 7
Updated by Adam Dingle over 3 years ago
- Subject changed from [strings] straighten photo to straighten photo
Comment 8
Updated by Mirek Rewak over 3 years ago
Hi,
I read your notes after a break and now I see what you would like to get. I just looked how it is done in Picasa and I see that you are moving towards iPhoto. It could be quite complicated for me now because to do it properly straightening as you said must be tied with cropping and the cropping code is not clear for me yet (cropping with scaling stuff). But do not lose hope, I'm still hacking in the free time :)
Comment 9
Updated by Adam Dingle over 3 years ago
Mirek,
ok – glad to hear you're still interested in working on this. :) We're about to start working on 0.8, which won't ship until November, so there's plenty of time.
Comment 10
Updated by Adam Dingle about 3 years ago
-
Priority deleted (
<strike>
_High_</strike>
)
Dropping to medium since there hasn't been any activity for a while. If and when a new patch arrives we'll up this to high again.
Comment 11
Updated by Mirek Rewak about 3 years ago
Hm, it is really tricky to implement straightening you would like to have. I got following problems:
-
when the image is cropped, then straightened AND then cropped again (also crop box moved so the rotation center moved), what should be visible when straightening again? I saw that in Lighroom crop box is reseted to fit entire photo – I don't think it is correct but can be acceptable (but in there the crop and straighten controls are visible together). I don't know how it works in iPhoto.
-
what should be done when a user straightens uncropped photo? Should there be added crop box, and this box would be viewable as a normal crop operation when crop tool choosen?
-
to view entire rotated photo (e.g. when cropping straightened photo) its pixbuf should be enlarged to fit entire straightened image, so there should be some kind of “temporary†pixbuf only used for viewport I think.
Comment 12
Updated by Adam Dingle about 3 years ago
Replying to [comment:11 mrewak]:
Hm, it is really tricky to implement straightening you would like to have.
Yes, this is tricky! :)
I got following problems: – when the image is cropped, then straightened AND then cropped again (also crop box moved so the rotation center moved), what should be visible when straightening again? I saw that in Lighroom crop box is reseted to fit entire photo – I don't think it is correct but can be acceptable (but in there the crop and straighten controls are visible together). I don't know how it works in iPhoto.
I'm not sure I quite understand the problem here. In this situtation, we should display the contents of the current crop box, and should allow straightening around the center of that box. If I'm not understanding your question correctly, then please clarify with a more detailed example.
I find that it's easiest to think about all this like this: at any time there is a crop box which represents the portion of the image which the user sees. The crop box is always a rectangle, but it may be rotated, i.e. its edges might not be parallel to the edges of the original photo. The crop tool moves the crop box without changing its rotation. The straighten tool rotates the crop box around its center without (usually) translating it, though if the rotated crop box no longer fits in the original image then it must be translated slightly so that it all fits. As an example of that, if I first crop a small square in the upper left hand corner of a photo and then rotate, any rotation must nudge the crop box slightly so that it fits.
– what should be done when a user straightens uncropped photo? Should there be added crop box, and this box would be viewable as a normal crop operation when crop tool choosen?
Yes.
– to view entire rotated photo (e.g. when cropping straightened photo) its pixbuf should be enlarged to fit entire straightened image, so there should be some kind of “temporary†pixbuf only used for viewport I think.
That's an implementation question, so maybe Jim (the Shotwell tech lead) can comment on that.
Thanks again for your interest in working on this tricky feature!
Comment 13
Updated by Jim Nelson about 3 years ago
Replying to [comment:11 mrewak]:
– to view entire rotated photo (e.g. when cropping straightened photo) its pixbuf should be enlarged to fit entire straightened image, so there should be some kind of “temporary†pixbuf only used for viewport I think.
Yes, that makes sense. There's no problem with that, and if you look at the other interactive editing tools (crop, color adjustment) that's exactly what they do. In particular, both crop and color adjustment load the pixbuf with an Exception field that specifies what transformation not to make -- for example, we show the original photo during cropping, so it uses a temporary pixbuf with the crop removed.
Comment 14
Updated by Wolfgang Steitz almost 3 years ago
I would love to see this feature in one of the next releases. I just tested mrewak's patch and therefore made it compile with trunk. Also changed the straightening tools max angle and the step size.
Comment 15
Updated by Adam Dingle almost 3 years ago
@wolfer, thanks for the updates. I don't think we'll be able to take this, however, until the points I listed in my comments 5 and 12 above are implemented (which hasn't happened yet, presumably).
Comment 16
Updated by Adam Dingle over 2 years ago
- Priority set to High
Comment 17
Updated by Adam Dingle over 2 years ago
- Target version set to 0.10
Comment 18
Updated by Adam Dingle over 2 years ago
- Assignee changed from Mirek Rewak to Anonymous
Haven't heard back from either external contributor in several months; unassigning. We may look at this for 0.10.
Comment 19
Updated by Adam Dingle over 2 years ago
-
Target version deleted (
<strike>
_0.10_</strike>
)
Comment 20
Updated by Adam Dingle over 2 years ago
#3505has been marked as a duplicate of this ticket.
Comment 21
Updated by Anonymous over 2 years ago
I would appreciate this feature a lot…
I currently use the auto-rotate gimp plugin for this need (see http://registry.gimp.org/node/22910), which makes it really easy to straighten a landscape picture without bothering about the number of degrees: just draw a line wich should be either horizontal or vertical between two points, and that's it…
It lacks the auto cropping part, which implies a boring manual crop, and takes more time to open an external editor, but I really appreciate its core feature. Would it be a possible option for this functionnality in Shotwell?
Comment 22
Updated by Adam Dingle over 2 years ago
Replying to [comment:22 julien]:
Would it be a possible option for this functionnality in Shotwell?
Yes, this would be nice to have.
Comment 23
Updated by Adam Dingle over 2 years ago
- Target version set to 0.11
Comment 24
Updated by Clinton Rogers over 2 years ago
- Assignee changed from Anonymous to Clinton Rogers
Per the discussion in the kickoff meeting.
Comment 25
Updated by Clinton Rogers over 2 years ago
- Subject changed from straighten photo to [strings] straighten photo
Reason for the change: will add some strings requiring translation to Resources.vala
Comment 26
Updated by Adam Dingle over 2 years ago
- Subject changed from [strings] straighten photo to straighten photo
Comment 27
Updated by Adam Dingle over 2 years ago
I've opened 3 subtickets for different phases of our implementation:
- #3667 (closed): enhance pipeline to crop/straighten
- #3668 (closed): enhance crop tool
- #3669 (closed): straighten tool
Comment 28
Updated by Adam Dingle over 2 years ago
-
Target version deleted (
<strike>
_0.11_</strike>
)
Comment 29
Updated by Adam Dingle about 2 years ago
- Target version set to 0.12
Comment 30
Updated by Clinton Rogers almost 2 years ago
- % Done changed from 0 to 30
Comment 31
Updated by Clinton Rogers almost 2 years ago
- % Done changed from 30 to 50
Comment 32
Updated by Clinton Rogers almost 2 years ago
- % Done changed from 50 to 60
Comment 33
Updated by Clinton Rogers almost 2 years ago
- Status changed from Open to 5
- % Done changed from 60 to 100
- Resolution set to fixed
Victory!
Got 32767 EXP
Got 10 GP
Got Potion x 1
Shotwell gained a level!
Comment 34
Updated by Adam Dingle almost 2 years ago
Awesome - this was a long time coming! Thanks for all your hard work, Clinton!
Comment 35
Updated by Charles Lindsay 7 months ago
- Status changed from 5 to Fixed
--- Bug imported by chaz@yorba.org 2013-11-25 21:41 UTC ---
This bug was previously known as bug 61 at http://redmine.yorba.org/show_bug.cgi?id=61 Imported an attachment (id=261497) Imported an attachment (id=261498) Imported an attachment (id=261499) Imported an attachment (id=261500)
Unknown Component Using default product and component set in Parameters Unknown milestone "unknown in product shotwell. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Version: 0.12
Resolution: RESOLVED FIXED