Created attachment 49377 [details] complete new ui to the bwsepiafilter Version: 1.4.0 (using KDE 4.3.3) OS: Linux I do a lot of black and white. Digikam is great at it, with good presets, but most of the time, I feel somewhat limited in what I really want to achieve. This led me to look at the code, but I'm not really a programmer, and to change the way the b&w filter works, a lot of coding is obviously needed. On the other hand, while I looked at the code, I saw that the UI was more immediately accessible to some coding, to improve the ergonomics and consistency with newer filters using some widgets written specifically for digikam (the RExpanderBox). In the process, I noticed the histogram was somehow eating up valuable screen space, and I tweaked the luminosity curvebox to update in real time to reflect the preview histogram in its background. I also changed the previewlist widget to display as a box of icons instead of a list, again to rationalize the ui use of available space, but without cluttering the screen with too small previews. Screenshot of my mods : http://picasaweb.google.com/lh/photo/gTK5ER0guRk9czU6p1xg2g?feat=directlink I attach a patch of my attempts. This compiles against the 1.4 svn version of digikam. I did to the best of my abilities, which doesn't means much I'm afraid, and I'd really love to hear comments and suggestions to improve what I did. I can provide a diff only for the modified previewlist widget on request. Reproducible: Didn't try
Created attachment 49432 [details] this version needs to alter the libkdcraw rexpandebox widget with the other patch below. Usability improvement. The RExpanderBox widget gains a toolbox mode, where only one included RExpander is shown at any given time.
Created attachment 49433 [details] Adds an exclusive mode to RExpander. this is a patch to libkdcraw (needs to recompile libkdcraw, kipi-plugins and digikam for effect). adds a toolbox mode to RExpanderBox widget.
Created attachment 49468 [details] rexpanderbox based ui to bw filter, only digikam modified I wasn't happy with my changes to libkdcraw and considered my changes to rexpanderbox too far reaching. So I thought this over and finally opted to limit my scope to digikam itself. I therefore created a subclass of rexpander named rexpanderexclusive (in libs/widgets/common) that belongs to the digikam namespace and basically adds a toolbox mode to the base widget. I added my subclass to the necessary CMakeLists. The RExpanderBoxExclusive class is just a toolbox-mode RExpanderBox by default, and can be reverted to a classic multi-pane behaviour at the flick of a switch with the provided setIsToolBox(bool) method.
Manuel, "The dynamic curvesbox replaces the non editable histogram at the top of the toolbox" No. the original histogram view is not static. It represent the modified image histogram. It used to check if change provide an under/over exposed picture. Please restore it. Gilles Caulier
(In reply to comment #4) > Manuel, > > "The dynamic curvesbox replaces the non editable histogram at the top of the > toolbox" > > No. the original histogram view is not static. It represent the modified image > histogram. It used to check if change provide an under/over exposed picture. Yes, I understood this first time we talked. That's the reason why I added a method Curvebox::updatedData() to refresh the histogram in the background of the curve to reflect the modified image histogram, so you know if your actions are going to over/underexpose the modified image. The data I'm not showing is the original image static histogram now. > Please restore it. No problem, I do it. Manuel
Created attachment 49490 [details] bwsepiafiltertool simple UI transformation from KTabWidget to RExpanderBox following on cg request. Cheers, Manuel
Some feedback after to test : 1/ in iconview, with B&W preview icons, if you select one item to render preview, you is unable to compare preview together, because selected one is grayed by selection mode. If fact a selected item must be grayed around the thumbnail (text + border), but not the image as well. 2/ your expander box running in exclusive mode is very interesting. I will add it to libkdcraw later KDE 4.5.0 release (it will still back in digiKam core for a while, to not break binary compatibility). But there is a little problem there : if you unexpand current item, the vertical layout is broken, an a lots of space is added between all expander items. Sound like a stretch is not added to the bottom of layout to move all items to the top. 3/ i don't like to see the same icon for all expander item. please set different image for each one. It will set it more suitable for end users. look icons installed from digiKam core to your computer Gilles Caulier
Created attachment 49552 [details] corrected following cg inputs. Thank you very much for the feedback, this is very encouraging. > 1/ in iconview, with B&W preview icons, if you select one item to render > preview, you is unable to compare preview together, because selected one is > grayed by selection mode. If fact a selected item must be grayed around the > thumbnail (text + border), but not the image as well. Done. A combination of stylesheet and Qicon role got this one. > 2/ your expander box running in exclusive mode is very interesting. I will add > it to libkdcraw later KDE 4.5.0 release (it will still back in digiKam core for > a while, to not break binary compatibility). But there is a little problem > there : if you unexpand current item, the vertical layout is broken, an a lots > of space is added between all expander items. Sound like a stretch is not added > to the bottom of layout to move all items to the top. Yes but the stretch conflict with the previewlist and I could find no meaningful way to address this concern because until this mode can be backported to libkdcraw, the layout of the parent RExpanderBox is opaque. I mitigated the issue by hardcoding a larger initial size for previewlist. It's not as clean as I wish, but I don't know how to do better. > 3/ i don't like to see the same icon for all expander item. please set > different image for each one. It will set it more suitable for end users. look > icons installed from digiKam core to your computer Done. I'm not a graphic artist, so I chose some icons I thought more or less fitting, new icons would be a better solution, but it does look better. Cheers, Manuel
Done. Excelent ! Just a little remark : in preview icon view, when you start tool, it do not select the last used settings. In fact, tool apply it, but nothing is selected to icon view. For icons, i will review it later, after to have applied your patch on svn trunk (digiKam 1.4.0) Also, for digiKam 2.0.0, we use a separate branch, where i will fork from trunk libkdcraw, and integrate your expander patch directly. For information, code is there. Warnings, it's unstable for the moment, do not use in production. It include all GSoC 2010 works, as Face detection, Image versioning, and reverse geolocation : http://websvn.kde.org/branches/extragear/graphics/ I hope that you will interested to patch more digiKam in the future. Your implementation is very suitable, very well written, and easily reviewable. Gilles Caulier
(In reply to comment #9) > Just a little remark : in preview icon view, when you start tool, it do not > select the last used settings. In fact, tool apply it, but nothing is selected > to icon view. This most strange, and I'm completely lost here, because on my working copy the behaviour is correct, and the preview icon is shown selected. (QT 4.5.3 & KDE 4.3.3 with updated kdegraphics.) > For icons, i will review it later, after to have applied your patch on svn > trunk (digiKam 1.4.0) Thank you. > Also, for digiKam 2.0.0, we use a separate branch, where i will fork from trunk > libkdcraw, and integrate your expander patch directly. That would be much more convenient. I tried to settle the geometry interactions between my derived instance and the displayed objects from the outside, but I realised I would need to worm a way through the QObject tree to get inside the RExpanderBox main layout. Very bad (and bug-prone) practice, I scrapped my attempts. > For information, code is there. Warnings, it's unstable for the moment, do not > use in production. It include all GSoC 2010 works, as Face detection, Image > versioning, and reverse geolocation : > > http://websvn.kde.org/branches/extragear/graphics/ I'll sure peek a look as soon as I get back in mid-August. > I hope that you will interested to patch more digiKam in the future. Your > implementation is very suitable, very well written, and easily reviewable. Well this is much undeserved praise, and I hope I can find new areas where polishing efforts are needed :-) Cheers, Manuel
>> Just a little remark : in preview icon view, when you start tool, it do not >> select the last used settings. In fact, tool apply it, but nothing is selected >> to icon view. >This most strange, and I'm completely lost here, because on my working copy the >behaviour is correct, and the preview icon is shown selected. (QT 4.5.3 & KDE >4.3.3 with updated kdegraphics.) I will be more precise : i i don't apply yet B&W conversion using new setting layout (using preview as icon view), the first item from icon view is used to render preview effect on canvas (this is correct), but the icon view item is not selected. I don't have tested the case of B&W tool have been already used before. Gilles
(In reply to comment #11) > I will be more precise : i i don't apply yet B&W conversion using new setting > layout (using preview as icon view), the first item from icon view is used to > render preview effect on canvas (this is correct), but the icon view item is > not selected. I understand. I can't look at it now, but I let you know as soon as I can. Manuel
Manual, I can confirm that preview icon selection is not restored between tool session (in all case) Gilles
(In reply to comment #12) > (In reply to comment #11) > > I can confirm that preview icon selection is not restored between tool session > (in all case) That's most annoying. I can't duplicate it here. I strongly suspect a behaviour change in QListWidget in Icon Mode between Qt 4.5 and Qt 4.6. I need to setup an up-to-date new development box before I can tackle this one, at the moment, I'm blind. Manuel
Here i use Qt 4.6.2 from Mandriva 2010.1 open source edition : digiKam version 1.4.0 (rev.: 1152961) Exiv2 can write to Jp2: Yes Exiv2 can write to Jpeg: Yes Exiv2 can write to Pgf: Yes Exiv2 can write to Png: Yes Exiv2 can write to Tiff: Yes Exiv2 supports XMP metadata: Yes LibCImg: 130 LibExiv2: 0.20 LibJPEG: 80 LibJasper: 1.900.1 LibKDE: 4.4.3 (KDE 4.4.3) LibKExiv2: 1.2.0 LibKdcraw: 1.1.0 LibLCMS: 119 LibPGF: 6.09.44 LibPNG: 1.2.43 LibQt: 4.6.2 LibRaw: 0.10.0-Beta2 LibTIFF: LIBTIFF, Version 3.9.2 Copyright (c) 1988-1996 Sam Leffler Copyright (c) 1991-1996 Silicon Graphics, Inc. Marble widget: 0.9.3 Parallelized demosaicing: Yes Database backend: QSQLITE LibGphoto2: 2.4.9 LibKipi: 1.2.0
(In reply to comment #15) Looks like I'm not up to date on several counts :-) digiKam version 1.4.0 (rev.: 1154595) Dématriçage parallélisé: Oui Exiv2 peut écrire dans un fichier JP2: Oui Exiv2 peut écrire dans un fichier JPEG: Oui Exiv2 peut écrire dans un fichier PGF: Oui Exiv2 peut écrire dans un fichier PNG: Oui Exiv2 peut écrire dans un fichier TIFF: Oui Exiv2 prend en charge les métadonnées XMP: Oui LibCImg: 130 LibExiv2: 0.20 LibJPEG: 62 LibJasper: 1.900.1 LibKDE: 4.3.3 (KDE 4.3.3) LibKExiv2: 1.1.0 LibKdcraw: 1.1.0 LibLCMS: 118 LibPGF: 6.09.44 LibPNG: 1.2.37 LibQt: 4.5.3 LibRaw: 0.10.0-Beta2 LibTIFF: LIBTIFF, Version 3.8.2 Copyright (c) 1988-1996 Sam Leffler Copyright (c) 1991-1996 Silicon Graphics, Inc. Élément graphique Marble: 0.8.1 LibGphoto2: 2.4.3 LibKipi: 1.1.0 Moteur de base de données: QSQLITE
Created attachment 49629 [details] final bug squashing > Manual, > > I can confirm that preview icon selection is not restored between tool session > (in all case) > > Gilles I finally made a quick and crude Qt 4.6.3 install, enough to confirm this bug. It's a Qt 4.x bug. In 4.5, the calculated icons are not displayed properly over the text in case this text is translated until the QListWidget is reset(), and 4.6 is even further broken, because QListWidget::reset() doesn't restore the selection anymore like it used to previously. The attached patch works around this (see utilities/imageeditor/widgets/previewlist.cpp in void PreviewList::slotProgressTimerDone() at line 325). Cheers, Manuel
Just few remarks with rexpanderboxexclusive implementation : - no need to include rexpanderboxexclusive.moc and rexpanderboxexclusive.h. .moc file is enough. - with private members, when no d private container is used, add a "m_" prefix. - when it's possible set value returned by methods as const (ex: bool isToolBox() const;) For more details about coding style, take a look in digiKam HACKING file. Gilles Caulier
SVN commit 1156631 by cgilles: apply patch #49629 from Manuel Viet to improve ergonomics of BW converter BUGS: 245380 M +1 -0 CMakeLists.txt M +6 -6 imageplugins/color/autocorrectiontool.cpp M +1 -1 imageplugins/color/autocorrectiontool.h M +50 -43 libs/dimg/filters/bw/bwsepiasettings.cpp M +1 -1 libs/dimg/filters/bw/bwsepiasettings.h M +1 -0 libs/widgets/common/CMakeLists.txt M +53 -28 utilities/imageeditor/widgets/previewlist.cpp M +12 -14 utilities/imageeditor/widgets/previewlist.h WebSVN link: http://websvn.kde.org/?view=rev&revision=1156631
SVN commit 1156638 by cgilles: contrast adjustement have been dropped from the layout. Restore it in gui... CCBUGS: 245380 M +6 -7 bwsepiasettings.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1156638
Manuel, We have a little problem with B&W converter settings and icon-view used to host preview. This widget is used into Batch Queue Manager, and it do not render any preview, because we process more than one image, not only just one as in editor. Before, when we have used a simple listview, settings been fine. Now it's full of empty space. It's very annoying (:=))) The solution will to set a dummy image in Batch Queue Manager (as a photo colors chart for ex.) Also, i think that space between icon is very large and can be optimized. Look space between icon item title and top of next icon on the bottom. What do you think about ? Gilles Caulier
Created attachment 49666 [details] BQM and B&W converter settings screen-shot
Created attachment 49681 [details] first try to patch icon view in BQM Manuel, This patch is supposed to render a default image icon in icon view with B&W tool is assigned to BQM. But it crash. Please take a look. Perhaps i'm in a wrong way to fix it. Gilles Caulier
Manuel, Do you take a look to my patch ? Gilles
SVN commit 1162180 by cgilles: - B&W converter for BQM : Add preview using default image preview from Oxygen. Fix crash due to a non initialized pointer. Please, always set a pointer to zero : in GDB, you will see an use of null pointer which is wrong of course. An unitialized pointer as a dumy which is impossible to identify as wrong quickly... - Add a new kde notifier event when editor as saved a file. This is usefull when you review RAW files in icon view and you edit item in editor. During a save operation, which can take a while, you can go back to icon view to continue to sort items before to perform RAW import. The notifier event will bring you when editor is ready to load a new item. CCBUGS: 245380 M +7 -0 digikam/digikam.notifyrc M +37 -33 libs/dimg/filters/bw/bwsepiasettings.cpp M +9 -0 showfoto/showfoto.cpp U utilities/imageeditor/editor/editorwindow.h M +9 -0 utilities/imageeditor/editor/imagewindow.cpp M +2 -1 utilities/imageeditor/widgets/previewlist.cpp M +2 -0 utilities/queuemanager/basetools/color/bwconvert.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=1162180
Oups. I'm just back from 2 weeks of holidays without internet access and I discover your messages. I'll look into this asap. Cheers, Manuel