Bug 247659 - In big albums re-sorting thumbnails on opening right tab bar, move selected image out the screen [patch]
Summary: In big albums re-sorting thumbnails on opening right tab bar, move selected i...
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Thumbs-Image (show other bugs)
Version: 2.0.0
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-13 16:19 UTC by Fest
Modified: 2017-07-24 09:04 UTC (History)
1 user (show)

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


Attachments
Patch fixing this bug (against GSoC branch, but should be clearly appliable to 1.4 branch) (1.78 KB, patch)
2010-08-13 18:03 UTC, Martin Klapetek
Details
patch to apply on trunk (1.4.0) (1.38 KB, patch)
2010-08-16 15:23 UTC, caulier.gilles
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fest 2010-08-13 16:19:36 UTC
Version:           1.3.0 (using KDE 4.5.0) 
OS:                Linux

In big albums, clicking on right tab bar with selected image, moves it out of the screen.
IMHO it's pretty annoying bug, cause you lose selected image out of sight, and have funny situation: In right tab you have properties of image, that you don't see.
 

Reproducible: Always

Steps to Reproduce:
1. Open big album (with thumbnails that don't feet on one screen)
2. Scroll down and select image image in 7+ row.
3. Click on right tab bar (properties or etc)

Actual Results:  
4. Now in place of chosen image, locates other one. And if album is big enough - selected image even not on the screen.

Expected Results:  
5. Selected image stays at least on the screen.
Comment 1 Martin Klapetek 2010-08-13 18:03:31 UTC
Created attachment 50137 [details]
Patch fixing this bug (against GSoC branch, but should be clearly appliable to 1.4 branch)

I've created a simple patch solving this issue. It scrolls back to the image when the sidebar is expanded. It _should_ also work when the sidebar is shrinked back, but for some reason, it doesn't. The slot is called, id is set, but the scroll won't happen. I *think* that the scroll may occur immediately after the signal is emitted but before the sidebar is actually hidden, and then when it really hides, the view gets wider and thus the scrolled image moves. Either that, or it is a bug in Qt's QListView::scrollTo().
Comment 2 caulier.gilles 2010-08-16 14:29:29 UTC
Marcel,

I would to see this entry fixed too in GoSC2010 and trunk. After to have processed more than 1200 photos recently, this dysfunction has irritated me a little bit...

Patch from Martin is very simple to review. It sound fine for me. Your viewpoint ?

Gilles Caulier
Comment 3 caulier.gilles 2010-08-16 14:50:04 UTC
The crash appears in libpgf when thumbnails are created. 

Marcel, it's a possible race condition with thumbnail treads ?

Gilles Caulier
Comment 4 caulier.gilles 2010-08-16 14:59:49 UTC
Oups, sorry for #3, wrong entry...

Gilles Caulier
Comment 5 caulier.gilles 2010-08-16 15:23:47 UTC
Created attachment 50611 [details]
patch to apply on trunk (1.4.0)

Martin,

I just apply your patch against 1.4.0. This version is the same than you without any special modification.

Your patch work when you expand a sidebar, but it doesn't work when you reduce it, as you said (there Qt4.6 is used). Also, it doesn't work with a multiple selection : selection is disabled as well...

Note : please continue to try this patch against 1.4.0, not GoSC2010 branch. It's will be easy to sync both later, from trunk to branch, not the inverse...

Gilles Caulier
Comment 6 Martin Klapetek 2010-08-17 22:42:17 UTC
What should be the behaviour with multiple selection? Scroll to first, to last, to the midddle (think of selection spreaded through entire album, ie. 3 images in the first row, 7 images in the 12th row, 4 images in 14th row, 2 images in 22nd row etc)?

Regarding patching against 1.4.0 - reasonable, will do.
Comment 7 caulier.gilles 2010-08-17 22:46:23 UTC
I think to scroll to the first item from the selection is the more logic for me...

Gilles
Comment 8 Marcel Wiesweg 2010-08-23 10:32:50 UTC
Have you checked that multiple selection is not reset now when the sidebar is expanded?

scrollToCurrentSelected can be simplified, reducing sideeffects, probably like this:
if (selectionModel()->hasSelection()) 
    scrollTo(currentIndex());

A better implementation would store the a) first visible selected item; if none, b) first visible item before resize, and then, after resize, scroll so that this item is visible. I'm not sure if this is easily possible from resizeEvent, because the widget is already resized then.
Comment 9 Fest 2011-07-01 11:55:17 UTC
Digikam 2.0.0-beta6:

1)I didn't managed to understand on what photo Digikam center window (not on selected one), but selected image always stays on the screen.
2)Multiple selection is not reset, Digikam center window on last selected photo.

Is there be any further improvements (like center main window on selected photo), or close the bug ?
Comment 10 Fest 2011-07-01 11:57:12 UTC
Sorry, small mistake.
2)Multiple selection is not reset, Digikam window show last selected
photo. (Not center on it)
Comment 11 caulier.gilles 2011-07-01 13:22:34 UTC
Please check with 2.0.0-RC just released...

Gilles Caulier
Comment 12 Fest 2011-07-04 22:16:48 UTC
Digikam 2.0.0-rc. Same results as in beta.

Folder with 84 photos; main window show images in 3 rows (out of 14) and 6 columns.

1)Selected image stays on the screen after opening sidebar. 

But I didn't managed to understand on what photo Digikam will center window. Selected image can be moved to any row or "column".
For example:
Selecting image in 2 row (4 out of 14) and 3 column - move it after opening sidebar to 1 row and 1 column.
Same trick to image in 2 row (5 out of 14) and 3 column - move it after opening sidebar to 1 row and 2 column.


2)Multiple selection is not reset, Digikam window show last selected
photo.
Selected image can be moved to any row or "column".

So except of strange sorting it's resolved.
Comment 13 Marcel Wiesweg 2011-07-11 15:54:29 UTC
Digikam will ensure the image is visible - it need not be centered. If it's in the top row, it cannot be centered. If the number of images per row changes, position will change anyway. It can become too complex, so we just check that it is visible somewhere.

I understand this bug can be closed because testing is successful.