Bug 248087 - Preview not toggling from video mode to image mode [patch]
Summary: Preview not toggling from video mode to image mode [patch]
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Preview-Engine (show other bugs)
Version: 1.4.0
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-16 21:06 UTC by Berthier Lemieux
Modified: 2017-08-15 10:01 UTC (History)
1 user (show)

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


Attachments
Proposed patch (923 bytes, patch)
2010-08-16 21:09 UTC, Berthier Lemieux
Details
Proposed patch, improved version (4.03 KB, patch)
2010-08-17 22:03 UTC, Berthier Lemieux
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Berthier Lemieux 2010-08-16 21:06:42 UTC
Version:           1.4.0 (using KDE 4.4.4) 
OS:                Linux

The preview mode is not toggling from video to image, when "last_image_previewed == image_to_be_previewed"


Reproducible: Always

Steps to Reproduce:
   Have an album that has at least one still image and one video.

   1) Preview the still image 
   2) Watch the video
   3) Go back the the still image preview in step one (with the back/forward buttons, or clicking on the thumbnail)

Actual Results:  

The image preview is not shown. The preview stays on the video.

Expected Results:  

The image preview is shown.
Comment 1 Berthier Lemieux 2010-08-16 21:09:38 UTC
Created attachment 50628 [details]
Proposed patch
Comment 2 Berthier Lemieux 2010-08-16 21:33:18 UTC
Hmm. I'm noob here, can't use the tools correctly, apologies.

The patch submitted above address the error I just raised, the problem is that if  ImagePreviewView::setImageInfo() is called with an imageInfo that is same as already in use, setImageInfo() did not do anything to change the albumwidgetstack mode back to image mode.

I copied the code from ImagePreviewView::slotGotImagePreview, which after reflexion may not be the best thing to do here, at least I should have checked for isLoaded()... 

BTW, it would seem simpler & better to only emit signalPreviewLoaded(), instead of the current calls to d->stack:
        d->stack->setPreviewMode(AlbumWidgetStack::PreviewImageMode);
        d->stack->previewLoaded();
        emit signalPreviewLoaded(true);

Is there any special reason why it's like this? 

PPS. In my patch I also propose a small improvement to the mediaplayer logic:
The assignment:
    KUrl url = info.fileUrl();
doesn't return an empty url, when info.isEmpty(). Media player was not stopped by stop(), but by a request to play("").
Comment 3 Berthier Lemieux 2010-08-17 22:03:38 UTC
Created attachment 50668 [details]
Proposed patch, improved version

Hi, here is a better fix for the issue, this time it handles the cases of images not yet loaded. I feel good about that one.

In addition to the bug reported here, the patch fixes an other bug : the rotation icons are not disabled when image preview is loading. That leads to a situation where preview of image N is displayed, image N+1 preview is loading but not yet ready, user press one of the rotate button, image N+1 is rotated. I would expect that the image displayed (image N) would be rotated, not N+1. I suppose this is really minor in reality, but became very apparent with my test cases where I made the preview loading time to be *really* long.

I also removed the signal signalPreviewLoaded() that wasn't used by anyone.
Comment 4 caulier.gilles 2010-08-17 22:05:45 UTC
Thanks for your patch. If possible we will review your patch before 1.4.0 release planed this next sunday.

Gilles Caulier
Comment 5 Andi Clemens 2010-10-01 23:19:13 UTC
SVN commit 1181721 by aclemens:

Apply patch by  Berthier Lemieux

BUG: 248087

 M  +2 -1      NEWS  
 M  +22 -14    digikam/imagepreviewview.cpp  
 M  +0 -1      digikam/imagepreviewview.h  
 M  +2 -1      digikam/mediaplayerview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1181721