Bug 179548 - additional guide for aspect ratio crop [patches for KDE3 and KDE4]
Summary: additional guide for aspect ratio crop [patches for KDE3 and KDE4]
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Plugin-Editor-Crop (show other bugs)
Version: 4.9.0
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-03 21:50 UTC by Simon Oosthoek
Modified: 2016-06-29 19:35 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 0.10.0
Sentry Crash Report:


Attachments
patch with my (not compiling) changes (9.06 KB, patch)
2009-01-06 22:01 UTC, Simon Oosthoek
Details
this patch should work (10.84 KB, patch)
2009-01-07 22:35 UTC, Simon Oosthoek
Details
New - simpler - patch for DM (3.03 KB, patch)
2009-01-09 18:07 UTC, Mikolaj Machowski
Details
this one will also add the dotted coloured line (the ugly way) (3.67 KB, patch)
2009-01-11 13:48 UTC, Simon Oosthoek
Details
final (I hope) version of patch for 0.9.x (3.78 KB, patch)
2009-01-11 15:18 UTC, Mikolaj Machowski
Details
Adding feature for 0.10 (3.79 KB, patch)
2009-01-11 15:29 UTC, Mikolaj Machowski
Details
This patch shows how I meant in the previous comment, to draw the lines (16.30 KB, patch)
2009-01-11 16:13 UTC, Simon Oosthoek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Oosthoek 2009-01-03 21:50:01 UTC
Version:           0.9.5-svn (using KDE 3.5.10)
OS:                Linux
Installed from:    Compiled From Sources

I'd like to use diagonal guidelines in the aspect ration cropping tool to assist in selecting according to the diagonal method concept as described here: http://www.diagonalmethod.info/
The "Harmonious Triangles" guidelines only implements half of this method, the full DM uses 45 degree lines from all corners...

Apparently this has been requested/implemented earlier for the Gimp, but it wasn't accepted into trunk for some reason:
http://www.mail-archive.com/gimp-developer@lists.xcf.berkeley.edu/msg14463.html

It seems the feature is currently implemented in Photoshop/lightroom
(apparently without informing the author(s) of the original article, I suppose it would be nice to send them an e-mail if it is implemented in Digikam's Editor)

Thanks in advance,

Simon

PS, I also requested this feature on the gimp's bugzilla, since I also use that a lot to edit images...
Comment 1 caulier.gilles 2009-01-03 21:55:28 UTC
I cannot see any problem to implement it in digiKam

More viewpoints are welcome.

Note : link to composition guide web article is very nice to read.

Gilles Caulier
Comment 2 Mikolaj Machowski 2009-01-04 00:34:23 UTC
To paraphrase old saying:
"The nice thing about methods is that there are so many to choose from" :)

I ran this simple script (bash, ImageMagick, cut):
------------------------------------------------------------
#!/bin/bash
SIZE=`identify -format "%w %h" $1`
W=`echo $SIZE | cut -d' ' -f1`
H=`echo $SIZE | cut -d' ' -f2`
if [ $H -gt $W ]; then
convert $1 -stroke black \
		   -draw "line 0,0 $W,$W" \
		   -draw "line 0,$W $W,0" \
		   -draw "line 0,$(($H-$W)) $W,$H" \
		   -draw "line 0,$H $W,$(($H-$W))" $1-dm.jpg
else
convert $1 -stroke black \
		   -draw "line 0,0 $H,$H" \
		   -draw "line 0,$H $H,0" \
		   -draw "line $(($W-$H)),0 $W,$H" \
		   -draw "line $(($W-$H)),$H $W,0" $1-dm.jpg
fi
--------------------------------------------------------------------
Over my collection of few thousands of drawings and etchings, from 16th to 20th century. Results aren't very convincing, but: percentage of images where this theory is more often correct (although not always) are from beginning of 20th century onwards. Maybe some new, still existing school of composition? On Monday I will be able to test it on more examples.
Comment 3 Simon Oosthoek 2009-01-04 10:47:17 UTC
As can be interpreted from the site, it doesn't apply to all pictures, since not all of them are primarily about a few details. It is interesting as a method to put emphasis on a few important details, or to check in an image where the focus would be according to the diagonal method.

BTW, on a second look, the harmonious triangles don't even do half of the DM, since the corner/side diagonals are perpendicular to the corner/corner diagonal, which is not 45 degrees from the corners in almost all cases.

Cheers

Simon
Comment 4 Simon Oosthoek 2009-01-04 13:12:54 UTC
Ok, here's a picture I had found on photosight.ru a while ago (I'd forgotten I'd stored it somewhere), but it's a really catching portrait:

http://margo.student.utwente.nl/simon/temp/2241460.jpg

using Mikolaj's script:

http://margo.student.utwente.nl/simon/temp/2241460.jpg-dm.jpg

Look at the line crossing the eye, which is also parallel to the line of the face. And the other diagonal line carrying the chin. I'd say this is a perfect example for the diagonal method....

BTW, I don't have copyrights to this picture, so I won't leave it up on my site for long, however it can also be found here: http://www.photosight.ru/photos/2241460/
Comment 5 Mikolaj Machowski 2009-01-05 11:44:36 UTC
OK, tested it on paintings (where artists were more concerned with composition) and idea has more merit. 
Comment 6 Simon Oosthoek 2009-01-05 20:45:50 UTC
I don't know how to make all the right changes in the code, but the drawing section in imageselectionwidget.cpp should be somewhat like this I presume?

case DiagonalMethod:
{
// Move coordinates to top, left
	p.translate(d->localRegionSelection.topLeft().x(),
		d->localRegionSelection.topLeft().y());

// Flip horizontal.
// Flip verical.
	// noop; DiagonalMethod is symmetrical

	float w = (float)d->localRegionSelection.width();
	float h = (float)d->localRegionSelection.height();
	p.setPen(QPen(Qt::white, d->guideSize, Qt::SolidLine));

	if (w > h) {
		p.drawLine( 0, 0, h, h);
		p.drawLine( 0, h, h, 0);
		p.drawLine( w-h, 0, w, h);
		p.drawLine( w-h, h, w, 0);
		
	} else {
		p.drawLine( 0, 0, w, w);
		p.drawLine( 0, w, w, 0);
		p.drawLine( 0, h-w, w, h);
		p.drawLine( 0, h, w, w-h);
	}
	break;
}
Comment 7 caulier.gilles 2009-01-06 08:03:05 UTC
Simon,

If you want to experiment some code it's easy. Look here :

http://lxr.kde.org/source/extragear/graphics/digikam/imageplugins/coreplugin/ratiocrop/imageselectionwidget.cpp#708

Composition guide are draw in ImageSelectionWidget::updatePixmap() method. If you want to test your changes, you just need to recompile parent folder and install it (it's a plugin for image editor):

http://lxr.kde.org/source/extragear/graphics/digikam/imageplugins/coreplugin/

Just enter this command:

#make 
#su
#make install

... and rerun digiKam to test

Best

Gilles Caulier


Comment 8 caulier.gilles 2009-01-06 08:06:49 UTC
Simon, 

Warning. I see that you play with 0.9.5 code. It's KDE3 version where no new features is added now. We play with KDE4 code now. 

But like your wish sound simple to code and will be not very intrusive, it will be easy to backport patch from KDE3 to KDE4.

To contrib, look here :

http://www.digikam.org/drupal/contrib

To checkout code from svn :

http://www.digikam.org/drupal/download/svn

Gilles Caulier
Comment 9 Simon Oosthoek 2009-01-06 21:36:12 UTC
Gilles,

I'm afraid I don't have a good system available to play with 4.x code, so this will have to do for now :-(

I hi-jacked the HarmoniousTriangles option to test my code and found a bug ;-)
The last w-h, should be h-w, but it worked fine otherwise...

Working it into the ratiocrop code as a proper option will take me some more effort (time and understanding where to make modifications)

Mainly I'm glad I got it to work for myself now, if someone beats me to integrating it, great!

/Simon
Comment 10 Simon Oosthoek 2009-01-06 22:01:37 UTC
Created attachment 29983 [details]
patch with my (not compiling) changes

Well, I tried to add the whole thing, but I get stuck when I get the error:

...
make[3]: Entering directory `/home/simon/src/kde3/graphics/digikam/imageplugins/coreplugin/ratiocrop'
/bin/bash ../../../../libtool --silent --tag=CXX   --mode=compile g++ -DHAVE_CONFIG_H -I. -I../../../.. -I../../../../digikam/utilities/imageeditor/editor -I../../../../digikam/utilities/imageeditor/canvas -I../../../../digikam/libs/histogram -I../../../../digikam/libs/levels -I../../../../digikam/libs/curves -I../../../../digikam/libs/whitebalance -I../../../../digikam/libs/widgets/common -I../../../../digikam/libs/widgets/iccprofiles -I../../../../digikam/libs/widgets/imageplugins -I../../../../digikam/libs/dialogs -I../../../../digikam/libs/dimg -I../../../../digikam/libs/dmetadata -I../../../../digikam/libs/dimg/filters -I../../../../digikam/digikam  -I/usr/include/kde -I/usr/share/qt3/include -I.    -DQT_THREAD_SUPPORT  -D_REENTRANT  -fno-tree-pre -Wno-long-long -Wundef -ansi -D_XOPEN_SOURCE=500 -D_BSD_SOURCE -Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith -O2 -Wformat-security -Wmissing-format-attribute -Wno-non-virtual-dtor -fno-exceptions -fno-check-new -fno-common -DQT_CLEAN_NAMESPACE -DQT_NO_ASCII_CAST -DQT_NO_STL -DQT_NO_COMPAT -DQT_NO_TRANSLATION -DQT_CLEAN_NAMESPACE  -MT ratiocroptool.lo -MD -MP -MF .deps/ratiocroptool.Tpo -c -o ratiocroptool.lo ratiocroptool.cpp
ratiocroptool.cpp: In member function 'void DigikamImagesPluginCore::RatioCropTool::slotGuideTypeChanged(int)':
ratiocroptool.cpp:752: error: 'DiagonalMethod' is not a member of 'DigikamImagesPluginCore::ImageSelectionWidget'
ratiocroptool.cpp:784: error: no matching function for call to 'DigikamImagesPluginCore::ImageSelectionWidget::setGoldenGuideTypes(bool, bool, bool, bool, bool, bool, bool)'
imageselectionwidget.h:109: note: candidates are: void DigikamImagesPluginCore::ImageSelectionWidget::setGoldenGuideTypes(bool, bool, bool, bool, bool, bool)
make[3]: *** [ratiocroptool.lo] Error 1
make[3]: Leaving directory `/home/simon/src/kde3/graphics/digikam/imageplugins/coreplugin/ratiocrop'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/home/simon/src/kde3/graphics/digikam/imageplugins/coreplugin'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/simon/src/kde3/graphics/digikam/imageplugins'
make: *** [all-recursive] Error 1
Comment 11 Simon Oosthoek 2009-01-07 22:35:23 UTC
Created attachment 30020 [details]
this patch should work

This one should work, I'd forgotten to change some names in the previous patch...

At least it worked for me, the patch is run in the ratiocrop directory relative to the svn version

Cheers

Simon

PS, feel free to incorporate it, if you want a license term (though it's a small patch, I doubt it's necessary), consider it "GPLv2 or later"....
Comment 12 caulier.gilles 2009-01-07 23:01:01 UTC
Mik,

Can you test this patch and give me a feedback. I will review it later.

Thanks in advance

Gilles
Comment 13 Mikolaj Machowski 2009-01-07 23:53:34 UTC
Against which svn version is this patch? With 907203 it is not applicable.
Comment 14 Simon Oosthoek 2009-01-08 08:27:46 UTC
This is against the kde3 version in svn...
Comment 15 Andi Clemens 2009-01-08 10:25:33 UTC
It compiles fine (and seems to work correct) under KDE3...

Andi
Comment 16 Mikolaj Machowski 2009-01-08 21:22:25 UTC
Truly weird, it patches, it compiles, but I don't see it in interface...
Comment 17 Andi Clemens 2009-01-08 21:59:05 UTC
Mik,

you need to install digiKam, since it is a imageplugin that has been patched and it needs to be present in the plugin folder, it will not work if you start it from your build path (the same as with kipi-plugins).

Andi
Comment 18 Mikolaj Machowski 2009-01-08 22:16:02 UTC
I installed digiKam. Still nothing. Never mind. From code looks like changes in interface looks in line with the rest of plugin, you wrote it works. Now only Gilles have to review it :)
Comment 19 Andi Clemens 2009-01-08 22:30:23 UTC
This is weird. You are looking at the correct plugin? :-) 
Aspect Ratio Crop?

I can see it... maybe it has been cached somehow (but I couldn't think of how this would happen). Maybe running "kdeinit4" fixes it?

Andi
Comment 20 Mikolaj Machowski 2009-01-09 01:35:24 UTC
OK, don't know why but old build system turned off imageplugins and themedesigner. Whatever.

Ad rem - I was wrong when reading patch, interface should be fixed:

- DM is separate way of composition its place is in menu, not in checkboxes
- flipping does nothing for this method (it is symmetrical) so when DM is selected all checkboxes should be deactivated.
Comment 21 Simon Oosthoek 2009-01-09 10:02:07 UTC
Mikolaj

I agree with your comments, however I'm afraid my knowledge of the code and libraries is not sufficient to do this myself :-(

Would you be willing to do this, or give me a hint on how to do this myself?

Cheers

Simon
Comment 22 Mikolaj Machowski 2009-01-09 18:07:40 UTC
Created attachment 30063 [details]
New - simpler - patch for DM

This patch removes unnecessary elements/interactions from interface, also fixes whitespace issues from previous patches.

One bug still remains (don't know how to fix this) - guides in Diagonal Method don't react on changes in color and size.
Comment 23 Simon Oosthoek 2009-01-10 23:29:50 UTC
Hi Mikolaj, your patch works fine as well and deactivating the flips is better this way.

Regarding the colour, it appears that the others are all drawing twice, once in white, once in dotted line with the selected colour. This seems rather ugly in the code, because it's all code that appears twice. I'd make a new function to draw twice the same line in one go, but I don't know how much work this would be to implement this now in existing code...

Cheers

Simon
Comment 24 Simon Oosthoek 2009-01-11 13:48:48 UTC
Created attachment 30127 [details]
this one will also add the dotted coloured line (the ugly way)

In this patch, I've added the code to draw the dotted line in the way that all the other methods do it (just copy the code after a change of pen style)

The good way to solve this and all the other methods would be to write a function/method that will draw two lines (e.g. drawGuideLine() ) and use that instead of drawLine() and setPen().

void drawGuideLine( QPainter* p, int x1, y1, x2, y2) {
 p->setPen(QPen(Qt::white, d->guideSize, Qt::SolidLine));
 p->drawLine(x1, y1, x2, y2);
 p->setPen(QPen(d->guideColor, d->guideSize, Qt::DotLine));
 p->drawLine(x1, y1, x2, y2);
}

Or something like that (my c++ isn't fresh ;-)

/Simon
Comment 25 Mikolaj Machowski 2009-01-11 15:18:20 UTC
Created attachment 30130 [details]
final (I hope) version of patch for 0.9.x

Final version of patch for 0.9.x, fixes for Simon version:
- remove last unnecessary comments
- fixes white spaces
- swap DM and Harmonious Triangles. Latter use partially control panel below so belon near Golden Mean
Comment 26 Mikolaj Machowski 2009-01-11 15:29:59 UTC
Created attachment 30131 [details]
Adding feature for 0.10
Comment 27 Simon Oosthoek 2009-01-11 16:13:43 UTC
Created attachment 30132 [details]
This patch shows how I meant in the previous comment, to draw the lines

This one would conflict with the last patch from Mikolaj, but I think it would be easy to resolve this. Consider it a proof of concept, but I tested the patch and it works.

/Simon
Comment 28 Mikolaj Machowski 2009-01-16 16:58:03 UTC
I'd like to remind about this one: it introduces new strings and on 18.01 string freeze kicks in.
Comment 29 Simon Oosthoek 2009-01-16 20:07:25 UTC
Mikolaj

what needs to be done?
I think only Gilles needs to approve the patches for 0.9.5 and 0.10.0?

/Simon
Comment 30 caulier.gilles 2009-01-16 20:24:03 UTC
I promise, i take a look soon.

Question : patch exist for KDE3 _and_ KDE4 ?

Gilles
Comment 31 Simon Oosthoek 2009-01-16 20:53:02 UTC
Although I haven't been able to test it (due to not having a working 4.2.x setup) I believe Mikolaj's patch http://bugs.kde.org/attachment.cgi?id=30131 is for the KDE4 version...

/Simon
Comment 32 Mikolaj Machowski 2009-01-16 21:26:26 UTC
Yes, Gilles has to approve and commit patches :)

Yes, there are two patches, one for 0.9.5

http://bugs.kde.org/attachment.cgi?id=30130

and second for 0.10

http://bugs.kde.org/attachment.cgi?id=30131

Written by Simon, adjusted and tested by me (both for KDE3 and KDE4).
Comment 33 caulier.gilles 2009-01-17 18:06:55 UTC
SVN commit 912532 by cgilles:

digiKAm from KDE3 branch: added great patch from Simon Margo about to add new composition guide in Ratio Crop Tool based on Diagonal Rules.
CCBUG: 179548
CCMAIL: simon@margo.student.utwente.nl


 M  +54 -12    imageselectionwidget.cpp  
 M  +2 -1      imageselectionwidget.h  
 M  +52 -39    ratiocroptool.cpp  
 M  +1 -1      ratiocroptool.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=912532
Comment 34 caulier.gilles 2009-01-17 18:48:10 UTC
SVN commit 912554 by cgilles:

backport commit #912532 from KDE3 branch
BUG: 179548
CCMAIL: simon@margo.student.utwente.nl


 M  +76 -33    imageselectionwidget.cpp  
 M  +16 -15    imageselectionwidget.h  
 M  +56 -45    ratiocroptool.cpp  
 M  +2 -2      ratiocroptool.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=912554