Bug 114225 - Free rotation using an horizontal line
Summary: Free rotation using an horizontal line
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Plugin-Editor-Rotation (show other bugs)
Version: 0.9.0
Platform: Mandriva RPMs Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-11 16:02 UTC by Julien Narboux
Modified: 2016-06-30 11:53 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.0.0


Attachments
Marker points in free rotation plugin (390.10 KB, image/jpeg)
2009-04-15 20:18 UTC, Mikolaj Machowski
Details
After rotation (242.33 KB, image/jpeg)
2009-04-15 20:20 UTC, Mikolaj Machowski
Details
Clarify strings (1.28 KB, patch)
2009-04-15 20:55 UTC, Mikolaj Machowski
Details
rotation difference (-41.68°) (201.00 KB, image/jpeg)
2009-04-16 11:14 UTC, Andi Clemens
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Narboux 2005-10-11 16:02:18 UTC
Version:            (using KDE KDE 3.4.0)
Installed from:    Mandriva RPMs

I often use the free rotation plugin to fix the horizon.

It would be easier to have the ability to select two points on the horizon and click on "make it horizontal" to compute the rotation angle.

Of course, we could have the same feature for vertical lines too.
Comment 1 Arnd Baecker 2007-05-02 17:08:10 UTC
Would it be possible in some way to use the new routines for zooming
for the free rotation plugin as well?

Reason: usually it is very difficult to get the angles
right when the whole image is shown (small differences in angle
are essentially not visible). Only using a sufficiently large
magnification does allow for specifying the rotation correctly.
(This would still apply if the possibility of
selecting two points to define the angle was implemented.)


Also note, that this wish is similar (if not duplicate?) to 
http://bugs.kde.org/show_bug.cgi?id=118201
Comment 2 Mikolaj Machowski 2008-04-03 18:54:04 UTC
*** Bug 118201 has been marked as a duplicate of this bug. ***
Comment 3 Andi Clemens 2009-04-06 09:42:52 UTC
SVN commit 949893 by aclemens:

First implementation of the auto horizon feature.
The layout needs to be better and we need the same functionality for
vertical adjustments, but at least it is working.

CCBUG:114225

 M  +145 -15   freerotationtool.cpp  
 M  +16 -0     freerotationtool.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=949893
Comment 4 Andi Clemens 2009-04-07 14:25:16 UTC
I have implemented this feature now, it is working. But maybe we can improve the adding of points a little bit?

Any suggestions?

Andi
Comment 5 caulier.gilles 2009-04-07 15:05:47 UTC
yes,

depending of image content, point markers can be not visible. i recommend to use dotted line with white and red color to draw circle, and perhaps, a semi transparent content into the circle...

Gilles
Comment 6 Andi Clemens 2009-04-07 15:25:47 UTC
Gilles,

I just tested it with the dotted line, but this is even harder to see (red-white). Maybe we should just use the guide color, because this can be changed in the plugin. So if somebody has problems with seeing the dots, he can change the guide color to make it more visible.
I will test a little bit here and commit something later on.

Andi
Comment 7 caulier.gilles 2009-04-07 15:31:15 UTC
yes, of course, using color guide is fine

Gilles
Comment 8 Andi Clemens 2009-04-07 16:51:32 UTC
SVN commit 950675 by aclemens:

Draw points by using the guide color. To distinguish them from each
other, add their index number, too.

For most of my pictures, a red guide is not good. Yellow or orange seems
to be much more suitable. But now you can change the color of the points
so this shouldn't be a problem anymore.

CCBUG:114225

 M  +23 -10    imageguidewidget.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=950675
Comment 9 caulier.gilles 2009-04-07 16:59:26 UTC
Andi,

With all your changes, can you reproduce this dysfunction :

https://bugs.kde.org/show_bug.cgi?id=179766

Gilles
Comment 10 caulier.gilles 2009-04-07 17:00:57 UTC
Julien,

As you is original creator of this entry, and like you play with current code from svn, _please_ (:=))) can you checkout last changes from Andi and give a fresh feedback ?

Thanks in advance

Gilles
Comment 11 Andi Clemens 2009-04-07 17:10:20 UTC
Reproduce: Yes
Understand: No
;-)

But I will take a look again, I had looked at this issue months ago, maybe I find something now.

Andi
Comment 12 Julien Narboux 2009-04-07 17:21:54 UTC
Gilles, Andi,

I am testing this right now !
Thank you very much for your work Andi, this is a nice addition.

It seems to me that the circles are set one pixel higher and on the left of the current selected point.

Concerning the gui I think it would be more intuitive to have three choices:
-1- First radio button: Manually set rotation angle (main angle and fine angle)
-2- Second radio button: Fix horizontal line.
-3- Third radio button: Fix vertical line.

If one choose 1 or 2 the red and white cross is replaced by just a red and white segment with two small circle for the end points. Those end points could be moved by dragging them on the picture.

I think it would be a little bit better but anyway the current implementation is fine!

Thanks again,

Julien


Julien
Comment 13 Andi Clemens 2009-04-07 18:02:16 UTC
(In reply to comment #12)
> It seems to me that the circles are set one pixel higher and on the left of the
> current selected point.

I tried to align the middle point so many times, but the problem is that a 1px point (or line, I had a crosshair in the middle first) doesn't seem to get centered. If I use a 2px wide line, it is working. I tried so many combinations of radius and such things, but it never fits perfectly.
Also note that the preview is not as precise, because it needs to interpolate between reduced size and original image size. This can't be changed right now.
About the middle point: Maybe I'm missing something here. If somebody has an idea how to fix it?
Also I noticed that different colors make the dot appear more in the middle, it seems to be an optical illusion :-)
For example blue looks always better then red... well at least for me!
Comment 14 Andi Clemens 2009-04-07 18:08:05 UTC
Julien,

about the rest of your comment:
We will rewrite the image preview widget (some day), so I guess your suggestions will not be implemented in the next time, because we will need to redo this widget for all imageplugins.
It was planned to have one widget that fits all, not three like we have now.
But this is not done yet.

So I guess we will leave it like it is right now, we can of course change other things that are weird or unintuitive, but having some draggable circles is something that needs deep changes (that are only needed by the freerotate plugin at the moment).

Gilles, 
what do you think?

Andi
Comment 15 Julien Narboux 2009-04-07 18:20:37 UTC
Andi,

I understand that draggable circles requires to have some canvas in the preview widget. In the future, draggable objects over preview could also be used for other tools. For example for local edits: one would be able to move  local modification of contrast and lightness for example.

Thanks for you work,

Julien
Comment 16 Mikolaj Machowski 2009-04-07 21:27:32 UTC
Wow. Great thing. I have few ideas for simplification:

1. Make this new feature default option for Free-rotation
2. Don't use two buttons for setting points - one button which will assign consecutively two points.
2a. If you really feel 2 buttons are necessary at least make them the same color - I didn't understood why second circle isn't blue.
3. Similarly I am not sure if differentiation horizontal/vertical is that useful. Overwhelming majority of corrections is just for few degrees to strengthen horizon or make columns vertical. Simply heuristic: if line connecting two set points are less than 45 degrees from horizontal line make it horizontal adjustment, in the other case make it vertical.

With those 3 steps you can simplify that part of dialog to just two buttons.

4. Precision of set points - yes, this is problem now. "My" idea (from Hugin) - after button press appears small panning widget with eg. 20px x 20 px 1:1 (or with bigger zoom factor) of image. User can set there mark point precisely. Such widget would be useful also in perspective correction or even crom. Maybe not default action but with some modifier (Ctrl?).
Comment 17 Andi Clemens 2009-04-07 21:41:29 UTC
Mik,

about 2: I think two buttons are necessary (right now), because you simply can't set the points that exactly most of the time, so you want to adjust them a little bit. So if you set a point and you don't like it, clicking ONE button a second time would set the second point, what you might not like. So 2 buttons are the best at the moment I think.

2a) Yes, the points had different colors before, so it made sense. But now I use one color for the points, so the buttons should be fixed (will do this).

3) I don't agree. Sometimes you have such a bad image, that you have an angle bigger than 45°. If it would be totally automatic, the image would be adjusted vertically instead of horizontally. But we can discuss about this for sure :-)

1) Well if you had activated it, the setting is saved. So it will be default.
But I can set it completely default, too.
Maybe we should change the layout, putting the sliders at the bottom (with a checkbox "fine-tuning" or something like this)?

Andi
Comment 18 Mikolaj Machowski 2009-04-07 23:52:27 UTC
Andi,

Ad. 1 - yes I was thinking about moving sliders to bottom. Also if possible add labels Left / Right to sides of slider (above them to no force wider panel) to make direction of rotation clearer.

Ad. 2 - true, without possibility to set two points easy and precisely current solution is better; altough it would be good to get rid of one of them in future

Ad. 3 - don't agree; big majority of photos I make minor - few degrees corrections; here is the choice: make things easier for big majority of user scenarios with penalty of small inconvenience for the rest? Note, to "fix" this in my scenario you need only move main slider generously to one side.
Comment 19 Andi Clemens 2009-04-08 01:00:21 UTC
SVN commit 950871 by aclemens:

Re-arrange layout...
Setting the auto-adjust widgets as the main component might make it
harder for the average user to understand how to use this plugin.
What do you think?

CCBUG:114225

 M  +47 -37    freerotationtool.cpp  
 M  +3 -2      freerotationtool.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=950871
Comment 20 Andi Clemens 2009-04-08 09:24:48 UTC
After playing a lot with free-rotation in the last day, it really annoys me that the angle is saved when the tool is used.
I constantly have to reset the angle because I never want to use the same settings again on the next image. For me saving the angle doesn't make sense.
This becomes even worse now when we have the sliders disabled / hidden when "fine adjust" is not checked.
You either need to hit "defaults", but this will reset the auto-crop, too, or you need to check "fine-adjust", reset the angles and then apply the auto correction.

What do you think? Do we really need to save the angles here?

Andi
Comment 21 Andi Clemens 2009-04-08 09:28:54 UTC
Hmm now that I think about, I personally never want to save the tool settings, because most of the time I reset them anyway.
Maybe we should add a save button, that will not open a file dialog, but instead just call writeSettings(). So every time you think you want to save the settings, you can click the button, otherwise you just close the tool and the defaults will be saved.

What do you think about this? Personally I spent more time in resetting tools then using them.

Andi
Comment 22 Gerhard Kulzer 2009-04-08 09:57:23 UTC
I agree with, in the overwhelming majority of cases I have to reset to defaults, while sometimes I use for example white balance on a series of images. Your proposal looks good to me: no file dialog, just save the current settings to rc file. A manual reset to default values should then automatically reset the defaults in the rc files without need to push the save button.

Gerhard
Comment 23 Andi Clemens 2009-04-08 10:15:47 UTC
Yes, that's what I meant. Click the save button, settings go into the rc file, hit defaults button, defaults go into rc file.
If you don't click any of these buttons, nothing is saved.
Comment 24 Arnd Baecker 2009-04-08 10:18:03 UTC
a big "+1" from me - yes!
Comment 25 Mikolaj Machowski 2009-04-08 11:24:05 UTC
I agree. Please disable saving of settings for *all* plugins.

I will check new layout in the evening.
Comment 26 Andi Clemens 2009-04-08 12:26:19 UTC
Another possibility:

Make this save button a toggle button, maybe call it "keep settings".
If toggled, settings are saved, if not, defaults are saved. The difference to the former approach is that you can press the "defaults" button now without saving the settings in the rc file.
Maybe you want to save the settings, but for one image you want the defaults. Pressing the button will now only temporarly reset the settings, but on a next start of the tool all saved settings are restored again.

Hmm while I write this I don't think a toggle is a good idea. Maybe use a checkbox that says "Save settings". This checkbox becomes unchecked again on the next tool start.

There are so many ways to do this. Any other ideas?

Andi
Comment 27 Julien Narboux 2009-04-08 13:55:17 UTC
I think the idea of the checkbox is the best one.
But I think the status of the checkbox itself should be always saved.
People who like saving settings keep it checked, other keep it unchecked !

For some tools, there are some settings that IMHO should be always saved some should not be. For instance, I like to always have the same kind of preview (mouseover), but it does not make sense to save the amount of contrast.

Julien
Comment 28 caulier.gilles 2009-04-08 14:04:00 UTC
Andi,

I just follow the discussion. Nothing more.

Some tips : 

- Preview mode need to be moved in Editor Tool API. It will be more easy to save/restore settings about for all plugins. Currently, these modes are hosted in Preview Widgets.

- Take a care that we can really improve all plugins behaviors to merge all Preview Widgets to one common implementation. It will be long to do, but it's the only way to have a clean API. So, take a care to not try to rewritte all plugins if we make later a common implementation in preview widgets.

Note: we need to plan a coding sprint about somewhere. 3 days is enough, At least, you, me and Marcel. We can also, share this moment with kipi-plugins developers of course. there are a lots of subject to do... The thread is open.

Gilles
Comment 29 Andi Clemens 2009-04-08 17:19:26 UTC
Gilles,

I wasn't about to change anything now :-) I know that we want to create a new editor widget so this is just one more point we should think about in the future.

Andi
Comment 30 Mikolaj Machowski 2009-04-08 17:54:16 UTC
New layout

I think this is good direction:
1) grey circles are hardl to see in dark schemes; red is good to see in all schemes (OK, maybe some mad man will create bloody theme...)
2) to make things easier I suggest to build up point buttons from simple graphics to text "(1) Not set" and "(2) Not set" where "Not set" is text and (1), (2) your graphic. Or simply put (1),(2) in layout and put only "Not set" in button; bigger buttons are easier to hit; you can assign accelerator to them
2a) similarly adding graphics for Horizontal/Vertical buttons would make use clearer - some simple landscape icon for horizontal adjustment and column for verticals (just examples)
3) I know this is Preview widget issue and I read Gilles caveats but fast way for making precision easier would be possibility to zoom images in preview (without fancy panning widgets I wrote before); and all plugins would benefit from this greatly
4) Also not sure if hiding of sliders is good thing - greying them out? That would make possible to move Auto-crop option to the bottom - as it is the final retouch
Comment 31 Andi Clemens 2009-04-08 17:57:13 UTC
SVN commit 951186 by aclemens:

Disabling the saving of the angle in this plugin for now, I really don't
think someone needs this and it is more annoying then helpful.

CCBUG:114225

 M  +7 -4      freerotationtool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=951186
Comment 32 Andi Clemens 2009-04-08 18:15:59 UTC
About 1) I can make the text / circles appear in the system text color of course, this should fix it
About 2a) I'm right now implementing the one-button solution... maybe this is really enough, so graphics for hori / verti are not needed.
About 4) Ok I only disable the sliders and I have moved the auto-crop down
Comment 33 Andi Clemens 2009-04-08 18:17:08 UTC
SVN commit 951194 by aclemens:

Only disable manual adjust sliders, don't hide them. Move auto-crop to
the bottom of the tool settings.

CCBUG:114225

 M  +10 -9     freerotationtool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=951194
Comment 34 Andi Clemens 2009-04-08 20:24:30 UTC
SVN commit 951228 by aclemens:

Remove the horizontal and vertical adjust buttons, try to detect the
appropriate method automatically. This means that only one button is
needed now.
The buttons now contain the point information, but this looks strange.
Any idea how to make the buttons look better? Right now the icon moves
to the left when the text is expanded, it would look better when it is
already on the left side of the button.

CCBUG:114225

 M  +47 -57    freerotationtool.cpp  
 M  +9 -19     freerotationtool.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=951228
Comment 35 Mikolaj Machowski 2009-04-15 20:08:50 UTC
Thanks Andi. Interface looks perfect IMO. But... leveling isn't precise. Can anyone confirm?

Especially in vertical or bigger rotations I have visible problems.
Comment 36 Mikolaj Machowski 2009-04-15 20:18:44 UTC
Created attachment 32860 [details]
Marker points in free rotation plugin

Markers set in easy to recognize points.
Comment 37 Mikolaj Machowski 2009-04-15 20:20:51 UTC
Created attachment 32861 [details]
After rotation

After rotation. Guideline should overlay two marked previously corners. It is not pixel precise but error in calculation is visible.
Comment 38 Andi Clemens 2009-04-15 20:34:59 UTC
Yes sometimes it doesn't work, which is strange. I realized that, too. But after rotating again, it suddenly worked :-)
Need to find out why. But normally it works here, even with little tiny super mini (...) angles.
Comment 39 Andi Clemens 2009-04-15 20:46:27 UTC
I cut out your image from the screenshot and tested it. I get an angle of -41.95°, which is not so good. But turning it into -41.00° gives a perfect result. I don't know why this is happening sometimes, it seems to happen more often when rotating CCW. Eventually I should round down the angle in that case, it seems to work better then taking the angle from the computation.

Maybe this happens due to the imprecise setting of the points, because if you try it many times, it will look better.

I'll see if I can fix this...
Comment 40 Mikolaj Machowski 2009-04-15 20:52:40 UTC
After more tests I am pretty much sure this miscalculation happens only when rotating left - no matter whether this is vertical or horizontal adjustment
Comment 41 Mikolaj Machowski 2009-04-15 20:55:29 UTC
Created attachment 32862 [details]
Clarify strings

Tiny patch to clarify strings. "not set" -> "Click to set"
Comment 42 Mikolaj Machowski 2009-04-15 20:59:38 UTC
One small bug: when clicking first button (2) point on preview is labelled (1). When you click (1), new marker is labeled (1) and old relabeled as (2). Minor glitch but glitch.

Suggestion: make only (1) button active at the beginning and un-gray out (2) only after setting (1).
Comment 43 Andi Clemens 2009-04-15 22:03:32 UTC
SVN commit 954466 by aclemens:

Change button label

CCBUG:114225

 M  +7 -7      freerotationtool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=954466
Comment 44 Andi Clemens 2009-04-15 22:03:40 UTC
SVN commit 954467 by aclemens:

experimental: draw a line between points

CCBUG:114225

 M  +1 -1      imageplugins/freerotation/freerotationtool.cpp  
 M  +20 -1     libs/widgets/imageplugins/imageguidewidget.cpp  
 M  +1 -1      libs/widgets/imageplugins/imageguidewidget.h  
 M  +2 -2      libs/widgets/imageplugins/imagewidget.cpp  
 M  +1 -1      libs/widgets/imageplugins/imagewidget.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=954467
Comment 45 Andi Clemens 2009-04-15 22:03:54 UTC
SVN commit 954470 by aclemens:

Disable button2 if P1 has not been set

CCBUG:114225

 M  +9 -0      freerotationtool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=954470
Comment 46 Julien Narboux 2009-04-16 08:30:36 UTC
Nice !
I think the antialiasing option should be at the bottom near "auto crop" option.
First you would have automatic or manual angle, then the options.

Julien
Comment 47 Andi Clemens 2009-04-16 11:11:26 UTC
There seems to be a problem with our rotation algorithm (not the auto rotation).

Mik,

take your image in the above screenshot and rotate it
-41.68°
in showFoto and Gimp. The Gimp version looks right, the showFoto version does not.
I you take both results and paste them into a Gimp document, setting the top layer to "difference", you can see the problem more clearly.

Somehow rotating to the left is wrong.

Gilles,

any idea?
Can someone confirm this difference between showFoto and Gimp?

Andi
Comment 48 Andi Clemens 2009-04-16 11:14:53 UTC
Created attachment 32870 [details]
rotation difference (-41.68°)

Look here, these are the two results merged in Gimp with the "difference" layer mode.
Normally this image should be perfectly black, but as you can see it is not.
Comment 49 caulier.gilles 2009-04-16 11:18:46 UTC
This can be a side effect with Anti-aliasing filter.

Gimp and digiKam rotation aliasing code are not the same.

Gilles
Comment 50 Andi Clemens 2009-04-16 11:26:23 UTC
No I guess I found the problem...

Problem is I set the angle -41.68 as
Main angle: -41
fine angle: 0.68

but it should be
Main angle: -41
fine angle: -0.68

So if turning to the left, I need to add a negative value...
I'll check that.
If it works, I will close this bugreport, because the main issue has been solved. All further discussion can take place on the maillinglist or in the closed bugreport.

Andi
Comment 51 Andi Clemens 2009-04-16 11:32:30 UTC
SVN commit 954775 by aclemens:

Fix rotation to the left. Now the auto-correction should work perfectly
fine.

BUG:114225

 M  +1 -0      freerotationtool.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=954775
Comment 52 Mikolaj Machowski 2009-04-16 17:37:50 UTC
Thanks Andi! :)
Comment 53 Andi Clemens 2009-04-26 20:18:52 UTC
Somehow I'm not happy with the headline for the auto correction. Right now it reads "Automatic Adjustment", but the average user might think: "What is adjusted? What will this tool do?"
I showed the feature to some colleagues and they all automatically asked the above questions.
A better title would be "Automatic horizon adjustment", but unfortunately this tool also fixes vertical lines.
Do you have better suggestions? I really think "Automatic Adjustment" is wrong and not clear enough.

Andi
Comment 54 Andi Clemens 2009-04-26 20:22:00 UTC
Hmm just a quick thought:
Maybe we can add a label beneath the headline?
This label can be used for a very quick description. A tooltip or whatsThis message might be insufficient, and a normal user often doesn't know how to invoke the whatsThis messages anyway.
Comment 55 Mikolaj Machowski 2009-04-26 22:38:17 UTC
"Auto-correction" ?

"Automatic horizon adjustment" isn't bad. Yes, plugin can also fix things by using vertical lines but they are rather some help for leveling of image.
Comment 56 Andi Clemens 2009-04-27 09:46:02 UTC
Yes, this is my problem right now. Horizon correction is more meaningful, but you get the impression that it will only straighten horizontal lines. The "cool" thing about the auto correction is that it also detects vertical adjustment (even in Photoshop you can't do this automatically... :-), it will only work horizontal as far as I know).
So I'm looking for a way to show users that they can do this, without reading the manual first. Maybe a tooltip for the whole container widget is enough, I'll see what this will look like.

Andi
Comment 57 caulier.gilles 2009-04-27 10:07:18 UTC
>Maybe a tooltip for the whole container widget is enough.

I think a visible help in settings container will be more readable... Tooltip is only displayed when user ask it. He must know that tooltip is available over preview widget.

To resume tooltip can be not enough here (:=)

Gilles
Comment 58 Andi Clemens 2009-04-27 10:17:39 UTC
I will be offline for some hours, but I'm gonna think about something...

Andi
Comment 59 Andi Clemens 2009-05-06 14:02:51 UTC
One little note on the centering of the middle point:
I just discovered that the point is sometimes more in the upper left of the circle, sometimes more in the bottom right. I can't tell why.
In showFoto it mostly looks the best, in the IE the centerpoint is moved to the upper left corner.
Weird.
Sometimes this happens in showFoto, too, after several restarts (but always using the same image and application layout).
I don't understand this. Has somebody an idea what could be wrong?

Andi