Bug 257933 - DigiKam zoom slider resets to minimum when returning from image preview [patch]
Summary: DigiKam zoom slider resets to minimum when returning from image preview [patch]
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Preview-Image (show other bugs)
Version: 1.5.0
Platform: openSUSE Linux
: NOR minor
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-26 03:58 UTC by Vivek
Modified: 2014-03-04 21:14 UTC (History)
2 users (show)

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


Attachments
screenshot: changed zoom level with identical slider #2 (22.74 KB, image/png)
2010-11-26 04:00 UTC, Vivek
Details
screenshot: changed zoom level with identical slider #1 (28.49 KB, image/png)
2010-11-26 04:00 UTC, Vivek
Details
screenshot: changed zoom level with identical slider #3 (32.15 KB, image/png)
2010-11-26 04:01 UTC, Vivek
Details
zoom_slider_bug_01.png (141.74 KB, image/png)
2013-05-02 15:26 UTC, nurupo
Details
zoom_slider_bug_02.png (140.32 KB, image/png)
2013-05-02 15:26 UTC, nurupo
Details
patch_for_bug_257933.diff (2.61 KB, patch)
2013-05-02 15:29 UTC, nurupo
Details
patch_for_bug_257933_v2.diff (1.79 KB, patch)
2013-05-03 22:50 UTC, nurupo
Details
patch_for_bug_257933_v3.diff (3.46 KB, patch)
2013-05-05 17:23 UTC, nurupo
Details
remerged patch patch_for_bug_257933_v4.diff (3.45 KB, patch)
2014-03-03 23:48 UTC, kde
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vivek 2010-11-26 03:58:31 UTC
Version:           1.5.0 (using KDE 4.5.3) 
OS:                Linux

DigiKam's zoom slider resets to minimum when returning from image preview - the preview size stays the same as earlier.  This makes the zoom slider harder to use, although it usually "hops" one spot away from the correct zoom level if clicking zoom in/out.

See attached screenshots.

Reproducible: Didn't try

Steps to Reproduce:
Change zoom level from minimum in browsing view, open image preview, return to browsing images.

Actual Results:  
Zoom slider returns to minimum.

Expected Results:  
Zoom slider should stay at previously adjusted level.

OS: Linux (i686) release 2.6.34.7-0.5-default
Compiler: gcc
Comment 1 Vivek 2010-11-26 04:00:04 UTC
Created attachment 53745 [details]
screenshot: changed zoom level with identical slider #2
Comment 2 Vivek 2010-11-26 04:00:35 UTC
Created attachment 53746 [details]
screenshot: changed zoom level with identical slider #1
Comment 3 Vivek 2010-11-26 04:01:29 UTC
Created attachment 53747 [details]
screenshot: changed zoom level with identical slider #3

Expected behaviour, as seen before previewing image.
Comment 4 caulier.gilles 2011-12-13 09:54:28 UTC
Vivek,

This entry still valid with 2.x serie ?

Gilles Caulier
Comment 5 nurupo 2013-05-02 15:25:01 UTC
It's still somewhat present in 3.x.

If you set the zoom slider to n in Thumbnail [zoom_slider_bug_01.png], then switch to Preview Image and back to Thumbnail, the zoom slider may show values of n+2 [zoom_slider_bug_02.png].

This happens because when switching back to Thumbnail, the slider gets reset with value of size of thumbnails, which will return size+2 if thumbnails have a highlight border [`ThumbnailLoadThread::ThumbnailLoadThreadPriv::pixmapSizeForThumbnailSize(...)`].

Note that this bug not only increases zoom slider's value to +2, but also resizes thumbnails to +2.

The following patch fixes that.
It checks is thumbnail has a border, and if it does, it returns sizeWithBorders-2, which then used in calling `DigikamApp::slotThumbSizeChanged(int size)`.
Comment 6 nurupo 2013-05-02 15:26:02 UTC
Created attachment 79631 [details]
zoom_slider_bug_01.png
Comment 7 nurupo 2013-05-02 15:26:23 UTC
Created attachment 79632 [details]
zoom_slider_bug_02.png
Comment 8 nurupo 2013-05-02 15:29:28 UTC
Created attachment 79636 [details]
patch_for_bug_257933.diff
Comment 9 Marcel Wiesweg 2013-05-03 14:36:40 UTC
My suggestions would be: name the reverse of "thumbnailPixmapSize()" "thumbnailImageSize" or rename both to a pair of words which better transport the meaning.
Why not patch "ThumbnailSize ImageCategorizedView::thumbnailSize() const" to return the actual size without border? It is ImageCategorizedView::setThumbnailSize(const ThumbnailSize& s) which adds the 2 pixels, so the same class should subtract them.
Comment 10 nurupo 2013-05-03 22:49:12 UTC
(In reply to comment #9)
> My suggestions would be: name the reverse of "thumbnailPixmapSize()"
> "thumbnailImageSize" or rename both to a pair of words which better
> transport the meaning.

I agree that names are confusing. What would you say about renaming `ThumbnailLoadThread::thumbnailPixmapSize` to `ThumbnailLoadThread::thumbnailToPixmapSize` and `ThumbnailLoadThread::pixmapThumbnailSize` to `ThumbnailLoadThread::pixmapToThumbnailSize`, because it's basically what they are doing, converting pixmap size into thumbnail size and back.

> Why not patch "ThumbnailSize ImageCategorizedView::thumbnailSize() const" to
> return the actual size without border? It is
> ImageCategorizedView::setThumbnailSize(const ThumbnailSize& s) which adds
> the 2 pixels, so the same class should subtract them.

Yes, patching `ImageCategorizedView::thumbnailSize` looks like a better solution, because logically it should return the value which was initially set, not the value+2. But still, we can't just return size-2 from it, -2 should be done only if thumbnail has highlight boarders.
Comment 11 nurupo 2013-05-03 22:50:37 UTC
Created attachment 79680 [details]
patch_for_bug_257933_v2.diff

Here is the new patch. Please disregard patch_for_bug_257933.diff.
I didn't rename methods because we still haven't decided on names.
Comment 12 Marcel Wiesweg 2013-05-05 14:34:32 UTC
I agree with your renaming suggestion, it's symmetric and speaking after all.
Patch is welcome, or commit if you've got KDE commit rights.
Comment 13 nurupo 2013-05-05 17:23:07 UTC
Created attachment 79720 [details]
patch_for_bug_257933_v3.diff

The same as previous + renamed methods.
(I don't have KDE commit rights.)
Comment 14 caulier.gilles 2013-10-30 15:19:55 UTC
Marcel,

What's new about this entry ? Patch is acceptable for you ?

Nurupo,

Please update your patch against current digiKam code from git/master. It cannot be applied :

[gilles@localhost core]$ patch -p1 < patch_for_bug_257933_v3.diff 
patching file digikam/items/imagecategorizedview.cpp
Hunk #1 succeeded at 431 (offset -1 lines).
Hunk #2 succeeded at 446 (offset -1 lines).
patching file digikam/items/imagethumbnaildelegate.cpp
patching file libs/threadimageio/thumbnailloadthread.cpp
Hunk #1 succeeded at 277 (offset 3 lines).
Hunk #2 FAILED at 289.
1 out of 2 hunks FAILED -- saving rejects to file libs/threadimageio/thumbnailloadthread.cpp.rej
patching file libs/threadimageio/thumbnailloadthread.h

Gilles Caulier
Comment 15 Marcel Wiesweg 2013-10-31 20:51:42 UTC
Yes it's fine see comment 12 ;-)
Comment 16 caulier.gilles 2013-10-31 22:37:04 UTC
Nurupo,

Accordingly with comment #15 from Marcel, i waiting your patch updated to include it with 4.0.0 release

Thanks in advance

Gilles Caulier
Comment 17 nurupo 2013-10-31 22:39:17 UTC
Surprised the patch wasn't applied 6 month ago. It will take me some time to update it.
Comment 18 caulier.gilles 2013-11-25 14:26:42 UTC
Nurupo,

I always waiting your patch updated... 

Thanks in advance

Gilles Caulier
Comment 19 kde 2014-03-03 23:48:29 UTC
Created attachment 85403 [details]
remerged patch patch_for_bug_257933_v4.diff

remerged patch just in case nobody else does it
Comment 20 caulier.gilles 2014-03-04 06:47:55 UTC
I tried to understand and reproduce the original problem here without to apply the patch, but i cannot using 4.0.0-beta3...

Switching between Icon-View and Preview mode restore properly zoom settings for these modes.

Can you explain better how to reproduce the dysfunction please ?

Gilles Caulier
Comment 21 caulier.gilles 2014-03-04 21:12:22 UTC
Git commit bbcee345a825861e7a7e8d2e4a6cacdea92ce77c by Gilles Caulier.
Committed on 04/03/2014 at 21:08.
Pushed by cgilles into branch 'master'.

Apply patch #85403 from nurupo which fix bug increasing zoom slider/thumbs size value to +2 when user swith between preview and icon-view mode.
FIXED-IN: 4.0.0

M  +2    -2    digikam/items/imagecategorizedview.cpp
M  +1    -1    digikam/items/imagethumbnaildelegate.cpp
M  +7    -2    libs/threadimageio/thumbnailloadthread.cpp
M  +7    -2    libs/threadimageio/thumbnailloadthread.h

http://commits.kde.org/digikam/bbcee345a825861e7a7e8d2e4a6cacdea92ce77c