2474: Fix FreeImage texture loader bugs, add test of image contents

The main one was not calling FreeImage_ConvertTo24Bits() on 32-bit
input images, but the std::memcpy() was also wrong.

Fixes #2474

common/src/Assets/Texture.cpp

@@ -240,6 +240,18 @@ namespace TrenchBroom {
             }
         }
 
+        const TextureBuffer::List& Texture::buffersIfUnprepared() const {
+            return m_buffers;
+        }
+
+        GLenum Texture::format() const {
+            return m_format;
+        }
+
+        TextureType Texture::type() const {
+            return m_type;
+        }
+
         void Texture::setCollection(TextureCollection* collection) {
             m_collection = collection;
         }

common/src/Assets/Texture.h

@@ -86,6 +86,19 @@ namespace TrenchBroom {
 
             void activate() const;
             void deactivate() const;
+
+        public: // exposed for tests only
+            /**
+             * Returns the texture data in the format returned by format().
+             * Once prepare() is called, this will be an empty vector.
+             */
+            const TextureBuffer::List& buffersIfUnprepared() const;
+            /**
+             * Will be one of GL_RGB, GL_BGR, GL_RGBA.
+             */
+            GLenum format() const;
+            TextureType type() const;
+
         private:
             void setCollection(TextureCollection* collection);
             friend class TextureCollection;

common/src/IO/FreeImageTextureReader.cpp

@@ -53,20 +53,48 @@ namespace TrenchBroom {
             Assets::setMipBufferSize(buffers, m_mipCount, imageWidth, imageHeight, format);
 
             // TODO: Alpha channel seems to be unsupported by the Texture class
-            if (imageColourType != FIC_RGB) {
+
+            const auto inputBytesPerPixel = FreeImage_GetLine(image) / FreeImage_GetWidth(image);
+            if (imageColourType != FIC_RGB || inputBytesPerPixel != 3) {
                 FIBITMAP* tempImage = FreeImage_ConvertTo24Bits(image);
                 FreeImage_Unload(image);
                 image = tempImage;
             }
 
             FreeImage_FlipVertical(image);
 
-            std::memcpy(buffers[0].ptr(), FreeImage_GetBits(image), buffers[0].size());
-            for (size_t mip = 1; mip < m_mipCount; ++mip) {
+            for (size_t mip = 0; mip < m_mipCount; ++mip) {
                 const auto mipSize = Assets::sizeAtMipLevel(imageWidth, imageHeight, mip);
-                FIBITMAP* mipImage = FreeImage_Rescale(image, static_cast<int>(mipSize.x()), static_cast<int>(mipSize.y()), FILTER_BICUBIC);
-                std::memcpy(buffers[mip].ptr(), FreeImage_GetBits(mipImage), buffers[mip].size());
-                FreeImage_Unload(mipImage);
+
+                FIBITMAP* mipImage;
+                if (mip > 0) {
+                    mipImage = FreeImage_Rescale(image, static_cast<int>(mipSize.x()), static_cast<int>(mipSize.y()), FILTER_BICUBIC);
+                } else {
+                    mipImage = image;
+                }
+
+                const auto bytesPerPixel = FreeImage_GetLine(mipImage) / FreeImage_GetWidth(mipImage);
+                ensure(bytesPerPixel == 3, "expected to have converted image to 24-bit");
+
+                unsigned char* outBytes = buffers.at(mip).ptr();
+
+                for (size_t y = 0; y < mipSize.y(); ++y) {
+                    BYTE* const scanline = FreeImage_GetScanLine(mipImage, static_cast<int>(y));
+
+                    BYTE* inPixel = scanline;
+                    for (size_t x = 0; x < mipSize.x(); ++x) {
+
+                        *(outBytes++) = inPixel[FI_RGBA_BLUE];
+                        *(outBytes++) = inPixel[FI_RGBA_GREEN];
+                        *(outBytes++) = inPixel[FI_RGBA_RED];
+
+                        inPixel += 3;
+                    }
+                }
+
+                if (mip > 0) {
+                    FreeImage_Unload(mipImage);
+                }
             }
 
             FreeImage_Unload(image);

test/src/IO/FreeImageTextureReaderTest.cpp

@@ -49,5 +49,53 @@ namespace TrenchBroom {
             assertTexture("5x5.png",          5,   5,   diskFS, textureLoader);
             assertTexture("707x710.png",      707, 710, diskFS, textureLoader);
         }
+
+        // https://github.com/kduske/TrenchBroom/issues/2474
+        TEST(FreeImageTextureReaderTest, testPNGContents) {
+            DiskFileSystem fs(IO::Disk::getCurrentWorkingDir());
+
+            TextureReader::TextureNameStrategy nameStrategy;
+            const auto mips = 4;
+            FreeImageTextureReader textureLoader(nameStrategy, mips);
+
+            const Path imagePath = Disk::getCurrentWorkingDir() + Path("data/IO/Image/");
+            DiskFileSystem diskFS(imagePath);
+
+            const Assets::Texture* texture = textureLoader.readTexture(diskFS.openFile(Path("pngContentsTest.png")));
+            ASSERT_TRUE(texture != nullptr);
+            ASSERT_EQ(64, texture->width());
+            ASSERT_EQ(64, texture->height());
+            ASSERT_EQ(4, texture->buffersIfUnprepared().size());
+            ASSERT_EQ(GL_BGR, texture->format());
+
+            auto& mip0Data = texture->buffersIfUnprepared().at(0);
+            ASSERT_EQ(64 * 64 * 3, mip0Data.size());
+
+            const auto w = 64;
+            const auto h = 64;
+
+            for (int y = 0; y < h; ++y) {
+                for (int x = 0; x < w; ++x) {
+                    if (x == 0 && y == 0) {
+                        // top left pixel is red
+                        ASSERT_EQ(0,   mip0Data[w * y + x * 3]);
+                        ASSERT_EQ(0,   mip0Data[w * y + x * 3 + 1]);
+                        ASSERT_EQ(255, mip0Data[w * y + x * 3 + 2]);
+                    } else if (x == w && y == h) {
+                        // bottom right pixel is green
+                        ASSERT_EQ(0,   mip0Data[w * y + x * 3]);
+                        ASSERT_EQ(255, mip0Data[w * y + x * 3 + 1]);
+                        ASSERT_EQ(0,   mip0Data[w * y + x * 3 + 2]);
+                    } else {
+                        // others are 161, 161, 161
+                        ASSERT_EQ(161, mip0Data[w * y + x * 3]);
+                        ASSERT_EQ(161, mip0Data[w * y + x * 3 + 1]);
+                        ASSERT_EQ(161, mip0Data[w * y + x * 3 + 2]);
+                    }
+                }
+            }
+
+            delete texture;
+        }
     }
 }