From fd9cdaa55da455b90eacec571aeb2c84fa55f7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Arve=20S=C3=A6ther?= Date: Wed, 22 Apr 2009 16:27:44 +0200 Subject: Improve the icon parsing for files with a slightly wrong BMP header. The reason it failed was that we always expected the height property of the BMP header to be double the height of the icon. The kde_favicon.ico did not fulfill this requirement. We can fix that by simply reading from the ICONDIR entry instead, since that has always the correct height. Task-number: 229829 Reviewed-by: alexis --- src/plugins/imageformats/ico/qicohandler.cpp | 60 ++++++++++----------- tests/auto/qimagereader/baseline/35floppy.ico | Bin 0 -> 4286 bytes tests/auto/qimagereader/baseline/kde_favicon.ico | Bin 0 -> 1150 bytes .../auto/qimagereader/baseline/semitransparent.ico | Bin 0 -> 9662 bytes tests/auto/qimagereader/tst_qimagereader.cpp | 30 +++++++++++ 5 files changed, 58 insertions(+), 32 deletions(-) create mode 100644 tests/auto/qimagereader/baseline/35floppy.ico create mode 100644 tests/auto/qimagereader/baseline/kde_favicon.ico create mode 100644 tests/auto/qimagereader/baseline/semitransparent.ico diff --git a/src/plugins/imageformats/ico/qicohandler.cpp b/src/plugins/imageformats/ico/qicohandler.cpp index aa53370..da5ae15 100644 --- a/src/plugins/imageformats/ico/qicohandler.cpp +++ b/src/plugins/imageformats/ico/qicohandler.cpp @@ -80,7 +80,7 @@ typedef struct typedef struct { // BMP information header quint32 biSize; // size of this struct quint32 biWidth; // pixmap width - quint32 biHeight; // pixmap height + quint32 biHeight; // pixmap height (specifies the combined height of the XOR and AND masks) quint16 biPlanes; // should be 1 quint16 biBitCount; // number of bits per pixel quint32 biCompression; // compression method @@ -108,7 +108,7 @@ private: bool readHeader(); bool readIconEntry(int index, ICONDIRENTRY * iconEntry); - bool readBMPHeader(ICONDIRENTRY & iconEntry, BMP_INFOHDR * header); + bool readBMPHeader(quint32 imageOffset, BMP_INFOHDR * header); void findColorInfo(QImage & image); void readColorTable(QImage & image); @@ -343,7 +343,7 @@ bool ICOReader::readHeader() return headerRead; } -bool ICOReader::readIconEntry(int index, ICONDIRENTRY * iconEntry) +bool ICOReader::readIconEntry(int index, ICONDIRENTRY *iconEntry) { if (iod) { if (iod->seek(startpos + ICONDIR_SIZE + (index * ICONDIRENTRY_SIZE))) { @@ -355,37 +355,12 @@ bool ICOReader::readIconEntry(int index, ICONDIRENTRY * iconEntry) -bool ICOReader::readBMPHeader(ICONDIRENTRY & iconEntry, BMP_INFOHDR * header) +bool ICOReader::readBMPHeader(quint32 imageOffset, BMP_INFOHDR * header) { - memset(&icoAttrib, 0, sizeof(IcoAttrib)); if (iod) { - if (iod->seek(startpos + iconEntry.dwImageOffset)) { + if (iod->seek(startpos + imageOffset)) { if (readBMPInfoHeader(iod, header)) { - - icoAttrib.nbits = header->biBitCount ? header->biBitCount : iconEntry.wBitCount; - icoAttrib.h = header->biHeight / 2; // this height is always double the iconEntry height (for the mask) - icoAttrib.w = header->biWidth; - - switch (icoAttrib.nbits) { - case 32: - case 24: - case 16: - icoAttrib.depth = 32; - break; - case 8: - case 4: - icoAttrib.depth = 8; - break; - default: - icoAttrib.depth = 1; - } - - if ( icoAttrib.depth == 32 ) // there's no colormap - icoAttrib.ncolors = 0; - else // # colors used - icoAttrib.ncolors = header->biClrUsed ? header->biClrUsed : 1 << icoAttrib.nbits; - //qDebug() << "Bits:" << icoAttrib.nbits << "Depth:" << icoAttrib.depth << "Ncols:" << icoAttrib.ncolors; - return TRUE; + return TRUE; } } } @@ -548,7 +523,28 @@ QImage ICOReader::iconAt(int index) if (readIconEntry(index, &iconEntry)) { BMP_INFOHDR header; - if (readBMPHeader(iconEntry, &header)) { + if (readBMPHeader(iconEntry.dwImageOffset, &header)) { + icoAttrib.nbits = header.biBitCount ? header.biBitCount : iconEntry.wBitCount; + + switch (icoAttrib.nbits) { + case 32: + case 24: + case 16: + icoAttrib.depth = 32; + break; + case 8: + case 4: + icoAttrib.depth = 8; + break; + default: + icoAttrib.depth = 1; + } + if (icoAttrib.depth == 32) // there's no colormap + icoAttrib.ncolors = 0; + else // # colors used + icoAttrib.ncolors = header.biClrUsed ? header.biClrUsed : 1 << icoAttrib.nbits; + icoAttrib.w = iconEntry.bWidth; + icoAttrib.h = iconEntry.bHeight; QImage::Format format = QImage::Format_ARGB32; if (icoAttrib.nbits == 24) diff --git a/tests/auto/qimagereader/baseline/35floppy.ico b/tests/auto/qimagereader/baseline/35floppy.ico new file mode 100644 index 0000000..59fd37e Binary files /dev/null and b/tests/auto/qimagereader/baseline/35floppy.ico differ diff --git a/tests/auto/qimagereader/baseline/kde_favicon.ico b/tests/auto/qimagereader/baseline/kde_favicon.ico new file mode 100644 index 0000000..15bcdbb Binary files /dev/null and b/tests/auto/qimagereader/baseline/kde_favicon.ico differ diff --git a/tests/auto/qimagereader/baseline/semitransparent.ico b/tests/auto/qimagereader/baseline/semitransparent.ico new file mode 100644 index 0000000..dd23de9 Binary files /dev/null and b/tests/auto/qimagereader/baseline/semitransparent.ico differ diff --git a/tests/auto/qimagereader/tst_qimagereader.cpp b/tests/auto/qimagereader/tst_qimagereader.cpp index 3841111..8f7094c 100644 --- a/tests/auto/qimagereader/tst_qimagereader.cpp +++ b/tests/auto/qimagereader/tst_qimagereader.cpp @@ -153,6 +153,9 @@ private slots: void autoDetectImageFormat(); void fileNameProbing(); + + void pixelCompareWithBaseline_data(); + void pixelCompareWithBaseline(); }; // Testing get/set functions @@ -1368,5 +1371,32 @@ void tst_QImageReader::fileNameProbing() QCOMPARE(r.fileName(), name); } +void tst_QImageReader::pixelCompareWithBaseline_data() +{ + QTest::addColumn("fileName"); + + QTest::newRow("floppy (16px,32px - 16 colors)") << "35floppy.ico"; + QTest::newRow("semitransparent") << "semitransparent.ico"; + QTest::newRow("slightlybroken") << "kde_favicon.ico"; +} + +void tst_QImageReader::pixelCompareWithBaseline() +{ + QFETCH(QString, fileName); + + QImage icoImg; + // might fail if the plugin does not exist, which is ok. + if (icoImg.load(QString::fromAscii("images/%1").arg(fileName))) { + QString baselineFileName = QString::fromAscii("baseline/%1").arg(fileName); +#if 0 + icoImg.save(baselineFileName); +#else + QImage baseImg; + QVERIFY(baseImg.load(baselineFileName)); + QCOMPARE(baseImg, icoImg); +#endif + } +} + QTEST_MAIN(tst_QImageReader) #include "tst_qimagereader.moc" -- cgit v0.12