| Summary: | light-table should also work with the full image | ||
|---|---|---|---|
| Product: | [Applications] digikam | Reporter: | Arnd Baecker <arnd.baecker> |
| Component: | LightTable-Canvas | Assignee: | Digikam Developers <digikam-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | wishlist | CC: | caulier.gilles |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Debian stable | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | 0.9.2 | |
| Sentry Crash Report: | |||
| Attachments: |
Patch adding a higher quality preview loading option
Adding support toload preview first, then high quality preview |
||
|
Description
Arnd Baecker
2007-05-08 20:15:31 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 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. > 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
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 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. 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
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? 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 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. 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.
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. Dnia pi Mik, The patch is in the B.K.O file... http://bugs.kde.org/attachment.cgi?id=20608&action=view Gilles > 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 :)
> 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 >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
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 ...) >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
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.
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.) >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 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) 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). 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. 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. 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);
}
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();
}
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 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 > 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).
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 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. Ad #28, loading in the background is now in: http://bugs.kde.org/show_bug.cgi?id=146260 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);
}
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 ... ;-) |