From dc314851e70a4302d2988aa0e07aeaae19dec990 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Wed, 11 Oct 2023 21:41:53 +0200 Subject: [PATCH] QtIcoHandler::canRead(): avoid checking more than once QtIcoHandler::canRead() calls ICOReader::canRead(), which assumes that QIODevice::pos() is at the position where the .ico data starts (i.e. pos() == 0 if this is a separate .ico file). But if an AnimatedImage in Qt Quick opens an .ico file, canRead() gets called multiple times: the first is when QQuickAnimatedImage::frameCount() eventually results in QImageReaderPrivate::initHandler(); then ICOReader::readHeader() is called, which moves the file position. The second time is when QQuickAnimatedImage calls QMovie::isValid(). At that time, QIODevice::pos() == 6: we need to avoid calling ICOReader::canRead() because it's no longer at the start of the data. Without this change, AnimatedImage reports "Error Reading Animated Image File" and doesn't show anything. The fix is to store the known-good state, the same way that QTiffHandler::canRead() returns true if its d->tiff already exists (TIFFClientOpen() succeeded). The test checks that it's ok to call QMovie::frameCount() first and then QMovie::isValid(). Calling frameCount() has the effect of moving QIODevice::pos(). Pick-to: 6.6 6.5 Task-number: QTBUG-117429 Change-Id: Ie3a5225f2cea9a0d76d685e83ce4d4a10cbe9188 Reviewed-by: Eirik Aavitsland Reviewed-by: Qt CI Bot (cherry picked from commit 3f515fa7aff7cb24565f0bb61b16bb2bde6faf60) Reviewed-by: Qt Cherry-pick Bot --- src/plugins/imageformats/ico/qicohandler.cpp | 3 +++ src/plugins/imageformats/ico/qicohandler.h | 2 +- tests/auto/gui/image/qmovie/tst_qmovie.cpp | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/plugins/imageformats/ico/qicohandler.cpp b/src/plugins/imageformats/ico/qicohandler.cpp index e58dde4e974..18b39766f5b 100644 --- a/src/plugins/imageformats/ico/qicohandler.cpp +++ b/src/plugins/imageformats/ico/qicohandler.cpp @@ -735,6 +735,8 @@ bool QtIcoHandler::supportsOption(ImageOption option) const */ bool QtIcoHandler::canRead() const { + if (knownCanRead) + return true; bool bCanRead = false; QIODevice *device = QImageIOHandler::device(); if (device) { @@ -744,6 +746,7 @@ bool QtIcoHandler::canRead() const } else { qCWarning(lcIco, "QtIcoHandler::canRead() called with no device"); } + knownCanRead = bCanRead; return bCanRead; } diff --git a/src/plugins/imageformats/ico/qicohandler.h b/src/plugins/imageformats/ico/qicohandler.h index 0fe251ab60b..61c3eea4651 100644 --- a/src/plugins/imageformats/ico/qicohandler.h +++ b/src/plugins/imageformats/ico/qicohandler.h @@ -30,7 +30,7 @@ public: private: int m_currentIconIndex; ICOReader *m_pICOReader; - + mutable bool knownCanRead = false; }; QT_END_NAMESPACE diff --git a/tests/auto/gui/image/qmovie/tst_qmovie.cpp b/tests/auto/gui/image/qmovie/tst_qmovie.cpp index 4a95493b45a..fc810632813 100644 --- a/tests/auto/gui/image/qmovie/tst_qmovie.cpp +++ b/tests/auto/gui/image/qmovie/tst_qmovie.cpp @@ -272,8 +272,8 @@ void tst_QMovie::multiFrameImage() QMovie movie(QFINDTESTDATA("multiframe/Obj_N2_Internal_Mem.ico")); const int expectedFrameCount = 9; - QVERIFY(movie.isValid()); QCOMPARE(movie.frameCount(), expectedFrameCount); + QVERIFY(movie.isValid()); movie.setSpeed(1000); // speed up the test: play at 10 FPS (1000% of normal) QElapsedTimer playTimer; QSignalSpy frameChangedSpy(&movie, &QMovie::frameChanged);