Summary: | gwenview first shows picture then uses exif and rotates it => you have to wait two twice until picture is on screen | ||
---|---|---|---|
Product: | [Applications] gwenview | Reporter: | Gerd Grass <gerd.grass> |
Component: | general | Assignee: | Gwenview Bugs <gwenview-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | wishlist | CC: | mwieser |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | openSUSE | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Sentry Crash Report: |
Description
Gerd Grass
2005-11-27 23:50:40 UTC
Aurelien, Lubos, Shouldn't it be fixed in 1.3.x? No, we switch to DocumentJPEGLoadedImpl only when the image is fully loaded and that handles the rotation of jpegs. right thanks *** Bug 133228 has been marked as a duplicate of this bug. *** Lubos, does your statement "(Shouldn't it be fixed in 1.3.x? -) No, we switch to DocumentJPEGLoadedImpl only when the image is fully loaded" mean that this BUG will also not be fixed for 1.4? Unfortunately this would make gwenview unsuitable for many people. SVN commit 592500 by gateau: Started to work on loading JPEG pictures immediatly rotated instead of loading them then rotating them. CCBUG: 117173 M +0 -3 documentjpegloadedimpl.cpp M +5 -5 documentloadingimpl.cpp M +133 -53 imageloader.cpp M +2 -2 imageloader.h --- trunk/extragear/graphics/gwenview/gvcore/documentjpegloadedimpl.cpp #592499:592500 @@ -70,9 +70,6 @@ && orientation!=ImageUtils::NOT_AVAILABLE && orientation!=ImageUtils::NORMAL) { - LOG("jpeg rotating"); - setImage(ImageUtils::transform(mDocument->image(), orientation)); - emitImageRectUpdated(); d->mJPEGContent.transform(orientation); } --- trunk/extragear/graphics/gwenview/gvcore/documentloadingimpl.cpp #592499:592500 @@ -93,13 +93,13 @@ connect( d->mLoader, SIGNAL( imageLoaded( bool )), SLOT( imageLoaded( bool ))); // it's possible the loader already has the whole or at least part of the image loaded - QSize s = d->mLoader->knownSize(); - if( !s.isEmpty()) { + QImage image = d->mLoader->processedImage(); + if (!image.isNull()) { if( d->mLoader->frames().count() > 0 ) { setImage( d->mLoader->frames().first().image); emitImageRectUpdated(); } else { - setImage( d->mLoader->processedImage()); + setImage(image); QMemArray< QRect > rects = d->mLoader->loadedRegion().rects(); for( unsigned int i = 0; i < rects.count(); ++i ) { emit rectUpdated(rects[i]); @@ -107,7 +107,7 @@ } } if( d->mLoader->completed()) emit imageLoaded( d->mLoader->frames().count() != 0 ); - // this may be deleted here + // 'this' may be deleted here } @@ -146,7 +146,7 @@ void DocumentLoadingImpl::imageChanged(const QRect& rect) { - if( d->mLoader->frames().count() > 0 ) return; + //if( d->mLoader->frames().count() > 0 ) return; setImage(d->mLoader->processedImage()); emit rectUpdated(rect); } --- trunk/extragear/graphics/gwenview/gvcore/imageloader.cpp #592499:592500 @@ -25,18 +25,24 @@ // Qt #include <qtimer.h> +#include <qwmatrix.h> // KDE #include <kapplication.h> // Local #include "cache.h" +#include "imageutils/imageutils.h" +#include "imageutils/jpegcontent.h" #include "imageloader.moc" namespace Gwenview { const unsigned int DECODE_CHUNK_SIZE=4096; +/** Interval between image updates, in milli seconds */ +const int IMAGE_UPDATE_INTERVAL=100; + #undef ENABLE_LOG #undef LOG //#define ENABLE_LOG @@ -178,9 +184,10 @@ , mUpdatedDuringLoad(false) , mSuspended(false) , mGetComplete(false) - , mAsyncImageComplete(false) + , mIncrementalDecoderComplete(false) , mNextFrameDelay(0) , mWasFrameData(false) + , mOrientation(ImageUtils::NOT_AVAILABLE) , mURLKind(MimeTypeUtils::KIND_UNKNOWN) , mDecodeComplete( false ) , mStatPending( false ) @@ -211,8 +218,6 @@ // Set to true if at least one changed() signals have been emitted bool mUpdatedDuringLoad; - // Set to image size if sizeLoaded() signal has been emitted - QSize mKnownSize; // A rect of recently loaded pixels that the rest of the application has // not been notified about with the imageChanged() signal @@ -228,7 +233,7 @@ bool mGetComplete; // Set to true when all the image has been decoded - bool mAsyncImageComplete; + bool mIncrementalDecoderComplete; // Delay used for next frame after it's finished decoding. int mNextFrameDelay; @@ -242,6 +247,8 @@ ImageFrames mFrames; QCString mImageFormat; + + ImageUtils::Orientation mOrientation; MimeTypeUtils::Kind mURLKind; @@ -291,7 +298,7 @@ connect(&d->mDecoderTimer, SIGNAL(timeout()), this, SLOT(decodeChunk()) ); connect(&d->mDecoderThread, SIGNAL(succeeded()), - this, SLOT(slotImageDecoded()) ); + this, SLOT(slotDecoderThreadSucceeded()) ); connect(&d->mDecoderThread, SIGNAL(failed()), this, SLOT(slotDecoderThreadFailed()) ); @@ -442,16 +449,14 @@ } int chunkSize = QMIN(DECODE_CHUNK_SIZE, int(d->mRawData.size())-d->mDecodedSize); - LOG("chunkSize: " << chunkSize); int decodedSize = 0; if (chunkSize>0) { decodedSize = d->mDecoder.decode( (const uchar*)(d->mRawData.data()+d->mDecodedSize), chunkSize); - LOG("decodedSize: " << decodedSize); if (decodedSize<0) { - // We can't use async decoding, switch to decoder thread + // We can't use incremental decoding, switch to threaded decoding d->mDecoderTimer.stop(); d->mUseThread=true; if( d->mGetComplete ) startThread(); @@ -466,10 +471,19 @@ // We decoded as much as possible from the buffer, wait to receive // more data before coming again in decodeChunk d->mDecoderTimer.stop(); - if (d->mGetComplete && !d->mAsyncImageComplete) { - // No more data is available, the image must be truncated, - // let's simulate its end - end(); + + if (d->mGetComplete) { + if (!d->mIncrementalDecoderComplete) { + // No more data is available, the image must be truncated, + // let's simulate its end + kdWarning() << "ImageLoader::decodeChunk(): image is truncated.\n"; + + if (d->mProcessedImage.isNull()) { + d->mProcessedImage = d->mDecoder.image(); + } + emit imageChanged(d->mProcessedImage.rect()); + end(); + } } } } @@ -489,24 +503,13 @@ } -void ImageLoader::slotImageDecoded() { +void ImageLoader::slotDecoderThreadSucceeded() { LOG(""); - - // Get image - if (d->mUseThread) { - d->mFrames.clear(); - d->mFrames.append( ImageFrame( d->mDecoderThread.popLoadedImage(), 0 )); - } else if( d->mFrames.count() == 0 ) { - d->mFrames.append( ImageFrame( d->mDecoder.image(), 0 )); - } - - // Set image format - QBuffer buffer(d->mRawData); - buffer.open(IO_ReadOnly); - d->mImageFormat = QImageIO::imageFormat(&buffer); - buffer.close(); - - finish( true ); + d->mProcessedImage = d->mDecoderThread.popLoadedImage(); + d->mFrames.append( ImageFrame( d->mProcessedImage, 0 )); + emit sizeLoaded(d->mProcessedImage.width(), d->mProcessedImage.height()); + emit imageChanged(d->mProcessedImage.rect()); + finish(true); } @@ -518,7 +521,7 @@ d->mDecodeComplete = true; - if( !ok || d->mFrames.count() == 0 ) { + if( !ok /*|| d->mFrames.count() == 0 */) { d->mFrames.clear(); d->mRawData = QByteArray(); d->mImageFormat = QCString(); @@ -526,15 +529,22 @@ emit imageLoaded( false ); return; } + + // Set image format + QBuffer buffer(d->mRawData); + buffer.open(IO_ReadOnly); + d->mImageFormat = QImageIO::imageFormat(&buffer); + buffer.close(); + /* Cache::instance()->addImage( d->mURL, d->mFrames, d->mImageFormat, d->mTimestamp ); ImageFrame lastframe = d->mFrames.last(); d->mFrames.pop_back(); // maintain that processedImage() is not included when calling imageChanged() d->mProcessedImage = lastframe.image; // The decoder did not cause some signals to be emitted, let's do it now - if (d->mKnownSize.isEmpty()) { - emit sizeLoaded( lastframe.image.width(), lastframe.image.height()); + if (d->mLoadedRegion.isEmpty()) { + emit sizeLoaded(d->mProcessedImage.width(), d->mProcessedImage.height()); } if (!d->mLoadChangedRect.isEmpty()) { emit imageChanged( d->mLoadChangedRect ); @@ -542,6 +552,7 @@ emit imageChanged( lastframe.image.rect() ); } d->mFrames.push_back( lastframe ); + */ emit imageLoaded( true ); } @@ -589,25 +600,94 @@ void ImageLoader::end() { LOG(""); + // Notify about the last loaded rectangle + LOG("mLoadChangedRect " << d->mLoadChangedRect); + if (!d->mLoadChangedRect.isEmpty()) { + emit imageChanged( d->mLoadChangedRect ); + } + d->mDecoderTimer.stop(); - d->mAsyncImageComplete=true; - + d->mIncrementalDecoderComplete = true; + + // We are done + if( d->mFrames.count() == 0 ) { + d->mFrames.append( ImageFrame( d->mDecoder.image(), 0 )); + } // The image has been totally decoded, we delay the call to finish because // when we return from this function we will be in decodeChunk(), after the // call to decode(), so we don't want to switch to a new impl yet, since // this means deleting "this". - QTimer::singleShot(0, this, SLOT(slotImageDecoded()) ); + QTimer::singleShot(0, this, SLOT(callFinish()) ); } -void ImageLoader::changed(const QRect& rect) { - d->mProcessedImage = d->mDecoder.image(); + +void ImageLoader::callFinish() { + finish(true); +} + + +void ImageLoader::changed(const QRect& constRect) { + LOG(constRect); + QRect rect = constRect; + + if (d->mLoadedRegion.isEmpty()) { + // This is the first time we get called. Init mProcessedImage and emit + // sizeLoaded. + LOG("mLoadedRegion is empty"); + + // By default, mProcessedImage should use the image from mDecoder + d->mProcessedImage = d->mDecoder.image(); + + ImageUtils::JPEGContent content; + + if (content.loadFromData(d->mRawData)) { + // This is a JPEG, extract orientation and adjust mProcessedImage + // if necessary + d->mOrientation = content.orientation(); + if (d->mOrientation != ImageUtils::NOT_AVAILABLE && d->mOrientation != ImageUtils::NORMAL) { + QSize size = content.size(); + d->mProcessedImage = QImage(size, d->mDecoder.image().depth()); + } + } + + LOG("emit sizeLoaded " << d->mProcessedImage.size()); + emit sizeLoaded(d->mProcessedImage.width(), d->mProcessedImage.height()); + } + + // Apply orientation if necessary + if (d->mOrientation != ImageUtils::NOT_AVAILABLE && d->mOrientation != ImageUtils::NORMAL) { + // We can only rotate whole images, so copy the loaded rect in a temp + // image, rotate the temp image and copy it to mProcessedImage + + // Copy loaded rect + QImage temp(rect.size(), d->mProcessedImage.depth()); + bitBlt(&temp, 0, 0, + &d->mDecoder.image(), rect.left(), rect.top(), rect.width(), rect.height()); + + // Rotate + temp = ImageUtils::transform(temp, d->mOrientation); + + // Compute destination rect + QWMatrix matrix = ImageUtils::transformMatrix(d->mOrientation); + + QRect imageRect = d->mDecoder.image().rect(); + imageRect = matrix.mapRect(imageRect); + + rect = matrix.mapRect(rect); + rect.moveBy(-imageRect.left(), -imageRect.top()); + + // copy temp to mProcessedImage + bitBlt(&d->mProcessedImage, rect.left(), rect.top(), + &temp, 0, 0, temp.width(), temp.height()); + } + + // Update state tracking vars d->mWasFrameData = true; - d->mUpdatedDuringLoad=true; + d->mUpdatedDuringLoad = true; d->mLoadChangedRect |= rect; d->mLoadedRegion |= rect; - if( d->mTimeSinceLastUpdate.elapsed() > 100 ) { - LOG(d->mLoadChangedRect.left() << "-" << d->mLoadChangedRect.top() - << " " << d->mLoadChangedRect.width() << "x" << d->mLoadChangedRect.height() ); + if( d->mTimeSinceLastUpdate.elapsed() > IMAGE_UPDATE_INTERVAL ) { + LOG("emitting imageChanged " << d->mLoadChangedRect); d->mTimeSinceLastUpdate.start(); emit imageChanged(d->mLoadChangedRect); d->mLoadChangedRect = QRect(); @@ -619,6 +699,7 @@ } void ImageLoader::frameDone(const QPoint& offset, const QRect& rect) { + LOG(""); // Another case where the image loading in Qt's is a bit borken. // It's possible to get several notes about a frame being done for one frame (with MNG). if( !d->mWasFrameData ) { @@ -642,7 +723,14 @@ d->mTimeSinceLastUpdate.start(); } d->mLoadedRegion = QRegion(); - QImage image = d->mDecoder.image().copy(); + + QImage image; + if (d->mProcessedImage.isNull()) { + image = d->mDecoder.image().copy(); + } else { + image = d->mProcessedImage.copy(); + } + if( offset != QPoint( 0, 0 ) || rect != image.rect()) { // Blit last frame below 'image' if( !d->mFrames.isEmpty()) { @@ -666,12 +754,9 @@ } } -void ImageLoader::setSize(int width, int height) { - LOG(width << "x" << height); - d->mKnownSize = QSize( width, height ); - // FIXME: There must be a better way than creating an empty image - d->mProcessedImage = QImage( width, height, 32 ); - emit sizeLoaded(width, height); +void ImageLoader::setSize(int, int) { + // Do nothing, size is handled when ::changed() is called for the first + // time } @@ -680,11 +765,6 @@ } -QSize ImageLoader::knownSize() const { - return d->mKnownSize; -} - - ImageFrames ImageLoader::frames() const { return d->mFrames; } --- trunk/extragear/graphics/gwenview/gvcore/imageloader.h #592499:592500 @@ -70,7 +70,6 @@ QByteArray rawData() const; MimeTypeUtils::Kind urlKind() const; KURL url() const; - QSize knownSize() const; QRegion loadedRegion() const; // valid parts of processedImage() bool completed() const; @@ -86,11 +85,12 @@ void slotDataReceived(KIO::Job*, const QByteArray& chunk); void slotGetResult(KIO::Job*); void decodeChunk(); - void slotImageDecoded(); void slotDecoderThreadFailed(); + void slotDecoderThreadSucceeded(); void slotBusyLevelChanged( BusyLevel ); void ownerDestroyed(); void startLoading(); + void callFinish(); private: ImageLoader(); SVN commit 593767 by gateau: Finished reworking of ImageLoader and DocumentLoadingImpl to load JPEG images correctly rotated from the start. BUG: 117173 M +2 -0 NEWS M +2 -9 gvcore/documentloadingimpl.cpp M +0 -1 gvcore/documentloadingimpl.h M +29 -39 gvcore/imageloader.cpp M +1 -2 gvcore/imageloader.h --- trunk/extragear/graphics/gwenview/NEWS #593766:593767 @@ -2,6 +2,8 @@ - Fixes: - Fix crash when showing current folder properties from context menu (Bug 129890) + - Load JPEG rotated images using the right rotation directly instead of + loading them first and rotate them after (Bug 117173) 2006.09.16 - v1.4.0 - Fixes: --- trunk/extragear/graphics/gwenview/gvcore/documentloadingimpl.cpp #593766:593767 @@ -89,7 +89,6 @@ connect( d->mLoader, SIGNAL( urlKindDetermined()), SLOT( slotURLKindDetermined() )); connect( d->mLoader, SIGNAL( sizeLoaded( int, int )), SLOT( sizeLoaded( int, int ))); connect( d->mLoader, SIGNAL( imageChanged( const QRect& )), SLOT( imageChanged( const QRect& ))); - connect( d->mLoader, SIGNAL( frameLoaded()), SLOT( frameLoaded())); connect( d->mLoader, SIGNAL( imageLoaded( bool )), SLOT( imageLoaded( bool ))); // it's possible the loader already has the whole or at least part of the image loaded @@ -106,7 +105,7 @@ } } } - if( d->mLoader->completed()) emit imageLoaded( d->mLoader->frames().count() != 0 ); + if( d->mLoader->completed()) imageLoaded( d->mLoader->frames().count() != 0 ); // 'this' may be deleted here } @@ -146,17 +145,11 @@ void DocumentLoadingImpl::imageChanged(const QRect& rect) { - //if( d->mLoader->frames().count() > 0 ) return; + LOG(rect); setImage(d->mLoader->processedImage()); emit rectUpdated(rect); } -void DocumentLoadingImpl::frameLoaded() { - if( d->mLoader->frames().count() == 1 ) { - // explicit sharing - don't modify the image in document anymore - setImage(d->mLoader->frames().first().image.copy()); - } -} void DocumentLoadingImpl::sizeLoaded(int width, int height) { LOG(width << "x" << height); --- trunk/extragear/graphics/gwenview/gvcore/documentloadingimpl.h #593766:593767 @@ -46,7 +46,6 @@ void slotURLKindDetermined(); void sizeLoaded(int, int); void imageChanged(const QRect&); - void frameLoaded(); void imageLoaded( bool ok ); }; --- trunk/extragear/graphics/gwenview/gvcore/imageloader.cpp #593766:593767 @@ -266,6 +266,14 @@ MimeTypeUtils::Kind mURLKind; QValueVector< OwnerData > mOwners; // loaders may be shared + + + void determineImageFormat() { + Q_ASSERT(mRawData.size()>0); + QBuffer buffer(mRawData); + buffer.open(IO_ReadOnly); + mImageFormat = QImageIO::imageFormat(&buffer); + } }; @@ -339,26 +347,22 @@ // We have the image in cache LOG(d->mURL << ", We have the image in cache"); d->mRawData = Cache::instance()->file( d->mURL ); - QCString format; - ImageFrames frames; - Cache::instance()->getFrames( d->mURL, &frames, &format ); - if( !frames.isEmpty()) { + Cache::instance()->getFrames(d->mURL, &d->mFrames, &d->mImageFormat); + + if( !d->mFrames.isEmpty()) { LOG("The image in cache can be used"); - d->mImageFormat = format; - d->mFrames = frames; d->mProcessedImage = d->mFrames[0].image; emit sizeLoaded(d->mProcessedImage.width(), d->mProcessedImage.height()); emit imageChanged(d->mProcessedImage.rect()); - if( !d->mRawData.isNull() || format != "JPEG" ) { - // The raw data is only needed for JPEG. If it is already - // loaded or if we are not loading a JPEG file, we are done. - finish( true ); - return; - } else { - // Wait for raw data to be downloaded + if (d->mRawData.isNull() && d->mImageFormat=="JPEG") { + // Raw data is needed for JPEG, wait for it to be downloaded LOG("Wait for raw data to be downloaded"); d->mDecodeState = DECODE_CACHED; + } else { + // We don't care about raw data + finish(true); + return; } } else { // Image in cache is broken @@ -451,14 +455,8 @@ } LOG("emit urlKindDetermined(raster)"); emit urlKindDetermined(); - - // Set image format - QBuffer buffer(d->mRawData); - buffer.open(IO_ReadOnly); - d->mImageFormat = QImageIO::imageFormat(&buffer); - buffer.close(); } - + // Decode the received data if( !d->mDecoderTimer.isActive() && (d->mDecodeState==DECODE_WAITING || d->mDecodeState==DECODE_INCREMENTAL_DECODING) @@ -549,14 +547,14 @@ /** - * Make the final adjustments to the image. + * Cache image and emit imageLoaded */ void ImageLoader::finish( bool ok ) { LOG(""); d->mDecodeState = DECODE_DONE; - if( !ok /*|| d->mFrames.count() == 0 */) { + if (!ok) { d->mFrames.clear(); d->mRawData = QByteArray(); d->mImageFormat = QCString(); @@ -565,22 +563,11 @@ return; } - Cache::instance()->addImage( d->mURL, d->mFrames, d->mImageFormat, d->mTimestamp ); - - /* - ImageFrame lastframe = d->mFrames.last(); - d->mFrames.pop_back(); // maintain that processedImage() is not included when calling imageChanged() - d->mProcessedImage = lastframe.image; - // The decoder did not cause some signals to be emitted, let's do it now - if (d->mLoadedRegion.isEmpty()) { - emit sizeLoaded(d->mProcessedImage.width(), d->mProcessedImage.height()); + if (d->mImageFormat.isEmpty()) { + d->determineImageFormat(); } - if (!d->mLoadChangedRect.isEmpty()) { - emit imageChanged( d->mLoadChangedRect ); - } - d->mFrames.push_back( lastframe ); - */ - + Q_ASSERT(d->mFrames.count() > 0); + Cache::instance()->addImage( d->mURL, d->mFrames, d->mImageFormat, d->mTimestamp ); emit imageLoaded( true ); } @@ -654,7 +641,7 @@ void ImageLoader::changed(const QRect& constRect) { - LOG(""); + LOG2(""); QRect rect = constRect; if (d->mLoadedRegion.isEmpty()) { @@ -665,6 +652,10 @@ // By default, mProcessedImage should use the image from mDecoder d->mProcessedImage = d->mDecoder.image(); + if (d->mImageFormat.isEmpty()) { + d->determineImageFormat(); + } + Q_ASSERT(!d->mImageFormat.isEmpty()); if (d->mImageFormat == "JPEG") { // This is a JPEG, extract orientation and adjust mProcessedImage // if necessary @@ -771,7 +762,6 @@ } d->mFrames.append( ImageFrame( image, d->mNextFrameDelay )); d->mNextFrameDelay = 0; - emit frameLoaded(); } void ImageLoader::setLooping(int) { --- trunk/extragear/graphics/gwenview/gvcore/imageloader.h #593766:593767 @@ -76,8 +76,7 @@ signals: void urlKindDetermined(); void sizeLoaded(int, int); - void imageChanged(const QRect&); // use processedImage(), is not in frames() yet - void frameLoaded(); + void imageChanged(const QRect&); void imageLoaded( bool ok ); private slots: |