Make PSD image reader endianess-agnostic.

By using the same strategy as the SoftImage PIC handler (and sharing
some code with it), we should avoid reading the image data incorrectly
on big-endian architectures.

REVIEW: 122650
BUG: 337918

src/imageformats/pic.cpp

@@ -1,5 +1,6 @@
 /*
- * Softimage PIC support for QImage
+ * Softimage PIC support for QImage.
+ *
  * Copyright 1998 Halfdan Ingvarsson
  * Copyright 2007 Ruben Lopez <r.lopez@bren.es>
  * Copyright 2014 Alex Merry <alex.merry@kde.org>
@@ -27,6 +28,8 @@
 
 #include "pic_p.h"
 
+#include "rle_p.h"
+
 #include <QDataStream>
 #include <QDebug>
 #include <QImage>
@@ -167,82 +170,6 @@ static QDataStream &operator<< (QDataStream &s, const QList<PicChannel> &channel
     return s;
 }
 
-/**
- * Decodes data written in mixed run-length encoding format.
- *
- * This is intended to be used with lambda functions.
- *
- * Note that this functions expects that, at the current location in @p stream,
- * exactly @p length items have been encoded as a unit (and so it will not be
- * partway through a run when it has decoded @p length items). If this is not
- * the case, it will return @c false.
- *
- * @param stream      The stream to read the data from.
- * @param data        The location to write the data.
- * @param length      The number of items to read.
- * @param readItem    A function that takes a QDataStream reference and reads a
- *                    single item.
- * @param updateItem  A function that takes an item from @p data and an item
- *                    read by @p readItem and produces the item that should be
- *                    written to @p data.
- *
- * @returns @c true if @p length items in mixed RLE were successfully read
- *          into @p data, @c false otherwise.
- */
-template<typename Item, typename Func1, typename Func2>
-static bool decodeMixedRLEData(QDataStream &stream,
-                               Item *data,
-                               quint16 length,
-                               Func1 readItem,
-                               Func2 updateItem)
-{
-    unsigned offset = 0; // in data
-    while (offset < length) {
-        unsigned remaining = length - offset;
-        quint8 count1;
-        stream >> count1;
-
-        if (count1 >= 128u) {
-            unsigned length;
-            if (count1 == 128u) {
-                // If the value is exactly 128, it means that it is more than
-                // 127 repetitions
-                quint16 count2;
-                stream >> count2;
-                length = count2;
-            } else {
-                // If last bit is 1, then it is 2 to 127 repetitions
-                length = count1 - 127u;
-            }
-            if (length > remaining) {
-                qDebug() << "Row overrun:" << length << ">" << remaining;
-                return false;
-            }
-            Item item = readItem(stream);
-            for (unsigned i = offset; i < offset + length; ++i) {
-                data[i] = updateItem(data[i], item);
-            }
-            offset += length;
-        } else {
-            // No repetitions
-            unsigned length = count1 + 1u;
-            if (length > remaining) {
-                qDebug() << "Row overrun:" << length << ">" << remaining;
-                return false;
-            }
-            for (unsigned i = offset; i < offset + length; ++i) {
-                Item item = readItem(stream);
-                data[i] = updateItem(data[i], item);
-            }
-            offset += length;
-        }
-    }
-    if (stream.status() != QDataStream::Ok) {
-        qDebug() << "DataStream status was" << stream.status();;;;
-    }
-    return stream.status() == QDataStream::Ok;
-}
-
 static bool readRow(QDataStream &stream, QRgb *row, quint16 width, QList<PicChannel> channels)
 {
     Q_FOREACH(const PicChannel &channel, channels) {
@@ -273,8 +200,10 @@ static bool readRow(QDataStream &stream, QRgb *row, quint16 width, QList<PicChan
                 qAlpha((channel.code & ALPHA) ? newPixel : oldPixel));
         };
         if (channel.encoding == MixedRLE) {
-            if (!decodeMixedRLEData(stream, row, width, readPixel, updatePixel)) {
+            bool success = decodeRLEData(RLEVariant::PIC, stream, row, width,
-                qDebug() << "decodeMixedRLEData failed";
+                                         readPixel, updatePixel);
+            if (!success) {
+                qDebug() << "decodeRLEData failed";
                 return false;
             }
         } else if (channel.encoding == Uncompressed) {
@@ -294,67 +223,6 @@ static bool readRow(QDataStream &stream, QRgb *row, quint16 width, QList<PicChan
     return stream.status() == QDataStream::Ok;
 }
 
-/**
- * Encodes data in mixed run-length encoding format.
- *
- * This is intended to be used with lambda functions.
- *
- * @param stream      The stream to write the data to.
- * @param data        The data to be written.
- * @param length      The number of items to write.
- * @param itemsEqual  A function that takes two items and returns whether
- *                    @p writeItem would write them identically.
- * @param writeItem   A function that takes a QDataStream reference and an item
- *                    and writes the item to the data stream.
- */
-template<typename Item, typename Func1, typename Func2>
-static void encodeMixedRLEData(QDataStream &stream, const Item *data, unsigned length, Func1 itemsEqual, Func2 writeItem)
-{
-    unsigned offset = 0;
-    while (offset < length) {
-        const Item *chunkStart = data + offset;
-        unsigned maxChunk = qMin(length - offset, 65535u);
-
-        const Item *chunkEnd = chunkStart + 1;
-        quint16 chunkLength = 1;
-        while (chunkLength < maxChunk && itemsEqual(*chunkStart, *chunkEnd)) {
-            ++chunkEnd;
-            ++chunkLength;
-        }
-
-        if (chunkLength > 127) {
-            // Sequence of > 127 identical pixels
-            stream << quint8(128);
-            stream << quint16(chunkLength);
-            writeItem(stream, *chunkStart);
-        } else if (chunkLength > 1) {
-            // Sequence of < 128 identical pixels
-            stream << quint8(chunkLength + 127);
-            writeItem(stream, *chunkStart);
-        } else {
-            // find a string of up to 128 values, each different from the one
-            // that follows it
-            if (maxChunk > 128) {
-                maxChunk = 128;
-            }
-            chunkLength = 1;
-            chunkEnd = chunkStart + 1;
-            while (chunkLength < maxChunk &&
-                    (chunkLength + 1u == maxChunk ||
-                     !itemsEqual(*chunkEnd, *(chunkEnd+1))))
-            {
-                ++chunkEnd;
-                ++chunkLength;
-            }
-            stream << quint8(chunkLength - 1);
-            for (unsigned i = 0; i < chunkLength; ++i) {
-                writeItem(stream, *(chunkStart + i));
-            }
-        }
-        offset += chunkLength;
-    }
-}
-
 bool SoftimagePICHandler::canRead() const
 {
     if (!SoftimagePICHandler::canRead(device())) {
@@ -445,7 +313,8 @@ bool SoftimagePICHandler::write(const QImage &_image)
                 << quint8(qBlue(pixel));
         };
         if (m_compression) {
-            encodeMixedRLEData(stream, row, image.width(), rgbEqual, writeRgb);
+            encodeRLEData(RLEVariant::PIC, stream, row, image.width(),
+                          rgbEqual, writeRgb);
         } else {
             for (int i = 0; i < image.width(); ++i) {
                 writeRgb(stream, row[i]);
@@ -461,7 +330,8 @@ bool SoftimagePICHandler::write(const QImage &_image)
                 str << quint8(qAlpha(pixel));
             };
             if (m_compression) {
-                encodeMixedRLEData(stream, row, image.width(), alphaEqual, writeAlpha);
+                encodeRLEData(RLEVariant::PIC, stream, row, image.width(),
+                              alphaEqual, writeAlpha);
             } else {
                 for (int i = 0; i < image.width(); ++i) {
                     writeAlpha(stream, row[i]);

src/imageformats/psd.cpp

@@ -1,27 +1,41 @@
-/* This file is part of the KDE project
+/*
-   Copyright (C) 2003 Ignacio Castaño <castano@ludicon.com>
+ * Photoshop File Format support for QImage.
-
+ *
-   This program is free software; you can redistribute it and/or
+ * Copyright 2003 Ignacio Castaño <castano@ludicon.com>
-   modify it under the terms of the Lesser GNU General Public
+ * Copyright 2015 Alex Merry <alex.merry@kde.org>
-   License as published by the Free Software Foundation; either
+ *
-   version 2 of the License, or (at your option) any later version.
+ * This library is free software; you can redistribute it and/or
-
+ * modify it under the terms of the GNU Lesser General Public
-   This code is based on Thacher Ulrich PSD loading code released
+ * License as published by the Free Software Foundation; either
-   on public domain. See: http://tulrich.com/geekstuff/
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-/* this code supports:
+/*
- * reading:
+ * This code is based on Thacher Ulrich PSD loading code released
- *     rle and raw psd files
+ * into the public domain. See: http://tulrich.com/geekstuff/
- * writing:
+ */
- *     not supported
+
+/*
+ * Documentation on this file format is available at
+ * http://www.adobe.com/devnet-apps/photoshop/fileformatashtml/
  */
 
 #include "psd_p.h"
 
+#include "rle_p.h"
+
+#include <QDataStream>
+#include <QDebug>
 #include <QImage>
-#include <QtCore/QDataStream>
-// #include <QDebug>
 
 typedef quint32 uint;
 typedef quint16 ushort;
@@ -66,20 +80,6 @@ static QDataStream &operator>> (QDataStream &s, PSDHeader &header)
     s >> header.color_mode;
     return s;
 }
-static bool seekBy(QDataStream &s, unsigned int bytes)
-{
-    char buf[4096];
-    while (bytes) {
-        unsigned int num = qMin(bytes, (unsigned int)sizeof(buf));
-        unsigned int l = num;
-        s.readRawData(buf, l);
-        if (l != num) {
-            return false;
-        }
-        bytes -= num;
-    }
-    return true;
-}
 
 // Check that the header is a valid PSD.
 static bool IsValid(const PSDHeader &header)
@@ -108,125 +108,107 @@ static bool IsSupported(const PSDHeader &header)
     return true;
 }
 
-// Load the PSD image.
+static void skip_section(QDataStream &s)
-static bool LoadPSD(QDataStream &s, const PSDHeader &header, QImage &img)
 {
-    // Create dst image.
+    quint32 section_length;
-    img = QImage(header.width, header.height, QImage::Format_RGB32);
-
-    uint tmp;
-
     // Skip mode data.
-    s >> tmp;
+    s >> section_length;
-    s.device()->seek(s.device()->pos() + tmp);
+    s.skipRawData(section_length);
+}
 
-    // Skip image resources.
+static quint8 readPixel(QDataStream &stream) {
-    s >> tmp;
+    quint8 pixel;
-    s.device()->seek(s.device()->pos() + tmp);
+    stream >> pixel;
+    return pixel;
+};
+static QRgb updateRed(QRgb oldPixel, quint8 redPixel) {
+    return qRgba(redPixel, qGreen(oldPixel), qBlue(oldPixel), qAlpha(oldPixel));
+};
+static QRgb updateGreen(QRgb oldPixel, quint8 greenPixel) {
+    return qRgba(qRed(oldPixel), greenPixel, qBlue(oldPixel), qAlpha(oldPixel));
+};
+static QRgb updateBlue(QRgb oldPixel, quint8 bluePixel) {
+    return qRgba(qRed(oldPixel), qGreen(oldPixel), bluePixel, qAlpha(oldPixel));
+};
+static QRgb updateAlpha(QRgb oldPixel, quint8 alphaPixel) {
+    return qRgba(qRed(oldPixel), qGreen(oldPixel), qBlue(oldPixel), alphaPixel);
+};
+typedef QRgb(*channelUpdater)(QRgb,quint8);
 
-    // Skip the reserved data.
+// Load the PSD image.
-    s >> tmp;
+static bool LoadPSD(QDataStream &stream, const PSDHeader &header, QImage &img)
-    s.device()->seek(s.device()->pos() + tmp);
+{
+    // Mode data
+    skip_section(stream);
+
+    // Image resources
+    skip_section(stream);
+
+    // Reserved data
+    skip_section(stream);
 
     // Find out if the data is compressed.
     // Known values:
     //   0: no compression
     //   1: RLE compressed
-    ushort compression;
+    quint16 compression;
-    s >> compression;
+    stream >> compression;
 
     if (compression > 1) {
-        // Unknown compression type.
+        qDebug() << "Unknown compression type";
         return false;
     }
 
-    uint channel_num = header.channel_count;
+    quint32 channel_num = header.channel_count;
 
+    QImage::Format fmt = QImage::Format_RGB32;
     // Clear the image.
-    if (channel_num < 4) {
+    if (channel_num >= 4) {
-        img.fill(qRgba(0, 0, 0, 0xFF));
-    } else {
         // Enable alpha.
-        img = img.convertToFormat(QImage::Format_ARGB32);
+        fmt = QImage::Format_ARGB32;
 
         // Ignore the other channels.
         channel_num = 4;
     }
+    img = QImage(header.width, header.height, fmt);
+    img.fill(qRgb(0,0,0));
 
-    const uint pixel_count = header.height * header.width;
+    const quint32 pixel_count = header.height * header.width;
 
-    static const uint components[4] = {2, 1, 0, 3}; // @@ Is this endian dependant?
+    QRgb *image_data = reinterpret_cast<QRgb*>(img.bits());
+
+    static const channelUpdater updaters[4] = {
+        updateRed,
+        updateGreen,
+        updateBlue,
+        updateAlpha
+    };
 
     if (compression) {
-
         // Skip row lengths.
-        if (!seekBy(s, header.height * header.channel_count * sizeof(ushort))) {
+        int skip_count = header.height * header.channel_count * sizeof(quint16);
+        if (stream.skipRawData(skip_count) != skip_count) {
             return false;
         }
 
-        // Read RLE data.
+        for (unsigned short channel = 0; channel < channel_num; channel++) {
-        for (uint channel = 0; channel < channel_num; channel++) {
+            bool success = decodeRLEData(RLEVariant::PackBits, stream,
-
+			                 image_data, pixel_count,
-            uchar *ptr = img.bits() + components[channel];
+					 &readPixel, updaters[channel]);
-
+            if (!success) {
-            uint count = 0;
+                qDebug() << "decodeRLEData on channel" << channel << "failed";
-            while (count < pixel_count) {
-                uchar c;
-                if (s.atEnd()) {
                 return false;
             }
-                s >> c;
-                uint len = c;
-
-                if (len < 128) {
-                    // Copy next len+1 bytes literally.
-                    len++;
-                    count += len;
-                    if (count > pixel_count) {
-                        return false;
-                    }
-
-                    while (len != 0) {
-                        s >> *ptr;
-                        ptr += 4;
-                        len--;
-                    }
-                } else if (len > 128) {
-                    // Next -len+1 bytes in the dest are replicated from next source byte.
-                    // (Interpret len as a negative 8-bit int.)
-                    len ^= 0xFF;
-                    len += 2;
-                    count += len;
-                    if (s.atEnd() || count > pixel_count) {
-                        return false;
-                    }
-                    uchar val;
-                    s >> val;
-                    while (len != 0) {
-                        *ptr = val;
-                        ptr += 4;
-                        len--;
-                    }
-                } else if (len == 128) {
-                    // No-op.
-                }
-            }
         }
     } else {
-        // We're at the raw image data.  It's each channel in order (Red, Green, Blue, Alpha, ...)
+        for (unsigned short channel = 0; channel < channel_num; channel++) {
-        // where each channel consists of an 8-bit value for each pixel in the image.
+            for (unsigned i = 0; i < pixel_count; ++i) {
-
+                image_data[i] = updaters[channel](image_data[i], readPixel(stream));
-        // Read the data by channel.
+            }
-        for (uint channel = 0; channel < channel_num; channel++) {
+            // make sure we didn't try to read past the end of the stream
-
+            if (stream.status() != QDataStream::Ok) {
-            uchar *ptr = img.bits() + components[channel];
+                qDebug() << "DataStream status was" << stream.status();
-
+                return false;
-            // Read the data.
-            uint count = pixel_count;
-            while (count != 0) {
-                s >> *ptr;
-                ptr += 4;
-                count--;
             }
         }
     }

src/imageformats/rle_p.h

@@ -0,0 +1,220 @@
+/*
+ * Run-Length Encoding utilities.
+ * Copyright 2014-2015 Alex Merry <alex.merry@kde.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ * ----------------------------------------------------------------------------
+ */
+
+#ifndef KIMAGEFORMATS_RLE_H
+#define KIMAGEFORMATS_RLE_H
+
+/**
+ * The RLEVariant to use.
+ *
+ * This mostly concerns what to do values >= 128.
+ */
+enum class RLEVariant {
+    /**
+     * PackBits-style RLE
+     *
+     * Value 128 is ignored, 129 indicates a repetition
+     * of size 2, 130 of size 3, up to 255 of size 128.
+     */
+    PackBits,
+    /**
+     * PIC-style RLE
+     *
+     * Value 128 indicates a 16-bit repetition count
+     * follows, while 129 indicates a repetition
+     * of size 128, 130 of size 127, down to 255 of
+     * size 2.
+     */
+    PIC
+};
+
+/**
+ * Decodes data written in run-length encoding format.
+ *
+ * This is intended to be used with lambda functions.
+ *
+ * Note that this functions expects that, at the current location in @p stream,
+ * exactly @p length items have been encoded as a unit (and so it will not be
+ * partway through a run when it has decoded @p length items). If this is not
+ * the case, it will return @c false.
+ *
+ * @param variant     The RLE variant to decode.
+ * @param stream      The stream to read the data from.
+ * @param buf         The location to write the decoded data.
+ * @param length      The number of items to read.
+ * @param readData    A function that takes a QDataStream reference and reads a
+ *                    single value.
+ * @param updateItem  A function that takes an item from @p buf and the result
+ *                    of a readData call, and produces the item that should be
+ *                    written to @p buf.
+ *
+ * @returns @c true if @p length items in mixed RLE were successfully read
+ *          into @p buf, @c false otherwise.
+ */
+template<typename Item, typename Func1, typename Func2>
+static inline bool decodeRLEData(RLEVariant variant,
+                                 QDataStream &stream,
+                                 Item *dest,
+                                 quint16 length,
+                                 Func1 readData,
+                                 Func2 updateItem)
+{
+    unsigned offset = 0; // in dest
+    while (offset < length) {
+        unsigned remaining = length - offset;
+        quint8 count1;
+        stream >> count1;
+
+        if (count1 >= 128u) {
+            unsigned length;
+            if (variant == RLEVariant::PIC) {
+                if (count1 == 128u) {
+                    // If the value is exactly 128, it means that it is more than
+                    // 127 repetitions
+                    quint16 count2;
+                    stream >> count2;
+                    length = count2;
+                } else {
+                    // 2 to 128 repetitions
+                    length = count1 - 127u;
+                }
+            } else if (variant == RLEVariant::PackBits) {
+                if (count1 == 128u) {
+                    // Ignore value 128
+                    continue;
+                } else {
+                    // 128 to 2 repetitions
+                    length = 257u - count1;
+                }
+            } else {
+                Q_ASSERT(false);
+            }
+            if (length > remaining) {
+                qDebug() << "Row overrun:" << length << ">" << remaining;
+                return false;
+            }
+            auto datum = readData(stream);
+            for (unsigned i = offset; i < offset + length; ++i) {
+                dest[i] = updateItem(dest[i], datum);
+            }
+            offset += length;
+        } else {
+            // No repetitions
+            unsigned length = count1 + 1u;
+            if (length > remaining) {
+                qDebug() << "Row overrun:" << length << ">" << remaining;
+                return false;
+            }
+            for (unsigned i = offset; i < offset + length; ++i) {
+                auto datum = readData(stream);
+                dest[i] = updateItem(dest[i], datum);
+            }
+            offset += length;
+        }
+    }
+    if (stream.status() != QDataStream::Ok) {
+        qDebug() << "DataStream status was" << stream.status();
+    }
+    return stream.status() == QDataStream::Ok;
+}
+
+
+/**
+ * Encodes data in run-length encoding format.
+ *
+ * This is intended to be used with lambda functions.
+ *
+ * @param variant     The RLE variant to encode in.
+ * @param stream      The stream to write the data to.
+ * @param data        The data to be written.
+ * @param length      The number of items to write.
+ * @param itemsEqual  A function that takes two items and returns whether
+ *                    @p writeItem would write them identically.
+ * @param writeItem   A function that takes a QDataStream reference and an item
+ *                    and writes the item to the data stream.
+ */
+template<typename Item, typename Func1, typename Func2>
+static inline void encodeRLEData(RLEVariant variant,
+                                 QDataStream &stream,
+		                 const Item *data,
+				 unsigned length,
+				 Func1 itemsEqual,
+				 Func2 writeItem)
+{
+    unsigned offset = 0;
+    const unsigned maxEncodableChunk =
+        (variant == RLEVariant::PIC)
+        ? 65535u
+        : 128;
+    while (offset < length) {
+        const Item *chunkStart = data + offset;
+        unsigned maxChunk = qMin(length - offset, maxEncodableChunk);
+
+        const Item *chunkEnd = chunkStart + 1;
+        quint16 chunkLength = 1;
+        while (chunkLength < maxChunk && itemsEqual(*chunkStart, *chunkEnd)) {
+            ++chunkEnd;
+            ++chunkLength;
+        }
+
+        if (chunkLength > 128) {
+            // Sequence of > 128 identical pixels
+            Q_ASSERT(variant == RLEVariant::PIC);
+            stream << quint8(128);
+            stream << quint16(chunkLength);
+            writeItem(stream, *chunkStart);
+        } else if (chunkLength > 1) {
+            // Sequence of <= 128 identical pixels
+            quint8 encodedLength;
+            if (variant == RLEVariant::PIC) {
+                encodedLength = quint8(chunkLength + 127);
+            } else if (variant == RLEVariant::PackBits) {
+                encodedLength = quint8(257 - chunkLength);
+            } else {
+                Q_ASSERT(false);
+                encodedLength = 0;
+            }
+            stream << encodedLength;
+            writeItem(stream, *chunkStart);
+        } else {
+            // find a string of up to 128 values, each different from the one
+            // that follows it
+            if (maxChunk > 128) {
+                maxChunk = 128;
+            }
+            chunkLength = 1;
+            chunkEnd = chunkStart + 1;
+            while (chunkLength < maxChunk &&
+                    (chunkLength + 1u == maxChunk ||
+                     !itemsEqual(*chunkEnd, *(chunkEnd+1))))
+            {
+                ++chunkEnd;
+                ++chunkLength;
+            }
+            stream << quint8(chunkLength - 1);
+            for (unsigned i = 0; i < chunkLength; ++i) {
+                writeItem(stream, *(chunkStart + i));
+            }
+        }
+        offset += chunkLength;
+    }
+}
+
+#endif // KIMAGEFORMATS_RLE_H