Bug 179548

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

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.

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

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/

OK, tested it on paintings (where artists were more concerned with composition) and idea has more merit. 

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;
}

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

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

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

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

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"....

Mik,

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

Thanks in advance

Gilles

Against which svn version is this patch? With 907203 it is not applicable.

This is against the kde3 version in svn...

It compiles fine (and seems to work correct) under KDE3...

Andi

Truly weird, it patches, it compiles, but I don't see it in interface...

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

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 :)

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

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.

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

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.

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

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

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

Created attachment 30131 [details]
Adding feature for 0.10

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

I'd like to remind about this one: it introduces new strings and on 18.01 string freeze kicks in.

Mikolaj

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

/Simon

I promise, i take a look soon.

Question : patch exist for KDE3 _and_ KDE4 ?

Gilles

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

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).

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

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