Bug 145159 - improvements to the lightable
Summary: improvements to the lightable
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: LightTable-Engine (show other bugs)
Version: unspecified
Platform: Debian stable Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-07 21:35 UTC by Arnd Baecker
Modified: 2012-09-03 07:00 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 0.9.2


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Baecker 2007-05-07 21:35:41 UTC
Version:            (using KDE KDE 3.5.5)
Installed from:    Debian stable Packages
OS:                Linux

The new lighttable is very nice, but 
there are two important features which are, in my opinion, needed
to make it really useable:

- the light-table has to work with non-scaled images
  (i.e. like the image editor); otherwise
  a control of the quality is not possible.
  Presently a reduced-size version of the image is used.
- joint zooming and joint panning is essential
  (as discussed in http://bugs.kde.org/show_bug.cgi?id=135048,  #50, 
   for images not identical in size
   this should just be disabled;
   maybe there is a way to deal with this, but this
   really looks like a rare situation)

Less important than the previous two:
- In addition, even for similar pictures, the
  subject of interest might not be at the same place.
  For this the possibility to create an additional shift
  (while still keeping the linked panning and zooming) would
  be helpful. This could be achieved by declaring
  the left image as master and the right one just follows
  any zoom and pan.
  If one pans in the right image, only this one is
  affected. If one then pans in the left one,
  the right one follows, as before, but with
  the additional shift
  (there might be some issues near the borders, but
   one thing after another ;-).
Comment 1 caulier.gilles 2007-05-08 16:29:46 UTC
Arnd,

>- the light-table has to work with non-scaled images
>  (i.e. like the image editor); otherwise
>  a control of the quality is not possible.
>  Presently a reduced-size version of the image is used. 

Yes, and for performance reasons, this cannot be changed, especially with RAW files. Use Editor instead. If you want to toggle to editor from Light Table, use the preview panel pop-up menu.

I think this is a non-sense to use the full image in Light Table. This tool will be used to judge images and delete the wrong shots.

>- joint zooming and joint panning is essential
>  (as discussed in http://bugs.kde.org/show_bug.cgi?id=135048,  #50,
>   for images not identical in size
>   this should just be disabled;
>   maybe there is a way to deal with this, but this
>  really looks like a rare situation) 

Patch is ready on my computer...

Gilles
Comment 2 Arnd Baecker 2007-05-08 16:54:28 UTC
> >  Presently a reduced-size version of the image is used.
> Yes, and for performance reasons, this cannot be changed,


I don't know how slow it can get;
However, the image editor, which does offer the full view,
works really fast.

> especially with RAW files.


Maybe it makes sens to offer full-size version as an option?


Is there any place in the code, where I could easily modify
the settings (i.e. default sizes)
just to try it out, how slow it could get?

> Use Editor instead.


But this does not allow to compare images! ;-)

> I think this is a non-sense to use the full image in Light
> Table. This tool will be used to judge images and delete the wrong
> shots.


But I am only able to judge about the quality
(sharpness, ...) if they can be seen in in comparison in 100% view, without
any interpolation.

Best, Arnd
Comment 3 caulier.gilles 2007-05-08 18:57:11 UTC
Arnd,

Fir this one:

>- the light-table has to work with non-scaled images
>  (i.e. like the image editor); otherwise
>  a control of the quality is not possible.
>  Presently a reduced-size version of the image is used. 

Please open a new file about this subject in B.K.O. 

Also, please, do not mix major wishes in the same B.K.O file, else it very difficult to manage (from a developer view (:=)).

... and use the right B.K.O/digiKam/component to post a new file (:=))). We have a "Light Table" component in  B.K.O. It's a waste of time for me to redirect/reorganize the files at the right place.

For others points of this B.K.O file, i will commit a patch to close it.

Thanks in advance for your help.

Gilles
Comment 4 caulier.gilles 2007-05-08 19:00:26 UTC
SVN commit 662577 by cgilles:

digikam from trunk : Light Table improvement : add option to synchronize left and right panels when user pan/zoom preview images.
This option is only enable when left and right preview image have the same size.

BUG: 145159



 M  +2 -1      NEWS  
 M  +3 -5      digikam/imagepreviewview.cpp  
 M  +1 -1      digikam/imagepreviewview.h  
 M  +9 -4      libs/widgets/common/previewwidget.cpp  
 M  +1 -0      libs/widgets/common/previewwidget.h  
 M  +27 -9     utilities/lighttable/lighttablepreview.cpp  
 M  +4 -1      utilities/lighttable/lighttablepreview.h  
 M  +51 -0     utilities/lighttable/lighttableview.cpp  
 M  +9 -0      utilities/lighttable/lighttableview.h  
 M  +25 -0     utilities/lighttable/lighttablewindow.cpp  
 M  +2 -0      utilities/lighttable/lighttablewindow.h  
 M  +5 -4      utilities/lighttable/lighttablewindowui.rc  


--- trunk/extragear/graphics/digikam/NEWS #662576:662577
@@ -9,8 +9,9 @@
 digiKam BUGFIX FROM KDE BUGZILLA (alias B.K.O | http://bugs.kde.org):
 
 040 ==> 135048 : Easy comparing similar pictures using a lighttable.
+041 ==> 145159 : Improvements to the lightable.
+042 ==>
 
-
 **********************************************************************************************
 
 
--- trunk/extragear/graphics/digikam/digikam/imagepreviewview.cpp #662576:662577
@@ -149,9 +149,6 @@
 
     // ------------------------------------------------------------
 
-    connect(this, SIGNAL(signalZoomFactorChanged(double)),
-            this, SLOT(slotZoomChanged(double)));
-
     connect(d->cornerButton, SIGNAL(pressed()),
             this, SLOT(slotCornerButtonPressed()));
 
@@ -590,12 +587,14 @@
     }
 }
 
-void ImagePreviewView::slotZoomChanged(double zoom)
+void ImagePreviewView::zoomFactorChanged(double zoom)
 {
     if (zoom > calcAutoZoomFactor())
         d->cornerButton->show();
     else
         d->cornerButton->hide();        
+
+    PreviewWidget::zoomFactorChanged(zoom);
 }
 
 void ImagePreviewView::resizeEvent(QResizeEvent* e)
@@ -611,7 +610,6 @@
     QScrollView::resizeEvent(e);
 
     updateZoomAndSize(false);
-    //emit signalZoomFactorChanged(zoomFactor());
 }
 
 void ImagePreviewView::updateZoomAndSize(bool alwaysFitToWindow)
--- trunk/extragear/graphics/digikam/digikam/imagepreviewview.h #662576:662577
@@ -89,7 +89,6 @@
     void slotAssignRating(int rating);
     void slotThemeChanged();
     void slotCornerButtonPressed();
-    void slotZoomChanged(double);
     void slotPanIconSelectionMoved(QRect, bool);
     void slotPanIconHiden();
 
@@ -99,6 +98,7 @@
     int  previewHeight();
     bool previewIsNull();
     void resetPreview();
+    void zoomFactorChanged(double zoom);
     void updateZoomAndSize(bool alwaysFitToWindow);
     inline void paintPreview(QPixmap *pix, int sx, int sy, int sw, int sh);
 
--- trunk/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp #662576:662577
@@ -225,7 +225,7 @@
     viewport()->setUpdatesEnabled(true);
     viewport()->update();
 
-    emit signalZoomFactorChanged(d->zoom);
+    zoomFactorChanged(d->zoom);
 }
 
 double PreviewWidget::zoomFactor()
@@ -254,7 +254,7 @@
     else
     {
         d->zoom = 1.0;
-        emit signalZoomFactorChanged(d->zoom);
+        zoomFactorChanged(d->zoom);
     }
 
     updateContentsSize();
@@ -267,7 +267,7 @@
     d->zoomWidth  = (int)(previewWidth()  * d->zoom);
     d->zoomHeight = (int)(previewHeight() * d->zoom);
 
-    emit signalZoomFactorChanged(d->zoom);
+    zoomFactorChanged(d->zoom);
 }
 
 double PreviewWidget::calcAutoZoomFactor(AutoZoomMode mode)
@@ -337,7 +337,7 @@
 
     // To be sure than corner widget used to pan image will be hide/show 
     // accordinly with resize event.
-    emit signalZoomFactorChanged(d->zoom);
+    zoomFactorChanged(d->zoom);
 }
 
 void PreviewWidget::viewportPaintEvent(QPaintEvent *e)
@@ -508,4 +508,9 @@
     QScrollView::contentsWheelEvent(e);
 }
 
+void PreviewWidget::zoomFactorChanged(double zoom)
+{
+    emit signalZoomFactorChanged(zoom);
+}
+
 }  // NameSpace Digikam
--- trunk/extragear/graphics/digikam/libs/widgets/common/previewwidget.h #662576:662577
@@ -113,6 +113,7 @@
     virtual bool previewIsNull()=0;
     virtual void resetPreview()=0;
     virtual void paintPreview(QPixmap *pix, int sx, int sy, int sw, int sh)=0;
+    virtual void zoomFactorChanged(double zoom);
    
 private:
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablepreview.cpp #662576:662577
@@ -45,6 +45,7 @@
 #include <kdatetbl.h>
 #include <kiconloader.h>
 #include <kprocess.h>
+#include <kapplication.h>
 
 // LibKipi includes.
 
@@ -89,11 +90,16 @@
         hasPrev              = false;
         hasNext              = false;
         currentFitWindowZoom = 0;
+        previewSize          = 1024;
     }
 
     bool               hasPrev;
     bool               hasNext;
 
+    int                previewSize;
+
+    double             currentFitWindowZoom;
+
     QString            path;
     QString            nextPath;
     QString            previousPath;
@@ -106,8 +112,6 @@
 
     PanIconWidget     *panIconWidget;
 
-    double             currentFitWindowZoom;
-
     ImageInfo         *imageInfo;
 
     PreviewLoadThread *previewThread;
@@ -118,6 +122,14 @@
                  : PreviewWidget(parent)
 {
     d = new LightTablePreviewPriv;
+
+    // get preview size from screen size, but limit from VGA to WQXGA
+    d->previewSize = QMAX(KApplication::desktop()->height(),
+                          KApplication::desktop()->width());
+    if (d->previewSize < 640)
+        d->previewSize = 640;
+    if (d->previewSize > 2560)
+        d->previewSize = 2560;
     
     viewport()->setAcceptDrops(true);
     setAcceptDrops(true); 
@@ -136,9 +148,6 @@
 
     // ------------------------------------------------------------
 
-    connect(this, SIGNAL(signalZoomFactorChanged(double)),
-            this, SLOT(slotZoomChanged(double)));
-
     connect(d->cornerButton, SIGNAL(pressed()),
             this, SLOT(slotCornerButtonPressed()));
 
@@ -171,6 +180,11 @@
     return d->preview;
 }
 
+QSize LightTablePreview::getImageSize()
+{
+    return d->preview.size();
+}
+
 void LightTablePreview::reload()
 {
     // cache is cleaned from AlbumIconView::refreshItems
@@ -211,7 +225,7 @@
                 this, SLOT(slotNextPreload()));
     }
 
-    d->previewThread->load(LoadingDescription(path, 1024, AlbumSettings::instance()->getExifRotate()));
+    d->previewThread->load(LoadingDescription(path, d->previewSize, AlbumSettings::instance()->getExifRotate()));
 }
 
 void LightTablePreview::slotGotImagePreview(const LoadingDescription &description, const QImage& preview)
@@ -240,6 +254,8 @@
 
     unsetCursor();
     slotNextPreload();
+
+    emit signalPreviewLoaded();
 }
 
 void LightTablePreview::slotNextPreload()
@@ -258,7 +274,7 @@
     else
         return;
 
-    d->previewPreloadThread->load(LoadingDescription(loadPath, 1024,
+    d->previewPreloadThread->load(LoadingDescription(loadPath, d->previewSize,
                                   AlbumSettings::instance()->getExifRotate()));
 }
 
@@ -552,12 +568,14 @@
     }
 }
 
-void LightTablePreview::slotZoomChanged(double zoom)
+void LightTablePreview::zoomFactorChanged(double zoom)
 {
-    if (zoom > calcAutoZoomFactor() && !previewIsNull())
+    if (zoom > calcAutoZoomFactor())
         d->cornerButton->show();
     else
         d->cornerButton->hide();        
+
+    PreviewWidget::zoomFactorChanged(zoom);
 }
 
 void LightTablePreview::resizeEvent(QResizeEvent* e)
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablepreview.h #662576:662577
@@ -28,6 +28,7 @@
 
 #include <qstring.h>
 #include <qimage.h>
+#include <qsize.h>
 
 // Local includes
 
@@ -56,6 +57,8 @@
     void setImage(const QImage& image);
     QImage& getImage() const;
 
+    QSize getImageSize();
+
     void setImageInfo(ImageInfo* info=0, ImageInfo *previous=0, ImageInfo *next=0);
     ImageInfo* getImageInfo() const;
 
@@ -85,7 +88,6 @@
     void slotAssignRating(int rating);
     void slotThemeChanged();
     void slotCornerButtonPressed();
-    void slotZoomChanged(double);
     void slotPanIconSelectionMoved(QRect, bool);
     void slotPanIconHiden();
 
@@ -95,6 +97,7 @@
     int  previewHeight();
     bool previewIsNull();
     void resetPreview();
+    void zoomFactorChanged(double zoom);
     void updateZoomAndSize(bool alwaysFitToWindow);
     inline void paintPreview(QPixmap *pix, int sx, int sy, int sw, int sh);
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttableview.cpp #662576:662577
@@ -46,11 +46,14 @@
 
     LightTableViewPriv()
     {
+        syncPreview  = false;
         leftPreview  = 0;
         rightPreview = 0;
         grid         = 0;
     }
 
+    bool               syncPreview;
+
     QGridLayout       *grid;
 
     LightTablePreview *leftPreview;
@@ -107,6 +110,17 @@
     connect(d->rightPreview, SIGNAL(signalSlideShow()),
             this, SIGNAL(signalSlideShow()));
 
+    connect(d->leftPreview, SIGNAL(contentsMoving(int, int)),
+            this, SLOT(slotLeftContentsMoved(int, int)));
+
+    connect(d->rightPreview, SIGNAL(contentsMoving(int, int)),
+            this, SLOT(slotRightContentsMoved(int, int)));
+
+    connect(d->leftPreview, SIGNAL(signalPreviewLoaded()),
+            this, SLOT(slotPreviewLoaded()));
+
+    connect(d->rightPreview, SIGNAL(signalPreviewLoaded()),
+            this, SLOT(slotPreviewLoaded()));
 }
 
 LightTableView::~LightTableView()
@@ -114,6 +128,11 @@
     delete d;
 }
 
+void LightTableView::setSyncPreview(bool sync)
+{
+    d->syncPreview = sync;
+}
+
 void LightTableView::setLeftImageInfo(ImageInfo* info)
 {
     d->leftPreview->setImageInfo(info);    
@@ -246,5 +265,37 @@
     d->rightPreview->reload();
 }
 
+void LightTableView::slotLeftContentsMoved(int x, int y)
+{
+    if (d->syncPreview)
+    {
+        d->rightPreview->blockSignals(true);
+        setRightZoomFactor(d->leftPreview->zoomFactor());
+        emit signalRightZoomFactorChanged(d->leftPreview->zoomFactor());
+        d->rightPreview->setContentsPos(x, y);
+        d->rightPreview->blockSignals(false);
+    }
+}
+
+void LightTableView::slotRightContentsMoved(int x, int y)
+{
+    if (d->syncPreview)
+    {
+        d->leftPreview->blockSignals(true);
+        setLeftZoomFactor(d->rightPreview->zoomFactor());
+        emit signalLeftZoomFactorChanged(d->rightPreview->zoomFactor());
+        d->leftPreview->setContentsPos(x, y);
+        d->leftPreview->blockSignals(false);
+    }
+}
+
+void LightTableView::slotPreviewLoaded()
+{
+    if (d->leftPreview->getImageSize() == d->rightPreview->getImageSize())
+        emit signalToggleOnSyncPreview(true); 
+    else
+        emit signalToggleOnSyncPreview(false); 
+}
+
 }  // namespace Digikam
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttableview.h #662576:662577
@@ -50,6 +50,8 @@
     LightTableView(QWidget *parent=0);
     ~LightTableView();
 
+    void   setSyncPreview(bool sync);
+
     void   setLeftImageInfo(ImageInfo* info=0);
     void   setRightImageInfo(ImageInfo* info=0);
 
@@ -85,6 +87,7 @@
     void signalSlideShow();
     void signalDeleteItem(ImageInfo*);
     void signalEditItem(ImageInfo*);
+    void signalToggleOnSyncPreview(bool);
 
 public slots:
 
@@ -96,6 +99,12 @@
     void slotIncreaseRightZoom();
     void slotRightZoomSliderChanged(int);
 
+private slots:
+
+    void slotLeftContentsMoved(int, int);
+    void slotRightContentsMoved(int, int);
+    void slotPreviewLoaded();
+
 private :
 
     LightTableViewPriv* d;    
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablewindow.cpp #662576:662577
@@ -86,6 +86,7 @@
         barView                             = 0;
         hSplitter                           = 0;
         vSplitter                           = 0;
+        syncPreviewAction                   = 0;
         clearListAction                     = 0;
         removeItemAction                    = 0;
         fileDeleteAction                    = 0;
@@ -122,6 +123,7 @@
     KAction                  *zoomFitToWindowAction;
 
     KToggleAction            *fullScreenAction;
+    KToggleAction            *syncPreviewAction;
 
     KAccel                   *accelerators;
 
@@ -319,6 +321,9 @@
 
     connect(d->previewView, SIGNAL(signalRightDroppedItems(const ImageInfoList&)),
            this, SLOT(slotRightDroppedItems(const ImageInfoList&)));
+                                  
+    connect(d->previewView, SIGNAL(signalToggleOnSyncPreview(bool)),
+           this, SLOT(slotToggleOnSynPreview(bool)));
 
     ImageAttributesWatch *watch = ImageAttributesWatch::instance();
 
@@ -344,6 +349,11 @@
 
     // -- Standard 'View' menu actions ---------------------------------------------
 
+    d->syncPreviewAction = new KToggleAction(i18n("Sync Preview"), "goto",
+                                            CTRL+SHIFT+Key_Y, this,
+                                            SLOT(slotToggleSyncPreview()),
+                                            actionCollection(), "lighttable_syncpreview");
+
     d->zoomTo100percents = new KAction(i18n("Zoom to 1:1"), "viewmag1",
                                        CTRL+SHIFT+Key_Z, this, SLOT(slotZoomTo100Percents()),
                                        actionCollection(), "lighttable_zoomto100percents");
@@ -962,5 +972,20 @@
         d->rightZoomBar->setEnableZoomMinus(false);
 }
 
+void LightTableWindow::slotToggleSyncPreview()
+{
+    d->previewView->setSyncPreview(d->syncPreviewAction->isChecked());
+}
+
+void LightTableWindow::slotToggleOnSynPreview(bool t)
+{
+    d->syncPreviewAction->setEnabled(t);
+
+    if (!t)
+    {
+        d->syncPreviewAction->setChecked(false);
+    }
+}
+
 }  // namespace Digikam
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablewindow.h #662576:662577
@@ -85,6 +85,7 @@
 
 private slots:
 
+    void slotToggleSyncPreview();
     void slotEditItem(ImageInfo* info);
     void slotDeleteItem(ImageInfo* info);
     void slotItemSelected(ImageInfo*);
@@ -109,6 +110,7 @@
     void slotRightZoomFactorChanged(double);
     void slotLeftDroppedItems(const ImageInfoList&);
     void slotRightDroppedItems(const ImageInfoList&);
+    void slotToggleOnSynPreview(bool);
 
 private:
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablewindowui.rc #662576:662577
@@ -1,5 +1,5 @@
 <!DOCTYPE kpartgui SYSTEM "kpartgui.dtd">
-<gui version="5" name="lighttablewindow" >
+<gui version="6" name="lighttablewindow" >
 
 <MenuBar>
 
@@ -14,6 +14,8 @@
         <Action name="lighttable_fullscreen" />
         <Action name="lighttable_slideshow" />
         <Separator/>
+        <Action name="lighttable_syncpreview" />
+        <Separator/>
         <Action name="lighttable_zoomto100percents" />
         <Action name="lighttable_zoomfit2window" />
     </Menu>
@@ -37,9 +39,8 @@
 </MenuBar>
 
 <ToolBar name="ToolBar" ><text>Main Toolbar</text>
-     <Action name="lighttable_zoomplus" /> 
-     <Action name="lighttable_zoomto" />
-     <Action name="lighttable_zoomminus" /> 
+     <Action name="lighttable_syncpreview" /> 
+     <Separator/>     
      <Action name="lighttable_zoomfit2window" /> 
      <Action name="lighttable_zoomfit2select" /> 
      <Separator/>     
Comment 5 Arnd Baecker 2007-05-08 20:25:49 UTC
> >- the light-table has to work with non-scaled images
> >  (i.e. like the image editor); otherwise
> >  a control of the quality is not possible.
> >  Presently a reduced-size version of the image is used.
>
>
> Please open a new file about this subject in B.K.O.


Done.

> Also, please, do not mix major wishes in the same B.K.O file,
> else it very difficult to manage (from a developer view (:=)).


Fair enough! ;-)

> ... and use the right B.K.O/digiKam/component to post a new file (:=))). We have a "Light Table" component in  B.K.O. It's a waste of time for me to redirect/reorganize the files at the right place.


I looked hard, but I could not find that place last time
(I don't think that I will ever like
that B.K.O., too many steps and overly complicated)
- Now I know why: in the box, in which you can select the components,
there is a scroll bar, but this one is approx 300 pixels beyond
the edge of my desktop, presumably because of this extremely long
line on the digikamimageplugins (could this maybe be shortened?).

> For others points of this B.K.O file, i will commit a patch to close it.


Currently I get a compile error:

.libs/imagepreviewview.o: In function
`Digikam::ImagePreviewView::zoomFactorChanged(double)':
imagepreviewview.cpp:(.text+0x347): undefined reference to
`Digikam::PreviewWidget::zoomFactorChanged(double)'
../../digikam/libs/widgets/.libs/libwidgets.a(imageregionwidget.o):(.data.rel.ro._ZTVN7Digikam17ImageRegionWidgetE[vtable
for Digikam::ImageRegionWidget]+0x28c): undefined reference to
`Digikam::PreviewWidget::zoomFactorChanged(double)'
../../digikam/libs/widgets/.libs/libwidgets.a(previewwidget.o):(.data.rel.ro._ZTVN7Digikam13PreviewWidgetE[vtable
for Digikam::PreviewWidget]+0x28c): undefined reference to
`Digikam::PreviewWidget::zoomFactorChanged(double)'
../../digikam/utilities/lighttable/.libs/liblighttable.a(lighttablepreview.o):
In function `Digikam::LightTablePreview::zoomFactorChanged(double)':
lighttablepreview.cpp:(.text+0x377): undefined reference to
`Digikam::PreviewWidget::zoomFactorChanged(double)'
collect2: ld returned 1 exit status
make[2]: *** [libdigikam.la] Error 1

I did a ``make clean`` before, but maybe that was not enough ...

> Thanks in advance for your help.


Well it is really the other-way-round: *thanks* for all  your work!!!!

Arnd
Comment 6 caulier.gilles 2007-05-08 20:41:58 UTC
Hum, checkout svn again and perform "make distclean". Restart "make -f Makefile.cvs ; ./configure...; make" again from root source folder. Normally, all will be fine.

Gilles
Comment 7 Arnd Baecker 2007-05-08 21:57:01 UTC
Also tried this, without success. Only a completely new check-out
lead to a compile which worked....

It works really really nice - many thanks!!
(there are some small glitches, which I will post in a separate B.K.O)