Bug 145198 - light-table should also work with the full image
Summary: light-table should also work with the full image
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Unclassified
Component: LightTable-Canvas (show other bugs)
Version: unspecified
Platform: Debian stable Linux
: NOR wishlist (vote)
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-08 20:15 UTC by Arnd Baecker
Modified: 2022-02-01 09:07 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 0.9.2


Attachments
Patch adding a higher quality preview loading option (5.78 KB, patch)
2007-05-17 17:55 UTC, Marcel Wiesweg
Details
Adding support toload preview first, then high quality preview (6.35 KB, patch)
2007-05-20 00:01 UTC, Marcel Wiesweg
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Baecker 2007-05-08 20:15:31 UTC
Version:            (using KDE KDE 3.5.5)
Installed from:    Debian stable Packages

In order to really compare the quality (like sharpness) it
is in my opinion essential that the light-table can also work
with non-scaled images (i.e. like the image editor);
Presently a reduced-size version of the image is used. 

Gilles reply (in http://bugs.kde.org/show_bug.cgi?id=145159) was:
> 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.

Still, I think that a 100% view is needed in the light-table.
Maybe something as proposed by Frank in #16 
of http://bugs.kde.org/show_bug.cgi?id=140131
a possible solution, also from the users perspective? 
The idea is to replace the pixmap in the background 
(maybe from a certain zoom-level on) after zooming in.
Comment 1 caulier.gilles 2007-05-10 10:27:40 UTC
Actually, the implementation use the preview extraction mechanism from multithreaded File IO management of Marcel. This part use Qt image interface, not Digikam::Dimg.

Added a new option in LT GUI to handle the full image is trivial, but the multithreaded File IO management need to be patched to load the full image with Qt and not the reduced version.

Important : loading the full image with Qt have limitations :

- It's Slow (of course), especially with huge image (TIFF/PNG)
- It's do not support RAW image.
- No color management. The image is always displayed under sRGB color space.
- No feedback during loading.

In fact, we cannot reproduce the same way to manage image file with Image Editor under Lt using Qt...

Note than i think than color management is not necessary with LT.

Solution : using DImg instead Qt in LT.

Marcel, give me your viewpoint. Thanks in advance

Gilles
Comment 2 Marcel Wiesweg 2007-05-10 21:57:51 UTC
We have a number of options here, we need to know what we want.

Using QImage is preview is good because converting to pixmap is faster, and we only need 8bit, no CM. Preview loading is currently optimized for the preview.

What are the requirements of LT? Do we need 16bit? Do we need CM?
If we do not need neither 16bit nor color management, QImage is still a good format. This is even more true if loading the full image is only an option, e.g. offered at high zoom rates (I could imagine that for preview as well, btw).

Then this would mean loading the full image, with optional progress feedback, but only to 8bit. Loading with DImg in 8bit, then converting to QImage. No problem.
Comment 3 Arnd Baecker 2007-05-10 23:37:53 UTC
> What are the requirements of LT? Do we need 16bit? Do we need CM?
> If we do not need neither 16bit nor color management, QImage is still a good format. This is even more true if loading the full image is only an option, e.g. offered at high zoom rates (I could imagine that for preview as well, btw).


Presumably 16Bit and Color Management are not needed.
If there is a way to load the full size image in the background,
and to display that from a certain zoom-level on,
this would of course be fantastic -
The light-table would feel as fast as right now,
but offer the full size view ``just in time''.
(I think that picasa does something like this, at
least there is a short message "Refining", when
selecting and zooming into an image).

And yes: it might be also useful for the preview
(and maybe even for the image editor itself?)

Best, Arnd
Comment 4 Julien Narboux 2007-05-11 09:54:22 UTC
I do not understand the technical details, but I think that would be nice if the pictures in digikam were everywhere an 'abstraction'. I mean that it would be a function which displays the picture either preview or full size depending on the time. First the preview is displayed, then the real full size with CM when it is ready. It seems to be the way it is done in some other software such as iphoto for instance.

Best,

Julien
Comment 5 Fabien 2007-05-16 11:41:30 UTC
I just tried for the 1st time the light table. It looks really nice.
But, I fully agree with Arnd : if we can't see the full size picture, it's not possible to compare the sharpness and quality of 2 pictures...
I hope it will be possible to do that, even if it's slower.
Comment 6 caulier.gilles 2007-05-16 16:25:22 UTC
Marcel,

Light Table need a new option in configuration panel to solve this B.K.O file to not use the picture preview extraction. If this option is enable :

1/ With JPEG/TIFF/PNG files, the Qt image loaders must be used as well. 

2/ With RAW files, use an half decoding preview. Today, i have separated the implementation in libkdcraw (kdraw.cpp) to get RAW preview : one to extract embedded JPEG only, one to decode an half image of RAW picture. 

    static bool loadEmbeddedPreview(QImage& image, const QString& path);

    static bool loadHalfPreview(QImage& image, const QString& path);

3/ In all case LT do not limit the preview size to a specific value. I propose to use a zero value to not limit the preview size with LoadingDescription.

What do you think about ?

Gilles
Comment 7 Marcel Wiesweg 2007-05-16 23:49:24 UTC
I have a patch on my computer that uses the DImg loaders and then converts to QImage. This was very easy to implement because the preview loading class inherits from the normal loading code, and non-raw images can be shared with the image editor in the cache. RAWs are loaded as half-size images in 8-bit mode. We also get interruptable loading when using DImg. The cost is a bit more memory and memory copying. Do you want to test, or shall I commit?
Comment 8 caulier.gilles 2007-05-17 00:13:14 UTC
Marcel,

Well, post your patch in this B.K.O file. Me, Arnd, Fabien, Mick can test it and report performance test.

Note : if we use DImg to load image data, why continue to use Qt like image container in LT ??? all is benefit to use Dimg in this part especially about performance (no need post conversion to Qt) and we have too a fast algorithm to scale image...

Also, if in the future, we want support Color Management in LT, we can do it... but this is not a priority actually.

Your viewpoint ?

Gilles

Comment 9 Marcel Wiesweg 2007-05-17 17:52:38 UTC
The primary reason currently I thought is that per default, the preview is used, which is faster than the high-quality preview I suggest above most notably for RAW and JPEG images.

I you want to leave that and always use the high-quality preview:

I thought that PreviewWidget is based on QImage, but this is not true, you designed it image image format independent.

The fastscale algorithm does not support 16-bit. I do not know if this is a matter of writing some lines of code, or if the algorithm itself will not really support it. 
We can easily support scaling 8-bit DImgs with FastScale. Currently, the high-quality preview is always 8 bit.
Comment 10 Marcel Wiesweg 2007-05-17 17:55:11 UTC
Created attachment 20608 [details]
Patch adding a higher quality preview loading option

Patch as described above.
Note this also patches LightTablePreview to always use the new option.
Comment 11 Arnd Baecker 2007-05-17 20:46:08 UTC
Great! I applied the patch and everything works fine.
Concerning the performance, it works fast enough for me,
once the images are loaded (does not feel slower than
before...)
However, the drag-and-drop is very slow (especially the first time),
So I think that loading the full-size image should be done in two
steps: first the quick one as right now (without the patch), 
and in a background thread the full one is loaded 
(maybe later even with color-management).

In terms of the implementation this is of course much more
complicated and requires a bit of book-keeping and
memory management, but as also mentioned by Julien in #4 above,
this seems to be the way as the speed problem is also solved in other
applications.

Comment 12 Mikolaj Machowski 2007-05-18 20:12:04 UTC
Dnia pi
Comment 13 caulier.gilles 2007-05-18 20:17:31 UTC
Mik,

The patch is in the B.K.O file...

http://bugs.kde.org/attachment.cgi?id=20608&action=view

Gilles
Comment 14 Mikolaj Machowski 2007-05-19 09:48:37 UTC
> The patch is in the B.K.O file...
> http://bugs.kde.org/attachment.cgi?id=20608&action=view


Thanks. Looks like my mailbox provider is playing time games again, I've
got mail with link to Marcel patch several hours after Arnd's mail.
Whatever.

Tested it with big images and didn't notice time performance hit. Of
course it goes through RAM faster than 6-year old through chocolate bar
but it was also earlier the case. One unpleasant thing: with very low
zoom values scaled images look really ugly. Is it possible to turn on
smoothing for low zoom values (or use preview version as Arnd
suggested)?

Really great work :)
Comment 15 Arnd Baecker 2007-05-19 12:14:07 UTC
> Tested it with big images and didn't notice time performance hit.


Does it also take some time, until the images  get displayed
after a drag-and-drop from the thumb-bar to the two panels
of the light-table?

> One unpleasant thing: with very low
> zoom values scaled images look really ugly. Is it possible to turn on
> smoothing for low zoom values


I don't think I see this kind of effect - could
you maybe attach a screen-shot?

> (or use preview version as Arnd suggested)?


Hmm, this was more meant for a quick display
until the full image is loaded in the background -
usually it takes the user a few seconds to
move to the place of interest and in the mean time
the full image could be loaded and displayed to
replace the preview variant.

Best, Arnd
Comment 16 caulier.gilles 2007-05-19 12:39:16 UTC
>Tested it with big images and didn't notice time performance hit. Of
>course it goes through RAM faster than 6-year old through chocolate bar
>but it was also earlier the case. One unpleasant thing: with very low
>zoom values scaled images look really ugly. Is it possible to turn on
>smoothing for low zoom values (or use preview version as Arnd
>suggested)?

Yes. You see a title side effect generated by the new fastScale algorithm from Antonio. I have surprized than nobody have seen this problem before. I have discovers it many weeks ago (:=)))

To have already investigated about this problem, i'm sure than it's a FastScale implementation failure. If i use Qt:smoothScale instead, all is fine (but more slow of course). This problem only appear to high zoom level. This is not relevant of JPEG compression artifact. Try with a TIFF or PNG image. It's the same.

Nota : if we use DImg::scale implementation, all work fine.

I have pinged Antonio about this problem, but I have no recieve a response yet.

Marcel, like FastScale algorithm sound like very simple to understand, we can try to with it (:=)))

Gilles

Comment 17 Arnd Baecker 2007-05-19 13:09:36 UTC
Ah, so you are talking about *large* zoom values, i.e. 1200% etc.?!
I.e., is this the one already discussed in
http://bugs.kde.org/show_bug.cgi?id=140131
#10, #11, #12, ...
In #26 Antonio provided a fix, and I thought it was gone -
But, yes, at 1200% it is visible but not at 120%, it seems

The reason I did not see that is that
for images with 270 x 363 pixels a maximal zoom of 1200% is
possible, while for 3456 x 2304  the maximum is 148%!
(We discussed the reason for this some time ago, but
I forgot; still I find it not intuitive, why
the maximal zoom depends on the image size ...)
Comment 18 caulier.gilles 2007-05-19 14:05:30 UTC
>Ah, so you are talking about *large* zoom values, i.e. 1200% etc.?!
>I.e., is this the one already discussed in
>http://bugs.kde.org/show_bug.cgi?id=140131
>#10, #11, #12, ...
>In #26 Antonio provided a fix, and I thought it was gone -
>But, yes, at 1200% it is visible but not at 120%, it seems

No Arnd, This is not the same problem. the dysfunction is in the algorithm, not in the  preview tile mechanism.

Gilles
Comment 19 Marcel Wiesweg 2007-05-20 00:01:53 UTC
Created attachment 20635 [details]
Adding support toload preview first, then high quality preview

A slightly extended version of the patch. Now the fast preview is loaded first,
and then the full version, as suggested by Arnd.

For RAW images this has the effect that images get a bit darker when the full
version is loaded, as it seems that dcraw has a slightly different idea about
white balance or whatever as the camera creating the preview. Dont know if this
darkening is confusing or not.
Comment 20 Arnd Baecker 2007-05-20 14:47:54 UTC
Marcel,

many thanks - this is really great - it ``feels'' much faster now-
to me this seems like the right solution -
but let's hear what others think.

Concerning the effect for the raw images: I think this
is tolerable - in the end even color-management in the background
(maybe as a third step??) might be asked for...
((BTW, in my case, Raw images get slightly brighter ...))

This new way of loading the full image in the background works so
nicely, that this could also be an option for the normal preview
(because of memory concerns, this could/should be made
configurable.)
Comment 21 caulier.gilles 2007-05-20 14:52:14 UTC
>Adding support to load preview first, then high quality preview

>A slightly extended version of the patch. Now the fast preview is loaded first,
>and then the full version, as suggested by Arnd.

It work fine here excepted a little problem if you change the zoom factor between the moment of low quality preview appear and high quality preview is loaded. The preview is reseted to be fit on window.

A solution is to get the zoom factor and the position of low quality preview to be apply on high quality preview.

>For RAW images this has the effect that images get a bit darker when the full
>version is loaded, as it seems that dcraw has a slightly different idea about
>white balance or whatever as the camera creating the preview. Dont know if this
>darkening is confusing or not.

Yes. This is how dcraw decode WB settings. The embeded preview in rAW file is a JPEG image with a WB set by the camera device. dcraw try to reproduce this settings to apply on RAW decoded image, but it's not perfect... 

I see to issues for this problem:

1/ a general solution is to add an indicator behing preview panels to indicate what the user can see (low | high quality preview)

2/ an option from menu (with shortcut) to toogle manually the high quality loading on demand. Personally, i work with low quality. It perfect for me.

3/ an option in config panel to :

- load only low preview (the user can toogle manually on hjigh preview following 2/.
- load low preview and after high preview automaticly.
- load only high preview.

Arnd, Marcel, Mik, Fabien, etc... What do you think about ?

Gilles
Comment 22 Arnd Baecker 2007-05-20 15:03:36 UTC
I have a preference for option 3/

Here one should have the possibility to have different settings for
- F3 preview
- light-table
Reason: for the F3 preview it seems more common
that the small version (instead of the full image) suffices
(well, personally, I would set this to high quality... ;-) -
(This of course assumes that this new two-step background
loading is also applied to the F3 preview)
Comment 23 Mikolaj Machowski 2007-05-20 20:09:53 UTC
It is faster now. Unfortunately didn't help for "staircases" (I thought
that with smaller preview image, real zoom used for scaling preview
images wouldn't have such disastrous effects).

> It work fine here excepted a little problem if you change the zoom
> factor between the moment of low quality preview appear and high quality
> preview is loaded. The preview is reseted to be fit on window.


Yes, images are shivering :) Another effect is that when changing image
for some time old picture stays. Looks stupid with images of various
formats (landscape, portrait).
> 3/ an option in config panel to :
>
> - load only low preview (the user can toogle manually on hjigh preview
> following 2/. 
> - load low preview and after high preview automaticly.
> - load only high preview.
>
> Arnd, Marcel, Mik, Fabien, etc... What do you think about ?


Third option with second sub-option as default. Eventually try to guess
size of RAM and set this option automatically. RAM < 512MB - a); 
512MB > RAM > 1.5GB - b); over 1.5GB c).
Comment 24 Fabien 2007-05-21 12:58:12 UTC
I would vote for 3 (option in config panel)...

It would be really nice !

Thanks again Gilles and Marcel (and others) for the work you do.
Comment 25 Fabien 2007-05-21 17:08:33 UTC
I played a bit with lighttable with the 2nd patch (preview first, then hq), it's fine for me (note: I only have jpeg files). It's really fast and having the high quality picture is really a huge improvement !
Thanks.
Comment 26 caulier.gilles 2007-05-21 23:05:25 UTC
SVN commit 667096 by cgilles:

digiKam from trunk : Light Table preview : fix tile side effect with high zoom level. This is a first approach how to solv ethis problem. It's not optimum but work fine.

What the problem exactly ? Well it's simple : we scaling a part of full image to render a preview unsing a tile matrix. The part of ful image is scaled with take a care of 
image data outside the region. With a high zoom level the scale algorithm cannot compute properlly the border of the tile and a side effect appear.

CCBUGS: 145198


 M  +24 -5     lighttablepreview.cpp  


--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablepreview.cpp #667095:667096
@@ -218,6 +218,15 @@
 
 void LightTablePreview::setPreviousNextPaths(const QString& previous, const QString &next)
 {
+    if (d->previewPreloadThread)
+    {
+        // stop preloading, but only if the loading is no longer needed
+        if (d->nextPath != d->path && d->nextPath != next && d->nextPath != previous)
+            d->previewPreloadThread->stopLoading(d->nextPath);
+        if (d->previousPath != d->path && d->previousPath != previous && d->previousPath != next)
+            d->previewPreloadThread->stopLoading(d->previousPath);
+    }
+
     d->nextPath     = next;
     d->previousPath = previous;
 }
@@ -281,7 +290,16 @@
     }
 
     unsetCursor();
-    slotNextPreload();
+
+    if (description.previewParameters.size != 0)
+    {
+        d->previewThread->loadHighQuality(LoadingDescription(description.filePath,
+                                          0, AlbumSettings::instance()->getExifRotate()));
+    }
+    else
+    {
+        slotNextPreload();
+    }
 }
 
 void LightTablePreview::slotNextPreload()
@@ -300,7 +318,7 @@
     else
         return;
 
-    d->previewPreloadThread->load(LoadingDescription(loadPath, d->previewSize,
+    d->previewPreloadThread->loadHighQuality(LoadingDescription(loadPath, 0,
                                   AlbumSettings::instance()->getExifRotate()));
 }
 
@@ -645,9 +663,10 @@
 
 void LightTablePreview::paintPreview(QPixmap *pix, int sx, int sy, int sw, int sh)
 {
-    // Fast smooth scale method from Antonio.   
-    QImage img = FastScale::fastScaleQImage(d->preview.copy(sx, sy, sw, sh),
-                                            tileSize(), tileSize());
+    QImage img = FastScale::fastScaleQImage(d->preview.copy(sx-sw/10, sy-sh/10, sw+sw/5, sh+sh/5), 
+                                            tileSize()+tileSize()/5, tileSize()+tileSize()/5)
+                           .copy(tileSize()/10, tileSize()/10, tileSize(), tileSize());
+
     bitBlt(pix, 0, 0, &img, 0, 0);
 }
 
Comment 27 caulier.gilles 2007-05-29 14:34:25 UTC
SVN commit 669446 by cgilles:

digikam from trunk : Light Table : new option to handle full size image in preview panel, instead a reduced size.
For performance reasons, this option is disable by default. Use it if you have a fast computer...
BUG: 145198


 M  +9 -9      project/project.kdevelop  
 M  +12 -1     utilities/lighttable/lighttablepreview.cpp  
 M  +2 -0      utilities/lighttable/lighttablepreview.h  
 M  +6 -0      utilities/lighttable/lighttableview.cpp  
 M  +2 -0      utilities/lighttable/lighttableview.h  
 M  +3 -2      utilities/lighttable/lighttablewindow.cpp  
 M  +10 -1     utilities/setup/setuplighttable.cpp  


--- trunk/extragear/graphics/digikam/project/project.kdevelop #669445:669446
@@ -12,7 +12,7 @@
     </keywords>
     <projectdirectory>./</projectdirectory>
     <absoluteprojectpath>false</absoluteprojectpath>
-    <description></description>
+    <description/>
     <ignoreparts/>
     <projectname>digikam</projectname>
     <defaultencoding/>
@@ -74,11 +74,11 @@
   <kdevdebugger>
     <general>
       <dbgshell>libtool</dbgshell>
-      <programargs></programargs>
-      <gdbpath></gdbpath>
-      <configGdbScript></configGdbScript>
-      <runShellScript></runShellScript>
-      <runGdbScript></runGdbScript>
+      <programargs/>
+      <gdbpath/>
+      <configGdbScript/>
+      <runShellScript/>
+      <runGdbScript/>
       <breakonloadinglibs>true</breakonloadinglibs>
       <separatetty>false</separatetty>
       <floatingtoolbar>false</floatingtoolbar>
@@ -113,8 +113,8 @@
     <run>
       <directoryradio>build</directoryradio>
       <customdirectory>/</customdirectory>
-      <mainprogram>digikam/digikam/digikam</mainprogram>
-      <programargs></programargs>
+      <mainprogram>/home/gilles/Documents/devel/SVN/trunk/graphics/digikam/digikam/digikam</mainprogram>
+      <programargs/>
       <terminal>false</terminal>
       <autocompile>false</autocompile>
       <envvars/>
@@ -192,7 +192,7 @@
       <includePaths>.;</includePaths>
     </codecompletion>
     <creategettersetter>
-      <prefixGet></prefixGet>
+      <prefixGet/>
       <prefixSet>set</prefixSet>
       <prefixVariable>m_,_</prefixVariable>
       <parameterName>theValue</parameterName>
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablepreview.cpp #669445:669446
@@ -89,6 +89,7 @@
         hasNext              = false;
         selected             = false;
         dragAndDropEnabled   = true;
+        loadFullImageSize    = false;
         currentFitWindowZoom = 0;
         previewSize          = 1024;
     }
@@ -97,6 +98,7 @@
     bool               hasNext;
     bool               selected;
     bool               dragAndDropEnabled;
+    bool               loadFullImageSize;
 
     int                previewSize;
 
@@ -171,6 +173,12 @@
     delete d;
 }
 
+void LightTablePreview::setLoadFullImageSize(bool b)
+{
+    d->loadFullImageSize = b;
+    reload();
+}
+
 void LightTablePreview::setDragAndDropEnabled(bool b)
 {
     d->dragAndDropEnabled = b;
@@ -252,7 +260,10 @@
                 this, SLOT(slotNextPreload()));
     }
 
-    d->previewThread->load(LoadingDescription(path, d->previewSize, AlbumSettings::instance()->getExifRotate()));
+    if (d->loadFullImageSize)
+        d->previewThread->loadHighQuality(LoadingDescription(path, 0, AlbumSettings::instance()->getExifRotate()));
+    else
+        d->previewThread->load(LoadingDescription(path, d->previewSize, AlbumSettings::instance()->getExifRotate()));
 }
 
 void LightTablePreview::slotGotImagePreview(const LoadingDescription &description, const DImg& preview)
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablepreview.h #669445:669446
@@ -55,6 +55,8 @@
     LightTablePreview(QWidget *parent=0);
     ~LightTablePreview();
 
+    void setLoadFullImageSize(bool b);
+
     void setImage(const DImg& image);
     DImg& getImage() const;
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttableview.cpp #669445:669446
@@ -142,6 +142,12 @@
     delete d;
 }
 
+void LightTableView::setLoadFullImageSize(bool b)
+{
+    d->leftPreview->setLoadFullImageSize(b); 
+    d->rightPreview->setLoadFullImageSize(b); 
+}
+
 void LightTableView::setSyncPreview(bool sync)
 {
     d->syncPreview = sync;
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttableview.h #669445:669446
@@ -59,6 +59,8 @@
     ImageInfo* leftImageInfo() const;
     ImageInfo* rightImageInfo() const;
 
+    void   setLoadFullImageSize(bool b);
+
     void   setLeftZoomFactor(double z);
     void   setRightZoomFactor(double z);
 
--- trunk/extragear/graphics/digikam/utilities/lighttable/lighttablewindow.cpp #669445:669446
@@ -150,9 +150,10 @@
     KConfig* config = kapp->config();
     config->setGroup("LightTable Settings");
 
-    d->autoLoadOnRightPanel  = config->readBoolEntry("Auto Load Right Panel", true);
-    d->autoSyncPreview       = config->readBoolEntry("Auto Sync Preview", true);
+    d->autoLoadOnRightPanel  = config->readBoolEntry("Auto Load Right Panel",   true);
+    d->autoSyncPreview       = config->readBoolEntry("Auto Sync Preview",       true);
     d->fullScreenHideToolBar = config->readBoolEntry("FullScreen Hide ToolBar", false);
+    d->previewView->setLoadFullImageSize(config->readBoolEntry("Load Full Image size", false));
 }
 
 void LightTableWindow::closeEvent(QCloseEvent* e)
--- trunk/extragear/graphics/digikam/utilities/setup/setuplighttable.cpp #669445:669446
@@ -58,11 +58,13 @@
         hideToolBar          = 0;
         autoSyncPreview      = 0;
         autoLoadOnRightPanel = 0;
+        loadFullImageSize    = 0;
     }
 
     QCheckBox *hideToolBar;
     QCheckBox *autoSyncPreview;
     QCheckBox *autoLoadOnRightPanel;
+    QCheckBox *loadFullImageSize;
 };
 
 SetupLightTable::SetupLightTable(QWidget* parent )
@@ -81,11 +83,16 @@
                      "zooming and panning between left and right panels if the images have "
                      "the same size."));
 
-    d->autoLoadOnRightPanel = new QCheckBox(i18n("Selecting a thumbbar item loads the image to the right panel"),
+    d->autoLoadOnRightPanel = new QCheckBox(i18n("Selecting a thumbbar item loads image to the right panel"),
                                             interfaceOptionsGroup);
     QWhatsThis::add( d->autoLoadOnRightPanel, i18n("<p>Set this option to automatically load an image "
                      "into the right panel when the corresponding item is selected on the thumbbar."));
 
+    d->loadFullImageSize = new QCheckBox(i18n("Load full image size"), interfaceOptionsGroup);
+    QWhatsThis::add( d->loadFullImageSize, i18n("<p>Set this option to load full image size "
+                     "in preview panel instead a reduced one. Because this option will take more time "
+                     "to load image, use it only if you have a fast computer."));
+
     d->hideToolBar = new QCheckBox(i18n("H&ide toolbar in fullscreen mode"), interfaceOptionsGroup);
 
     // --------------------------------------------------------
@@ -112,6 +119,7 @@
     d->hideToolBar->setChecked(config->readBoolEntry("FullScreen Hide ToolBar", false));
     d->autoSyncPreview->setChecked(config->readBoolEntry("Auto Sync Preview", true));
     d->autoLoadOnRightPanel->setChecked(config->readBoolEntry("Auto Load Right Panel", true));
+    d->loadFullImageSize->setChecked(config->readBoolEntry("Load Full Image size", false));
 }
 
 void SetupLightTable::applySettings()
@@ -121,6 +129,7 @@
     config->writeEntry("FullScreen Hide ToolBar", d->hideToolBar->isChecked());
     config->writeEntry("Auto Sync Preview", d->autoSyncPreview->isChecked());
     config->writeEntry("Auto Load Right Panel", d->autoLoadOnRightPanel->isChecked());
+    config->writeEntry("Load Full Image size", d->loadFullImageSize->isChecked());
     config->sync();
 }
 
Comment 28 caulier.gilles 2007-05-29 14:40:06 UTC
Ok,

The new option to handle the full size image is on svn... But the idea to load in first the preview, and after the full image size is not implemented.

Why ? Because the reduced and full image size are different and we have a problem with the patch provided by Marcel to conserve the same zoom level between reduced size view and full size view. It can be complicated to solve, and i need more time to investigate.

Arnd, Mik, Fabien, please do not mix subject. I considerate than this bug is closed (the subject is use the full image size). Please open a new one about to load reduced image and after full image size. Note than this idea can be apply to the AlbumGUI preview mode.

Thanks in advance for your comprehension. 

Gilles

Comment 29 Mikolaj Machowski 2007-05-29 14:55:22 UTC
One more comment from me because it applies directly to last patch: IMO 
this behavior should be on by default. I have seasoned machine (Sempron 
2200, 768MB RAM) and it behaves nicely with full image. Easier to turn 
off unwanted feature than discover it.

----------------------------------------------------
Top Trendy. Najwa
Comment 30 Arnd Baecker 2007-05-29 17:06:03 UTC
> The new option to handle the full size image is on svn...


Great!
Only problem is that the exif orientation is not respected,
i.e. an portrait shot comes out in land-scape.
(This happens both in the Light-table and in the album
view if both are configured to display the full image).
Comment 31 caulier.gilles 2007-05-29 17:25:00 UTC
To Arnd, 

Right, i can reproduce it. 

Marcel, 

it sound like a problem in "threadimageio" implementation, because the LoadingDescription include the orientation parameter from AlbumSettings, as with reduced image size. This is not a problem from preview widget implementation.

Nota: in case of full Size image is loaded, we use a different implementation to handle image data. Something is wrong/missing in this part.

Gilles
Comment 32 Marcel Wiesweg 2007-05-30 23:02:31 UTC
Yes of course. Another thing I forgot ;-)
Usually images are unrotated in the cache, but (normal) preview images come readily rotated until now. That was a good idea as long as it was QImage. Now with DImg the distinction is somewhat artificial. I will see how to fix that.
Comment 33 Arnd Baecker 2007-06-01 17:56:36 UTC
Ad #28, loading in the background is now in:
http://bugs.kde.org/show_bug.cgi?id=146260
Comment 34 Marcel Wiesweg 2007-06-03 00:15:51 UTC
SVN commit 670899 by mwiesweg:

Fix rotation in LightTable for high quality image.
Move rotation method to a static method.
Store the fact that an image is exif-rotated in the image object.
Call method again in foreground thread, when as high quality
images are unrotated in the cache. Currently, previews are rotated
in the cache.

I hope this fixes the problem in all possible situations.

CCBUG: 145198


 M  +4 -1      digikam/imagepreviewview.cpp  
 M  +8 -5      libs/threadimageio/loadingdescription.h  
 M  +123 -0    libs/threadimageio/loadsavethread.cpp  
 M  +40 -20    libs/threadimageio/loadsavethread.h  
 M  +5 -110    libs/threadimageio/previewtask.cpp  
 M  +0 -1      libs/threadimageio/previewtask.h  
 M  +4 -1      utilities/lighttable/lighttablepreview.cpp  


--- branches/extragear/kde3/graphics/digikam/digikam/imagepreviewview.cpp #670898:670899
@@ -271,8 +271,11 @@
     }
     else
     {
+        DImg img(preview);
+        if (AlbumSettings::instance()->getExifRotate())
+            d->previewThread->exifRotate(img, description.filePath);
         d->parent->setPreviewMode(AlbumWidgetStack::PreviewImageMode);
-        setImage(preview);
+        setImage(img);
         d->parent->previewLoaded();
         emit signalPreviewLoaded(true);
     }
--- branches/extragear/kde3/graphics/digikam/libs/threadimageio/loadingdescription.h #670898:670899
@@ -52,7 +52,7 @@
 
     /**
      * An invalid LoadingDescription
-    */
+     */
     LoadingDescription()
     {
     }
@@ -60,7 +60,7 @@
     /**
      * Use this for files that are not raw files.
      * Stores only the filePath.
-    */
+     */
     LoadingDescription(const QString &filePath)
         : filePath(filePath)
         {
@@ -70,15 +70,18 @@
     /**
      * For raw files:
      * Stores filePath and RawDecodingSettings
-    */
+     */
     LoadingDescription(const QString &filePath, KDcrawIface::RawDecodingSettings settings)
         : filePath(filePath), rawDecodingSettings(settings)
         {};
 
     /**
      * For preview jobs:
-     * Stores preview max size and exif rotation
-    */
+     * Stores preview max size and exif rotation.
+     * The exif rotation is only a hint.
+     * Call LoadSaveThread::exifRotate to make sure that the image is really
+     * rotated. It is safe to call this method even if the image is rotated.
+     */
     LoadingDescription(const QString &filePath, int size, bool exifRotate)
         : filePath(filePath)
         {
--- branches/extragear/kde3/graphics/digikam/libs/threadimageio/loadsavethread.cpp #670898:670899
@@ -25,6 +25,7 @@
 // Local includes.
 
 #include "ddebug.h"
+#include "dmetadata.h"
 #include "loadsavethread.h"
 #include "managedloadsavethread.h"
 #include "sharedloadsavethread.h"
@@ -201,7 +202,129 @@
     return running() && !d->running;
 }
 
+bool LoadSaveThread::exifRotate(DImg &image, const QString& filePath)
+{
+    QVariant attribute(image.attribute("exifRotated"));
+    if (attribute.isValid() && attribute.toBool())
+        return false;
 
+    // Raw files are already rotated properlly by dcraw. Only perform auto-rotation with JPEG/PNG/TIFF file.
+    // We don't have a feedback from dcraw about auto-rotated RAW file during decoding. Return true anyway.
+
+    if (DImg::fileFormat(filePath) == DImg::RAW)
+    {
+        return true;
+    }
+
+    // Rotate thumbnail based on metadata orientation information
+
+    DMetadata metadata(filePath);
+    DMetadata::ImageOrientation orientation = metadata.getImageOrientation();
+
+    bool rotatedOrFlipped = false;
+
+    if(orientation != DMetadata::ORIENTATION_NORMAL)
+    {
+        switch (orientation) 
+        {
+            case DMetadata::ORIENTATION_NORMAL:
+            case DMetadata::ORIENTATION_UNSPECIFIED:
+                break;
+
+            case DMetadata::ORIENTATION_HFLIP:
+                image.flip(DImg::HORIZONTAL);
+                rotatedOrFlipped = true;
+                break;
+
+            case DMetadata::ORIENTATION_ROT_180:
+                image.rotate(DImg::ROT180);
+                rotatedOrFlipped = true;
+                break;
+
+            case DMetadata::ORIENTATION_VFLIP:
+                image.flip(DImg::VERTICAL);
+                rotatedOrFlipped = true;
+                break;
+
+            case DMetadata::ORIENTATION_ROT_90_HFLIP:
+                image.rotate(DImg::ROT90);
+                image.flip(DImg::HORIZONTAL);
+                rotatedOrFlipped = true;
+                break;
+
+            case DMetadata::ORIENTATION_ROT_90:
+                image.rotate(DImg::ROT90);
+                rotatedOrFlipped = true;
+                break;
+
+            case DMetadata::ORIENTATION_ROT_90_VFLIP:
+                image.rotate(DImg::ROT90);
+                image.flip(DImg::VERTICAL);
+                rotatedOrFlipped = true;
+                break;
+
+            case DMetadata::ORIENTATION_ROT_270:
+                image.rotate(DImg::ROT270);
+                rotatedOrFlipped = true;
+                break;
+        }
+    }
+
+    image.setAttribute("exifRotated", true);
+    return rotatedOrFlipped;
+
+    /*
+    if (orientation == DMetadata::ORIENTATION_NORMAL ||
+        orientation == DMetadata::ORIENTATION_UNSPECIFIED)
+        return;
+
+    QWMatrix matrix;
+
+    switch (orientation)
+    {
+        case DMetadata::ORIENTATION_NORMAL:
+        case DMetadata::ORIENTATION_UNSPECIFIED:
+            break;
+
+        case DMetadata::ORIENTATION_HFLIP:
+            matrix.scale(-1, 1);
+            break;
+
+        case DMetadata::ORIENTATION_ROT_180:
+            matrix.rotate(180);
+            break;
+
+        case DMetadata::ORIENTATION_VFLIP:
+            matrix.scale(1, -1);
+            break;
+
+        case DMetadata::ORIENTATION_ROT_90_HFLIP:
+            matrix.scale(-1, 1);
+            matrix.rotate(90);
+            break;
+
+        case DMetadata::ORIENTATION_ROT_90:
+            matrix.rotate(90);
+            break;
+
+        case DMetadata::ORIENTATION_ROT_90_VFLIP:
+            matrix.scale(1, -1);
+            matrix.rotate(90);
+            break;
+
+        case DMetadata::ORIENTATION_ROT_270:
+            matrix.rotate(270);
+            break;
+    }
+
+    // transform accordingly
+    thumb = thumb.xForm( matrix );
+    */
+}
+
+
+
+
 }   // namespace Digikam
 
 #include "loadsavethread.moc"
--- branches/extragear/kde3/graphics/digikam/libs/threadimageio/loadsavethread.h #670898:670899
@@ -55,12 +55,14 @@
 
     enum NotificationPolicy
     {
-        // Always send notification, unless the last event is still in the event queue
+        /** Always send notification, unless the last event is still in the event queue */
         NotificationPolicyDirect,
-        // Always wait for a certain amount of time after the last event sent.
-        // In particular, the first event will be sent only after waiting for this time span.
-        // (Or no event will be sent, when the loading has finished before)
-        // This is the default.
+        /**
+         * Always wait for a certain amount of time after the last event sent.
+         * In particular, the first event will be sent only after waiting for this time span.
+         * (Or no event will be sent, when the loading has finished before)
+         * This is the default.
+         */
         NotificationPolicyTimeLimited
     };
 
@@ -74,35 +76,53 @@
     };
 
     LoadSaveThread();
-    // The thread will execute all pending tasks and wait for this upon destruction
+    /**
+     * Destructor:
+     * The thread will execute all pending tasks and wait for this upon destruction
+     */
     ~LoadSaveThread();
 
-    // Append a task to load the given file to the task list
+    /** Append a task to load the given file to the task list */
     void load(LoadingDescription description);
-    // Append a task to save the image to the task list
+    /** Append a task to save the image to the task list */
     void save(DImg &image, const QString& filePath, const QString &format);
 
     void setNotificationPolicy(NotificationPolicy notificationPolicy);
 
     bool isShuttingDown();
 
+    /**
+     * Utility to make sure that an image is rotated according to exif tag.
+     * Detects if an image has previously already been rotated: You can
+     * call this method more than one time on the same image.
+     * Returns true if the image has actually been rotated or flipped.
+     * Returns false if a rotation was not needed.
+     */
+    static bool exifRotate(DImg &image, const QString& filePath);
+
 signals:
 
-    // This signal is emitted when the loading process begins.
+    /** This signal is emitted when the loading process begins. */
     void signalImageStartedLoading(const LoadingDescription &loadingDescription);
-    // This signal is emitted whenever new progress info is available
-    // and the notification policy allows emitting the signal.
-    // No progress info will be sent for preloaded images (ManagedLoadSaveThread).
+    /**
+     * This signal is emitted whenever new progress info is available
+     * and the notification policy allows emitting the signal.
+     * No progress info will be sent for preloaded images (ManagedLoadSaveThread).
+     */
     void signalLoadingProgress(const LoadingDescription &loadingDescription, float progress);
-    // This signal is emitted when the loading process has finished.
-    // If the process failed, img is null.
+    /**
+     * This signal is emitted when the loading process has finished.
+     * If the process failed, img is null.
+     */
     void signalImageLoaded(const LoadingDescription &loadingDescription, const DImg& img);
-    // This signal is emitted if
-    //  - you are doing shared loading (SharedLoadSaveThread)
-    //  - you started a loading operation with a LoadingDescription for
-    //    a reduced version of the image
-    //  - another thread started a loading operation for a more complete version
-    // You may want to cancel the current operation and start with the given loadingDescription
+    /**
+     * This signal is emitted if
+     *  - you are doing shared loading (SharedLoadSaveThread)
+     *  - you started a loading operation with a LoadingDescription for
+     *    a reduced version of the image
+     *  - another thread started a loading operation for a more complete version
+     * You may want to cancel the current operation and start with the given loadingDescription
+     */
     void signalMoreCompleteLoadingAvailable(const LoadingDescription &oldLoadingDescription,
                                             const LoadingDescription &newLoadingDescription);
 
--- branches/extragear/kde3/graphics/digikam/libs/threadimageio/previewtask.cpp #670898:670899
@@ -77,16 +77,10 @@
 
             // rotate if needed - images are unrotated in the cache,
             // except for RAW images, which are already rotated by dcraw.
-            if (m_loadingDescription.previewParameters.exifRotate &&
-                 DImg::fileFormat(m_loadingDescription.filePath) != DImg::RAW)
+            if (m_loadingDescription.previewParameters.exifRotate)
             {
-                QVariant attribute(cachedImg->attribute("exifRotated"));
-                if (!attribute.isValid() || !attribute.toBool())
-                {
-                    // images in the cache are unrotated - except for RAWs
-                    img = img.copy();
-                    exifRotate(m_loadingDescription.filePath, img);
-                }
+                img = img.copy();
+                LoadSaveThread::exifRotate(img, m_loadingDescription.filePath);
             }
 
             QApplication::postEvent(m_thread, new LoadedEvent(m_loadingDescription.filePath, img));
@@ -191,8 +185,9 @@
         //img = img.fastScale(scaledSize.width(), scaledSize.height());
     }
 
+    // Scale if hinted, Store previews rotated in the cache (?)
     if (m_loadingDescription.previewParameters.exifRotate)
-        exifRotate(m_loadingDescription.filePath, img);
+        LoadSaveThread::exifRotate(img, m_loadingDescription.filePath);
 
     {
         LoadingCache::CacheLock lock(cache);
@@ -251,105 +246,5 @@
     return false;
 }
 
-
-
-void PreviewLoadingTask::exifRotate(const QString& filePath, DImg& image)
-{
-    // Rotate thumbnail based on metadata orientation information
-
-    DMetadata metadata(filePath);
-    DMetadata::ImageOrientation orientation = metadata.getImageOrientation();
-
-    if(orientation != DMetadata::ORIENTATION_NORMAL)
-    {
-        switch (orientation) 
-        {
-            case DMetadata::ORIENTATION_NORMAL:
-            case DMetadata::ORIENTATION_UNSPECIFIED:
-                break;
-
-            case DMetadata::ORIENTATION_HFLIP:
-                image.flip(DImg::HORIZONTAL);
-                break;
-
-            case DMetadata::ORIENTATION_ROT_180:
-                image.rotate(DImg::ROT180);
-                break;
-
-            case DMetadata::ORIENTATION_VFLIP:
-                image.flip(DImg::VERTICAL);
-                break;
-
-            case DMetadata::ORIENTATION_ROT_90_HFLIP:
-                image.rotate(DImg::ROT90);
-                image.flip(DImg::HORIZONTAL);
-                break;
-
-            case DMetadata::ORIENTATION_ROT_90:
-                image.rotate(DImg::ROT90);
-                break;
-
-            case DMetadata::ORIENTATION_ROT_90_VFLIP:
-                image.rotate(DImg::ROT90);
-                image.flip(DImg::VERTICAL);
-                break;
-
-            case DMetadata::ORIENTATION_ROT_270:
-                image.rotate(DImg::ROT270);
-                break;
-        }
-    }
-
-    image.setAttribute("exifRotated", true);
-
-    /*
-    if (orientation == DMetadata::ORIENTATION_NORMAL ||
-        orientation == DMetadata::ORIENTATION_UNSPECIFIED)
-        return;
-
-    QWMatrix matrix;
-
-    switch (orientation)
-    {
-        case DMetadata::ORIENTATION_NORMAL:
-        case DMetadata::ORIENTATION_UNSPECIFIED:
-            break;
-
-        case DMetadata::ORIENTATION_HFLIP:
-            matrix.scale(-1, 1);
-            break;
-
-        case DMetadata::ORIENTATION_ROT_180:
-            matrix.rotate(180);
-            break;
-
-        case DMetadata::ORIENTATION_VFLIP:
-            matrix.scale(1, -1);
-            break;
-
-        case DMetadata::ORIENTATION_ROT_90_HFLIP:
-            matrix.scale(-1, 1);
-            matrix.rotate(90);
-            break;
-
-        case DMetadata::ORIENTATION_ROT_90:
-            matrix.rotate(90);
-            break;
-
-        case DMetadata::ORIENTATION_ROT_90_VFLIP:
-            matrix.scale(1, -1);
-            matrix.rotate(90);
-            break;
-
-        case DMetadata::ORIENTATION_ROT_270:
-            matrix.rotate(270);
-            break;
-    }
-
-    // transform accordingly
-    thumb = thumb.xForm( matrix );
-    */
-}
-
 } // namespace Digikam
 
--- branches/extragear/kde3/graphics/digikam/libs/threadimageio/previewtask.h #670898:670899
@@ -50,7 +50,6 @@
 
     bool needToScale(const QSize &imageSize, int previewSize);
     bool loadImagePreview(QImage& image, const QString& path);
-    void exifRotate(const QString& filePath, DImg& img);
 };
 
 
--- branches/extragear/kde3/graphics/digikam/utilities/lighttable/lighttablepreview.cpp #670898:670899
@@ -289,7 +289,10 @@
     }
     else
     {
-        setImage(preview);
+        DImg img(preview);
+        if (AlbumSettings::instance()->getExifRotate())
+            d->previewThread->exifRotate(img, description.filePath);
+        setImage(img);
         emit signalPreviewLoaded(true);
     }
 
Comment 35 Arnd Baecker 2007-06-03 09:34:43 UTC
Many thanks, Marcel, it seems to work fine!
This is the first time that the light-table
is really useable (for all orientations)!

And now the real challenge of delayed loading
is waiting for you ... ;-)