From 434aa248ad5710c7f65283fc3beb7e8adb8b1ad7 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Tue, 26 Feb 2019 16:23:08 +0100 Subject: Heic handler: fix orientation and other image properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mac heic handler lacked support for any meta-data i/o. Most notably, the image orientation proprty was ignored, so images read in could be wrongly oriented. Fixes: QTBUG-73415 Change-Id: I779f91dc28c7441b124aab4557e1abcd3e69fde9 Reviewed-by: Morten Johan Sørvig --- .../imageformats/macheif/qmacheifhandler.cpp | 30 +--- src/plugins/imageformats/macheif/qmacheifhandler.h | 7 +- src/plugins/imageformats/shared/qiiofhelpers.cpp | 194 ++++++++++++++++++--- src/plugins/imageformats/shared/qiiofhelpers_p.h | 24 +++ tests/auto/heif/tst_qheif.cpp | 80 ++++++++- tests/shared/images/heif.qrc | 1 + tests/shared/images/heif/newlogoCCW.heic | Bin 0 -> 4847 bytes 7 files changed, 287 insertions(+), 49 deletions(-) create mode 100644 tests/shared/images/heif/newlogoCCW.heic diff --git a/src/plugins/imageformats/macheif/qmacheifhandler.cpp b/src/plugins/imageformats/macheif/qmacheifhandler.cpp index 7c63e52..385d5e2 100644 --- a/src/plugins/imageformats/macheif/qmacheifhandler.cpp +++ b/src/plugins/imageformats/macheif/qmacheifhandler.cpp @@ -43,22 +43,8 @@ QT_BEGIN_NAMESPACE -class QMacHeifHandlerPrivate -{ - Q_DECLARE_PUBLIC(QMacHeifHandler) - Q_DISABLE_COPY(QMacHeifHandlerPrivate) -public: - QMacHeifHandlerPrivate(QMacHeifHandler *q_ptr) - : writeQuality(-1), q_ptr(q_ptr) - {} - - int writeQuality; - QMacHeifHandler *q_ptr; -}; - - QMacHeifHandler::QMacHeifHandler() - : d_ptr(new QMacHeifHandlerPrivate(this)) + : d(new QIIOFHelper(this)) { } @@ -90,28 +76,30 @@ bool QMacHeifHandler::canRead() const bool QMacHeifHandler::read(QImage *image) { - return QIIOFHelpers::readImage(this, image); + return d->readImage(image); } bool QMacHeifHandler::write(const QImage &image) { - return QIIOFHelpers::writeImage(this, image, QStringLiteral("public.heic")); + return d->writeImage(image, QStringLiteral("public.heic")); } QVariant QMacHeifHandler::option(ImageOption option) const { - return QVariant(); + return d->imageProperty(option); } void QMacHeifHandler::setOption(ImageOption option, const QVariant &value) { - Q_UNUSED(option) - Q_UNUSED(value) + d->setOption(option, value); } bool QMacHeifHandler::supportsOption(ImageOption option) const { - return false; + return option == Quality + || option == Size + || option == ImageTransformation + || option == TransformedByDefault; } QT_END_NAMESPACE diff --git a/src/plugins/imageformats/macheif/qmacheifhandler.h b/src/plugins/imageformats/macheif/qmacheifhandler.h index 6e94a59..cf63de8 100644 --- a/src/plugins/imageformats/macheif/qmacheifhandler.h +++ b/src/plugins/imageformats/macheif/qmacheifhandler.h @@ -49,13 +49,13 @@ class QImage; class QByteArray; class QIODevice; class QVariant; -class QMacHeifHandlerPrivate; +class QIIOFHelper; class QMacHeifHandler : public QImageIOHandler { public: QMacHeifHandler(); - ~QMacHeifHandler(); + ~QMacHeifHandler() override; bool canRead() const override; bool read(QImage *image) override; @@ -67,8 +67,7 @@ public: static bool canRead(QIODevice *iod); private: - Q_DECLARE_PRIVATE(QMacHeifHandler) - QScopedPointer d_ptr; + QScopedPointer d; }; QT_END_NAMESPACE diff --git a/src/plugins/imageformats/shared/qiiofhelpers.cpp b/src/plugins/imageformats/shared/qiiofhelpers.cpp index 2b16787..f8172fe 100644 --- a/src/plugins/imageformats/shared/qiiofhelpers.cpp +++ b/src/plugins/imageformats/shared/qiiofhelpers.cpp @@ -38,15 +38,12 @@ ****************************************************************************/ #include -#include #include #include #include -#include #include "qiiofhelpers_p.h" -#include QT_BEGIN_NAMESPACE @@ -57,14 +54,14 @@ static size_t cbGetBytes(void *info, void *buffer, size_t count) QIODevice *dev = static_cast(info); if (!dev || !buffer) return 0; - qint64 res = dev->read(static_cast(buffer), count); - return qMax(qint64(0), res); + qint64 res = dev->read(static_cast(buffer), qint64(count)); + return size_t(qMax(qint64(0), res)); } static off_t cbSkipForward(void *info, off_t count) { QIODevice *dev = static_cast(info); - if (!dev || !count) + if (!dev || count <= 0) return 0; qint64 res = 0; if (!dev->isSequential()) { @@ -72,7 +69,7 @@ static off_t cbSkipForward(void *info, off_t count) dev->seek(prevPos + count); res = dev->pos() - prevPos; } else { - char *buf = new char[count]; + char *buf = new char[quint64(count)]; res = dev->read(buf, count); delete[] buf; } @@ -89,8 +86,8 @@ static size_t cbPutBytes(void *info, const void *buffer, size_t count) QIODevice *dev = static_cast(info); if (!dev || !buffer) return 0; - qint64 res = dev->write(static_cast(buffer), count); - return qMax(qint64(0), res); + qint64 res = dev->write(static_cast(buffer), qint64(count)); + return size_t(qMax(qint64(0), res)); } @@ -117,23 +114,50 @@ QImageIOPlugin::Capabilities QIIOFHelpers::systemCapabilities(const QString &uti } bool QIIOFHelpers::readImage(QImageIOHandler *q_ptr, QImage *out) +{ + QIIOFHelper h(q_ptr); + return h.readImage(out); +} + +bool QIIOFHelpers::writeImage(QImageIOHandler *q_ptr, const QImage &in, const QString &uti) +{ + QIIOFHelper h(q_ptr); + return h.writeImage(in, uti); +} + +QIIOFHelper::QIIOFHelper(QImageIOHandler *q) + : q_ptr(q) +{ +} + +bool QIIOFHelper::initRead() { static const CGDataProviderSequentialCallbacks cgCallbacks = { 0, &cbGetBytes, &cbSkipForward, &cbRewind, nullptr }; - if (!q_ptr || !q_ptr->device() || !out) + if (cgImageSource) + return true; + if (!q_ptr || !q_ptr->device()) return false; - QCFType cgDataProvider; if (QBuffer *b = qobject_cast(q_ptr->device())) { // do direct access to avoid data copy const void *rawData = b->data().constData() + b->pos(); - cgDataProvider = CGDataProviderCreateWithData(nullptr, rawData, b->data().size() - b->pos(), nullptr); + cgDataProvider = CGDataProviderCreateWithData(nullptr, rawData, size_t(b->data().size() - b->pos()), nullptr); } else { cgDataProvider = CGDataProviderCreateSequential(q_ptr->device(), &cgCallbacks); } - QCFType cgImageSource = CGImageSourceCreateWithDataProvider(cgDataProvider, nullptr); - if (!cgImageSource) + cgImageSource = CGImageSourceCreateWithDataProvider(cgDataProvider, nullptr); + + if (cgImageSource) + cfImageDict = CGImageSourceCopyPropertiesAtIndex(cgImageSource, 0, nullptr); + + return (cgImageSource); +} + +bool QIIOFHelper::readImage(QImage *out) +{ + if (!out || !initRead()) return false; QCFType cgImage = CGImageSourceCreateImageAtIndex(cgImageSource, 0, nullptr); @@ -141,11 +165,117 @@ bool QIIOFHelpers::readImage(QImageIOHandler *q_ptr, QImage *out) return false; *out = qt_mac_toQImage(cgImage); - return !out->isNull(); + if (out->isNull()) + return false; + + int dpi = 0; + if (getIntProperty(kCGImagePropertyDPIWidth, &dpi)) + out->setDotsPerMeterX(qRound(dpi / 0.0254f)); + if (getIntProperty(kCGImagePropertyDPIHeight, &dpi)) + out->setDotsPerMeterY(qRound(dpi / 0.0254f)); + + return true; } +bool QIIOFHelper::getIntProperty(CFStringRef property, int *value) +{ + if (!cfImageDict) + return false; -bool QIIOFHelpers::writeImage(QImageIOHandler *q_ptr, const QImage &in, const QString &uti) + CFNumberRef cfNumber = static_cast(CFDictionaryGetValue(cfImageDict, property)); + if (cfNumber) { + int intVal; + if (CFNumberGetValue(cfNumber, kCFNumberIntType, &intVal)) { + if (value) + *value = intVal; + return true; + } + } + return false; +} + +static QImageIOHandler::Transformations exif2Qt(int exifOrientation) +{ + switch (exifOrientation) { + case 1: // normal + return QImageIOHandler::TransformationNone; + case 2: // mirror horizontal + return QImageIOHandler::TransformationMirror; + case 3: // rotate 180 + return QImageIOHandler::TransformationRotate180; + case 4: // mirror vertical + return QImageIOHandler::TransformationFlip; + case 5: // mirror horizontal and rotate 270 CW + return QImageIOHandler::TransformationFlipAndRotate90; + case 6: // rotate 90 CW + return QImageIOHandler::TransformationRotate90; + case 7: // mirror horizontal and rotate 90 CW + return QImageIOHandler::TransformationMirrorAndRotate90; + case 8: // rotate 270 CW + return QImageIOHandler::TransformationRotate270; + } + return QImageIOHandler::TransformationNone; +} + +static int qt2Exif(QImageIOHandler::Transformations transformation) +{ + switch (transformation) { + case QImageIOHandler::TransformationNone: + return 1; + case QImageIOHandler::TransformationMirror: + return 2; + case QImageIOHandler::TransformationRotate180: + return 3; + case QImageIOHandler::TransformationFlip: + return 4; + case QImageIOHandler::TransformationFlipAndRotate90: + return 5; + case QImageIOHandler::TransformationRotate90: + return 6; + case QImageIOHandler::TransformationMirrorAndRotate90: + return 7; + case QImageIOHandler::TransformationRotate270: + return 8; + } + qWarning("Invalid Qt image transformation"); + return 1; +} + +QVariant QIIOFHelper::imageProperty(QImageIOHandler::ImageOption option) +{ + if (!initRead()) + return QVariant(); + + switch (option) { + case QImageIOHandler::Size: { + QSize sz; + if (getIntProperty(kCGImagePropertyPixelWidth, &sz.rwidth()) + && getIntProperty(kCGImagePropertyPixelHeight, &sz.rheight())) { + return sz; + } + break; + } + case QImageIOHandler::ImageTransformation: { + int orient; + if (getIntProperty(kCGImagePropertyOrientation, &orient)) + return int(exif2Qt(orient)); + break; + } + default: + break; + } + + return QVariant(); +} + +void QIIOFHelper::setOption(QImageIOHandler::ImageOption option, const QVariant &value) +{ + if (writeOptions.size() < option + 1) + writeOptions.resize(option + 1); + writeOptions[option] = value; +} + +bool QIIOFHelper::writeImage(const QImage &in, const QString &uti) { static const CGDataConsumerCallbacks cgCallbacks = { &cbPutBytes, nullptr }; @@ -159,17 +289,35 @@ bool QIIOFHelpers::writeImage(QImageIOHandler *q_ptr, const QImage &in, const QS if (!cgImageDest || !cgImage) return false; - QCFType cfVal; - QCFType cfProps; + QCFType cfQuality = nullptr; + QCFType cfOrientation = nullptr; + const void *dictKeys[2]; + const void *dictVals[2]; + int dictSize = 0; + if (q_ptr->supportsOption(QImageIOHandler::Quality)) { bool ok = false; - int writeQuality = q_ptr->option(QImageIOHandler::Quality).toInt(&ok); + int writeQuality = writeOptions.value(QImageIOHandler::Quality).toInt(&ok); // If quality is unset, default to 75% - float quality = (ok && writeQuality >= 0 ? (qMin(writeQuality, 100)) : 75) / 100.0; - cfVal = CFNumberCreate(nullptr, kCFNumberFloatType, &quality); - cfProps = CFDictionaryCreate(nullptr, (const void **)&kCGImageDestinationLossyCompressionQuality, (const void **)&cfVal, 1, - &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + float quality = (ok && writeQuality >= 0 ? (qMin(writeQuality, 100)) : 75) / 100.0f; + cfQuality = CFNumberCreate(nullptr, kCFNumberFloatType, &quality); + dictKeys[dictSize] = static_cast(kCGImageDestinationLossyCompressionQuality); + dictVals[dictSize] = static_cast(cfQuality); + dictSize++; + } + if (q_ptr->supportsOption(QImageIOHandler::ImageTransformation)) { + int orient = qt2Exif(static_cast(writeOptions.value(QImageIOHandler::ImageTransformation).toInt())); + cfOrientation = CFNumberCreate(nullptr, kCFNumberIntType, &orient); + dictKeys[dictSize] = static_cast(kCGImagePropertyOrientation); + dictVals[dictSize] = static_cast(cfOrientation); + dictSize++; } + + QCFType cfProps = nullptr; + if (dictSize) + cfProps = CFDictionaryCreate(nullptr, dictKeys, dictVals, dictSize, + &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + CGImageDestinationAddImage(cgImageDest, cgImage, cfProps); return CGImageDestinationFinalize(cgImageDest); } diff --git a/src/plugins/imageformats/shared/qiiofhelpers_p.h b/src/plugins/imageformats/shared/qiiofhelpers_p.h index da51731..1b1771b 100644 --- a/src/plugins/imageformats/shared/qiiofhelpers_p.h +++ b/src/plugins/imageformats/shared/qiiofhelpers_p.h @@ -52,6 +52,9 @@ // #include +#include +#include +#include QT_BEGIN_NAMESPACE @@ -67,6 +70,27 @@ public: static bool writeImage(QImageIOHandler *q_ptr, const QImage &in, const QString &uti); }; +class QIIOFHelper +{ +public: + QIIOFHelper(QImageIOHandler *q); + + bool readImage(QImage *out); + bool writeImage(const QImage &in, const QString &uti); + QVariant imageProperty(QImageIOHandler::ImageOption option); + void setOption(QImageIOHandler::ImageOption option, const QVariant &value); + +protected: + bool initRead(); + bool getIntProperty(CFStringRef property, int *value); + + QImageIOHandler *q_ptr = nullptr; + QVector writeOptions; + QCFType cgDataProvider = nullptr; + QCFType cgImageSource = nullptr; + QCFType cfImageDict = nullptr; +}; + QT_END_NAMESPACE #endif diff --git a/tests/auto/heif/tst_qheif.cpp b/tests/auto/heif/tst_qheif.cpp index faf22fa..26ddf73 100644 --- a/tests/auto/heif/tst_qheif.cpp +++ b/tests/auto/heif/tst_qheif.cpp @@ -37,6 +37,9 @@ private slots: void initTestCase(); void readImage_data(); void readImage(); + void readProperties_data(); + void readProperties(); + void writeImage(); }; void tst_qheif::initTestCase() @@ -49,8 +52,10 @@ void tst_qheif::readImage_data() { QTest::addColumn("fileName"); QTest::addColumn("size"); + QTest::addColumn("transform"); - QTest::newRow("col") << QString("col320x480.heic") << QSize(320, 480); + QTest::newRow("col") << QString("col320x480.heic") << QSize(320, 480) << int(QImageIOHandler::TransformationNone); + QTest::newRow("rot") << QString("newlogoCCW.heic") << QSize(110, 78) << int(QImageIOHandler::TransformationRotate90); } void tst_qheif::readImage() @@ -66,5 +71,78 @@ void tst_qheif::readImage() QCOMPARE(image.size(), size); } +void tst_qheif::readProperties_data() +{ + readImage_data(); +} + +void tst_qheif::readProperties() +{ + QFETCH(QString, fileName); + QFETCH(QSize, size); + QFETCH(int, transform); + + QSize rawSize = (transform & QImageIOHandler::TransformationRotate90) ? size.transposed() : size; + + QString path = QStringLiteral(":/heif/") + fileName; + QImageReader reader(path); + QCOMPARE(reader.size(), rawSize); + QCOMPARE(int(reader.transformation()), transform); + + QImage image = reader.read(); + QCOMPARE(image.size(), size); + + QCOMPARE(reader.size(), rawSize); + QCOMPARE(int(reader.transformation()), transform); +} + +void tst_qheif::writeImage() +{ + QImage img(20, 10, QImage::Format_ARGB32_Premultiplied); + img.fill(Qt::green); + + QBuffer buf1, buf2; + QImage rimg1; + + { + buf1.open(QIODevice::WriteOnly); + QImageWriter writer(&buf1, "heic"); + QVERIFY(writer.write(img)); + buf1.close(); + QVERIFY(buf1.size() > 0); + + buf1.open(QIODevice::ReadOnly); + QImageReader reader(&buf1); + QVERIFY(reader.read(&rimg1)); + buf1.close(); + QVERIFY(rimg1.size() == img.size()); + } + + { + buf2.open(QIODevice::WriteOnly); + QImageWriter writer(&buf2, "heic"); + writer.setQuality(20); + QVERIFY(writer.write(img)); + buf2.close(); + QVERIFY(buf2.size() > 0); + QVERIFY(buf2.size() < buf1.size()); + } + + { + buf2.open(QIODevice::WriteOnly); + QImageWriter writer(&buf2, "heic"); + writer.setTransformation(QImageIOHandler::TransformationRotate270); + QVERIFY(writer.write(img)); + buf2.close(); + + QImage rimg2; + buf2.open(QIODevice::ReadOnly); + QImageReader reader(&buf2); + QVERIFY(reader.read(&rimg2)); + buf2.close(); + QVERIFY(rimg2.size() == img.size().transposed()); + } +} + QTEST_MAIN(tst_qheif) #include "tst_qheif.moc" diff --git a/tests/shared/images/heif.qrc b/tests/shared/images/heif.qrc index 2a41c36..8232b6a 100644 --- a/tests/shared/images/heif.qrc +++ b/tests/shared/images/heif.qrc @@ -1,5 +1,6 @@ heif/col320x480.heic + heif/newlogoCCW.heic diff --git a/tests/shared/images/heif/newlogoCCW.heic b/tests/shared/images/heif/newlogoCCW.heic new file mode 100644 index 0000000..1604947 Binary files /dev/null and b/tests/shared/images/heif/newlogoCCW.heic differ -- cgit v1.2.3