Heic handler: fix orientation and other image properties

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 <morten.sorvig@qt.io>

7 files changed, 287 insertions, 49 deletions

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<QMacHeifHandlerPrivate> d_ptr;
+    QScopedPointer<QIIOFHelper> 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 <QGuiApplication>
-#include <qpa/qplatformnativeinterface.h>
 #include <QBuffer>
 #include <QImageIOHandler>
 #include <QImage>
-#include <private/qcore_mac_p.h>
 
 #include "qiiofhelpers_p.h"
 
-#include <ImageIO/ImageIO.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -57,14 +54,14 @@ static size_t cbGetBytes(void *info, void *buffer, size_t count)
     QIODevice *dev = static_cast<QIODevice *>(info);
     if (!dev || !buffer)
         return 0;
-    qint64 res = dev->read(static_cast<char *>(buffer), count);
-    return qMax(qint64(0), res);
+    qint64 res = dev->read(static_cast<char *>(buffer), qint64(count));
+    return size_t(qMax(qint64(0), res));
 }
 
 static off_t cbSkipForward(void *info, off_t count)
 {
     QIODevice *dev = static_cast<QIODevice *>(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<QIODevice *>(info);
     if (!dev || !buffer)
         return 0;
-    qint64 res = dev->write(static_cast<const char *>(buffer), count);
-    return qMax(qint64(0), res);
+    qint64 res = dev->write(static_cast<const char *>(buffer), qint64(count));
+    return size_t(qMax(qint64(0), res));
 }
 
 
@@ -118,22 +115,49 @@ 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<CGDataProviderRef> cgDataProvider;
     if (QBuffer *b = qobject_cast<QBuffer *>(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<CGImageSourceRef> 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<CGImageRef> 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<CFNumberRef>(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<CFNumberRef> cfVal;
-    QCFType<CFDictionaryRef> cfProps;
+    QCFType<CFNumberRef> cfQuality = nullptr;
+    QCFType<CFNumberRef> 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<const void *>(kCGImageDestinationLossyCompressionQuality);
+        dictVals[dictSize] = static_cast<const void *>(cfQuality);
+        dictSize++;
+    }
+    if (q_ptr->supportsOption(QImageIOHandler::ImageTransformation)) {
+        int orient = qt2Exif(static_cast<QImageIOHandler::Transformation>(writeOptions.value(QImageIOHandler::ImageTransformation).toInt()));
+        cfOrientation = CFNumberCreate(nullptr, kCFNumberIntType, &orient);
+        dictKeys[dictSize] = static_cast<const void *>(kCGImagePropertyOrientation);
+        dictVals[dictSize] = static_cast<const void *>(cfOrientation);
+        dictSize++;
     }
+
+    QCFType<CFDictionaryRef> 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 <QImageIOPlugin>
+#include <private/qcore_mac_p.h>
+#include <ImageIO/ImageIO.h>
+#include <QVector>
 
 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<QVariant> writeOptions;
+    QCFType<CGDataProviderRef> cgDataProvider = nullptr;
+    QCFType<CGImageSourceRef> cgImageSource = nullptr;
+    QCFType<CFDictionaryRef> 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<QString>("fileName");
     QTest::addColumn<QSize>("size");
+    QTest::addColumn<int>("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 @@
 <RCC>
     <qresource prefix="/">
         <file>heif/col320x480.heic</file>
+        <file>heif/newlogoCCW.heic</file>
     </qresource>
 </RCC>
diff --git a/tests/shared/images/heif/newlogoCCW.heic b/tests/shared/images/heif/newlogoCCW.heic
new file mode 100644
index 0000000..1604947
--- /dev/null
+++ b/tests/shared/images/heif/newlogoCCW.heic