Bug 280082

Summary: Light Table doesn't change right image properly
Product: [Applications] digikam Reporter: Lukas Jirkovsky <l.jirkovsky>
Component: LightTable-WorkflowAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: caulier.gilles
Priority: NOR    
Version: 2.0.0   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In: 2.2.0
Sentry Crash Report:

Description Lukas Jirkovsky 2011-08-14 17:22:05 UTC
Version:           2.0.0 (using KDE 4.7.0) 
OS:                Linux

When I open the light table and I have "By Pair" enabled, selecting the image shows the image in the left panel, but the right still reads "Drag and drop an image here." If I deselect the "By Pair" option the image is immediately opened in the right panel.

When I enable "By Pair" again and try to change the image, only the left panel changes, the right still shows the same image.

The arrows for moving through images (First/Back/Forward/Last) show the same behavior, ie. only the left image is changed.

Changing the right image using the "On right" button when the "By Pair" option is disabled works.

Reproducible: Didn't try

Steps to Reproduce:
Open Light table and try to switch between images with "By Pair" option enabled

Actual Results:  
Only the left image changes.

Expected Results:  
The behavior from digikam 1.9 – left and right image automatically switch.

Digikam version:
2.0.0 from Arch Linux packages and digikam from today's git (I think it's the revision 2b58f146)

It used to work with digikam 1.9
Comment 1 Lukas Jirkovsky 2011-09-02 17:35:05 UTC
My guess is that it has been introduced by the commit c351b5a0, but because I could find sources of libkmap which is needed to build such old revision I can't confirm it.
Comment 2 Marcel Wiesweg 2011-09-04 09:35:32 UTC
Git commit 4ead2a8a5181521f95388b4d005fb69c8a17f689 by Marcel Wiesweg.
Committed on 04/09/2011 at 11:34.
Pushed by mwiesweg into branch 'master'.

Fix "Navigate by pair" on lighttable at least mostly

CCBUG: 280082

M  +1    -1    utilities/lighttable/lighttablewindow.cpp

http://commits.kde.org/digikam/4ead2a8a5181521f95388b4d005fb69c8a17f689
Comment 3 Marcel Wiesweg 2011-09-04 09:41:32 UTC
I'm still not sure if the lightable's behavior is correct in all corner cases (for the problem reported here, and in a more general view)
But I dont use it frequently so I'm not sure what is desired behavior and what may be annoying or is to be improved.
Comment 4 Lukas Jirkovsky 2011-09-04 18:15:41 UTC
Marcel: thank you, it works much better. both previews are correctly updated.

However The Back/Forward buttons still doesn't work as expected. It seems to jump "randomly" between the images. I'll investigate the behavior further.
Comment 5 Lukas Jirkovsky 2011-09-05 13:12:28 UTC
Here's are the differences in behavior of first/forward/back/last between digikam 1.9 and 2.0.

by pair is disabled
-------------------
1.9: these actions change the image shown in the right panel.
2.0: first/last highlights the first/last image, but the image is not selected (ie. the image is not changed). Back always highlights the image on the left from the image which is selected to be on right. Again the image is not changed. Forward highlights the image right from the image which is selected to be on right panel. Also back/forward moves the highlight only once.

by pair is enabled
------------------
1.9: first selects the first pair (first image is on left, second image is on right panel). Last shows the last image on the left panel and the first image on the right panel. Back/Forward moves the pair one image at a time. Eg. when we have images A, B, C, D forward cycles through them so the (A,B), (B,C), (C,D), (D,A) are shown
2.0: The behavior is almost the same as with By Pair disabled, but Back moves highlight to the image left from the image shown on left panel and forward always highlights the image which is selected to be on the right panel.
Comment 6 caulier.gilles 2011-09-16 11:25:59 UTC
Git commit 56030d3ba09dd60aaed2056bd5c3b63a5fb8a13b by Gilles Caulier.
Committed on 16/09/2011 at 13:14.
Pushed by cgilles into branch 'master'.

use thumbbar item simple click to load image in LT right panel

M  +1    -0    utilities/lighttable/lighttablethumbbar.cpp
M  +1    -1    utilities/lighttable/lighttablewindow.cpp

http://commits.kde.org/digikam/56030d3ba09dd60aaed2056bd5c3b63a5fb8a13b

diff --git a/utilities/lighttable/lighttablethumbbar.cpp b/utilities/lighttable/lighttablethumbbar.cpp
index f7a987e..8ea5cd5 100644
--- a/utilities/lighttable/lighttablethumbbar.cpp
+++ b/utilities/lighttable/lighttablethumbbar.cpp
@@ -87,6 +87,7 @@ LightTableThumbBar::LightTableThumbBar(QWidget* parent)
    d->imageFilterModel->setCategorizationMode(ImageSortSettings::NoCategories);
    d->imageFilterModel->setSortRole((ImageSortSettings::SortRole)AlbumSettings::instance()->getImageSortOrder());
    d->imageFilterModel->setSortOrder((ImageSortSettings::SortOrder)AlbumSettings::instance()->getImageSorting());
+    d->imageFilterModel->sort(0); // an initial sorting is necessary

    d->dragDropHandler = new ImageDragDropHandler(d->imageInfoModel);
    d->dragDropHandler->setReadOnlyDrop(true);
diff --git a/utilities/lighttable/lighttablewindow.cpp b/utilities/lighttable/lighttablewindow.cpp
index b5cc740..332200d 100644
--- a/utilities/lighttable/lighttablewindow.cpp
+++ b/utilities/lighttable/lighttablewindow.cpp
@@ -360,7 +360,7 @@ void LightTableWindow::setupConnections()
    connect(d->thumbView, SIGNAL(signalDroppedItems(QList<ImageInfo>)),
            this, SLOT(slotThumbbarDroppedItems(QList<ImageInfo>)));

-    connect(d->thumbView, SIGNAL(imageActivated(ImageInfo)),
+    connect(d->thumbView, SIGNAL(currentChanged(ImageInfo)),
            this, SLOT(slotItemSelected(ImageInfo)));

    connect(d->thumbView, SIGNAL(signalContentChanged()),
Comment 7 caulier.gilles 2011-09-16 11:32:26 UTC
Marcel,

I can reproduce the problem in #4. Next/Previous button doesn't work. I don't understand why, because LT thumbbar implementation is based on ImageEditor thumbbar view which work very well. 

Especially, these methods are implemented to select previous and next items :

ImageThumbnailBar::nextIndex()
ImageThumbnailBar::prevIndex()

It's done like into ImageWindowPriv::nextIndex() and ImageWindowPriv::previousIndex().

What's wrong here ?

Gilles Caulier
Comment 8 Marcel Wiesweg 2011-09-18 09:27:49 UTC
Git commit 605dd063df2e4418fc6b22bbb710e8fe048ee50e by Marcel Wiesweg.
Committed on 17/09/2011 at 18:38.
Pushed by mwiesweg into branch 'master'.

Add a custom model to keep light table status. Simplify code.

Add a custom model which implemented the light table right/left roles,
removed them from global code.
Use methods provided by DCategorizedView for next/prev/first/last navigation.

CCBUG: 280082

M  +110  -65   utilities/lighttable/lighttablethumbbar.cpp
M  +0    -4    utilities/lighttable/lighttablethumbbar.h
M  +12   -44   utilities/lighttable/lighttablewindow.cpp

http://commits.kde.org/digikam/605dd063df2e4418fc6b22bbb710e8fe048ee50e
Comment 9 Marcel Wiesweg 2011-09-18 10:27:25 UTC
With your fix and my commit, this seems to work now. Did not test by-pair behavior yet.
Comment 10 Lukas Jirkovsky 2011-09-18 11:19:41 UTC
Thank you guys! Everything works perfectly with both by pair enabled and disabled.

I think this bug can be closed.