From a6877d5baa65a941225b7e5ed697314255e4edc3 Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Wed, 8 Aug 2018 16:24:49 -0700
Subject: [PATCH] Fix cmgen cross cubmap input

Fixes bug #45
---
 libs/imageio/src/ImageDecoder.cpp | 13 +++---
 tools/cmgen/src/CubemapUtils.cpp  | 25 ++++++++----
 tools/cmgen/src/CubemapUtils.h    |  7 +++-
 tools/cmgen/src/cmgen.cpp         | 68 +++++++++++++++++--------------
 4 files changed, 67 insertions(+), 46 deletions(-)

diff --git a/libs/imageio/src/ImageDecoder.cpp b/libs/imageio/src/ImageDecoder.cpp
index 5b6a8d08d4..95f19b08d7 100644
--- a/libs/imageio/src/ImageDecoder.cpp
+++ b/libs/imageio/src/ImageDecoder.cpp
@@ -1,3 +1,5 @@
+#include <memory>
+
 /*
  * Copyright (C) 2015 The Android Open Source Project
  *
@@ -290,7 +292,7 @@ Image PNGDecoder::decode() {
         uint32_t height = png_get_image_height(mPNG, mInfo);
         size_t rowBytes = png_get_rowbytes(mPNG, mInfo);
 
-        imageData.reset(new uint8_t[height * rowBytes]);
+        imageData = std::make_unique<uint8_t[]>(height * rowBytes);
         std::unique_ptr<png_bytep[]> rowPointers(new png_bytep[height]);
         for (size_t y = 0 ; y < height ; y++) {
             rowPointers[y] = &imageData[y * rowBytes];
@@ -369,8 +371,7 @@ HDRDecoder::HDRDecoder(std::istream& stream)
     : mStream(stream), mStreamStartPos(stream.tellg()) {
 }
 
-HDRDecoder::~HDRDecoder() {
-}
+HDRDecoder::~HDRDecoder() = default;
 
 Image HDRDecoder::decode() {
     try {
@@ -480,8 +481,7 @@ PSDDecoder::PSDDecoder(std::istream& stream)
         : mStream(stream), mStreamStartPos(stream.tellg()) {
 }
 
-PSDDecoder::~PSDDecoder() {
-}
+PSDDecoder::~PSDDecoder() = default;
 
 Image PSDDecoder::decode() {
     #pragma pack(push, 1)
@@ -593,8 +593,7 @@ EXRDecoder::EXRDecoder(std::istream& stream, const std::string& sourceName)
         : mStream(stream), mStreamStartPos(stream.tellg()), mSourceName(sourceName) {
 }
 
-EXRDecoder::~EXRDecoder() {
-}
+EXRDecoder::~EXRDecoder() = default;
 
 Image EXRDecoder::decode() {
     try {
diff --git a/tools/cmgen/src/CubemapUtils.cpp b/tools/cmgen/src/CubemapUtils.cpp
index 52e66af1ff..fe376fda31 100644
--- a/tools/cmgen/src/CubemapUtils.cpp
+++ b/tools/cmgen/src/CubemapUtils.cpp
@@ -81,7 +81,7 @@ void CubemapUtils::equirectangularToCubemap(Cubemap& dst, const image::Image& sr
                 c += Cubemap::sampleAt(src.getSampleRef((uint32_t)xf, (uint32_t)yf));
             }
             c *= iNumSamples;
-            dst.writeAt(data, c);
+            Cubemap::writeAt(data, c);
         }
     });
 }
@@ -92,7 +92,7 @@ void CubemapUtils::downsampleCubemapLevelBoxFilter(Cubemap& dst, const Cubemap&
             [&](EmptyState&, size_t y, Cubemap::Face f, Cubemap::Texel* data, size_t dim) {
         const image::Image& image(src.getImageForFace(f));
         for (size_t x=0 ; x<dim ; ++x, ++data) {
-            dst.writeAt(data, Cubemap::filterAt(image, x*scale+0.5, y*scale+0.5));
+            Cubemap::writeAt(data, Cubemap::filterAt(image, x*scale+0.5, y*scale+0.5));
         }
     });
 }
@@ -146,9 +146,13 @@ void CubemapUtils::setAllFacesFromCross(Cubemap& cm, const Image& image) {
     CubemapUtils::setFaceFromCross(cm, Cubemap::Face::PZ, image);
 }
 
-Image CubemapUtils::createCubemapImage(size_t dim) {
+Image CubemapUtils::createCubemapImage(size_t dim, bool horizontal) {
     size_t width = 4 * dim;
     size_t height = 3 * dim;
+    if (!horizontal) {
+        std::swap(width, height);
+    }
+
     // always allocate an extra column and row, to allow the cubemap to be "seamless"
     size_t bpr = (width + 1) * sizeof(Cubemap::Texel);
     bpr = (bpr + 31) & ~31;
@@ -170,20 +174,27 @@ std::string CubemapUtils::getFaceName(Cubemap::Face face) {
     }
 }
 
-Cubemap CubemapUtils::create(Image& image, size_t dim) {
+Cubemap CubemapUtils::create(Image& image, size_t dim, bool horizontal) {
     Cubemap cm(dim);
-    Image temp(CubemapUtils::createCubemapImage(dim));
+    Image temp(CubemapUtils::createCubemapImage(dim, horizontal));
     CubemapUtils::setAllFacesFromCross(cm, temp);
     std::swap(image, temp);
     return cm;
 }
 
+void CubemapUtils::copyImage(Image& dst, const Image& src) {
+    assert(dst.getWidth() >= src.getWidth() && dst.getHeight() >= src.getHeight());
+    for (size_t y = 0, my = src.getHeight(); y < my; ++y) {
+        memcpy(dst.getPixelRef(0, y), src.getPixelRef(0, y), src.getBytesPerRow());
+    }
+}
+
 void CubemapUtils::mirrorCubemap(Cubemap& dst, const Cubemap& src) {
     process<EmptyState>(dst,
             [&](EmptyState&, size_t y, Cubemap::Face f, Cubemap::Texel* data, size_t dim) {
         for (size_t x=0 ; x<dim ; ++x, ++data) {
             const double3 N(dst.getDirectionFor(f, x, y));
-            dst.writeAt(data, src.sampleAt(N * double3{ -N.x, N.y, N.z }));
+            Cubemap::writeAt(data, src.sampleAt(N * double3{ -N.x, N.y, N.z }));
         }
     });
 }
@@ -204,7 +215,7 @@ void CubemapUtils::generateUVGrid(Cubemap const& cml, size_t gridFrequency) {
                 for (size_t x = 0; x < dim; ++x, ++data) {
                     bool grid = bool(((x / gridSize) ^ (y / gridSize)) & 1);
                     Cubemap::Texel t = grid ? colors[(int)f] : 0;
-                    cml.writeAt(data, t);
+                    Cubemap::writeAt(data, t);
                 }
             });
 }
diff --git a/tools/cmgen/src/CubemapUtils.h b/tools/cmgen/src/CubemapUtils.h
index 6e2f407752..c87fa0870c 100644
--- a/tools/cmgen/src/CubemapUtils.h
+++ b/tools/cmgen/src/CubemapUtils.h
@@ -60,7 +60,10 @@ public:
     static std::string getFaceName(Cubemap::Face face);
 
     // Create a cubemap object and its backing Image
-    static Cubemap create(image::Image& image, size_t dim);
+    static Cubemap create(image::Image& image, size_t dim, bool horizontal = true);
+
+    // Copy an image
+    static void copyImage(image::Image& dst, const image::Image& src);
 
     // Sets a Cubemap faces from a cross image
     static void setAllFacesFromCross(Cubemap& cm, const image::Image& image);
@@ -73,7 +76,7 @@ public:
 
 private:
     static void setFaceFromCross(Cubemap& cm, Cubemap::Face face, const image::Image& image);
-    static image::Image createCubemapImage(size_t dim);
+    static image::Image createCubemapImage(size_t dim, bool horizontal = true);
 };
 
 // -----------------------------------------------------------------------------------------------
diff --git a/tools/cmgen/src/cmgen.cpp b/tools/cmgen/src/cmgen.cpp
index 1d080aacb5..41f3295c46 100644
--- a/tools/cmgen/src/cmgen.cpp
+++ b/tools/cmgen/src/cmgen.cpp
@@ -45,7 +45,7 @@ enum class ShFile {
     SH_NONE, SH_CROSS, SH_TEXT
 };
 static image::ImageEncoder::Format g_format = image::ImageEncoder::Format::PNG;
-static std::string g_compression = "";
+static std::string g_compression;
 static bool g_extract_faces = false;
 static double g_extract_blur = 0.0;
 static utils::Path g_extract_dir;
@@ -137,7 +137,7 @@ static void printUsage(char* name) {
             "   --mirror\n"
             "       Mirrors generated cubemaps for reflections\n\n"
             "   --ibl-samples=numSamples\n"
-            "       Number of samples tu use for IBL integrations (default 1024)\n\n"
+            "       Number of samples to use for IBL integrations (default 1024)\n\n"
             "\n"
             "Private use only:\n"
             "   --ibl-dfg=filename.[exr|hdr|psd|png|rgbm|dds|h|hpp|c|cpp|inc|txt]\n"
@@ -175,27 +175,27 @@ static void license() {
 static int handleCommandLineArgments(int argc, char* argv[]) {
     static constexpr const char* OPTSTR = "hqidf:c:s:x:";
     static const struct option OPTIONS[] = {
-            { "help",                       no_argument, 0, 'h' },
-            { "license",                    no_argument, 0, 'l' },
-            { "quiet",                      no_argument, 0, 'q' },
-            { "format",               required_argument, 0, 'f' },
-            { "compression",          required_argument, 0, 'c' },
-            { "size",                 required_argument, 0, 's' },
-            { "extract",              required_argument, 0, 'e' },
-            { "extract-blur",         required_argument, 0, 'r' },
-            { "sh",                   optional_argument, 0, 'z' },
-            { "sh-output",            required_argument, 0, 'o' },
-            { "sh-irradiance",              no_argument, 0, 'i' },
-            { "sh-shader",                  no_argument, 0, 'b' },
-            { "ibl-is-mipmap",        required_argument, 0, 'y' },
-            { "ibl-ld",               required_argument, 0, 'p' },
-            { "ibl-dfg",              required_argument, 0, 'a' },
-            { "ibl-dfg-multiscatter",       no_argument, 0, 'u' },
-            { "ibl-samples",          required_argument, 0, 'k' },
-            { "deploy",               required_argument, 0, 'x' },
-            { "mirror",                     no_argument, 0, 'm' },
-            { "debug",                      no_argument, 0, 'd' },
-            { 0, 0, 0, 0 }  // termination of the option list
+            { "help",                       no_argument, nullptr, 'h' },
+            { "license",                    no_argument, nullptr, 'l' },
+            { "quiet",                      no_argument, nullptr, 'q' },
+            { "format",               required_argument, nullptr, 'f' },
+            { "compression",          required_argument, nullptr, 'c' },
+            { "size",                 required_argument, nullptr, 's' },
+            { "extract",              required_argument, nullptr, 'e' },
+            { "extract-blur",         required_argument, nullptr, 'r' },
+            { "sh",                   optional_argument, nullptr, 'z' },
+            { "sh-output",            required_argument, nullptr, 'o' },
+            { "sh-irradiance",              no_argument, nullptr, 'i' },
+            { "sh-shader",                  no_argument, nullptr, 'b' },
+            { "ibl-is-mipmap",        required_argument, nullptr, 'y' },
+            { "ibl-ld",               required_argument, nullptr, 'p' },
+            { "ibl-dfg",              required_argument, nullptr, 'a' },
+            { "ibl-dfg-multiscatter",       no_argument, nullptr, 'u' },
+            { "ibl-samples",          required_argument, nullptr, 'k' },
+            { "deploy",               required_argument, nullptr, 'x' },
+            { "mirror",                     no_argument, nullptr, 'm' },
+            { "debug",                      no_argument, nullptr, 'd' },
+            { nullptr, 0, 0, 0 }  // termination of the option list
     };
     int opt;
     int option_index = 0;
@@ -395,6 +395,11 @@ int main(int argc, char* argv[]) {
             std::cerr << "Unsupported image format!" << std::endl;
             exit(0);
         }
+        if (inputImage.getChannelsCount() != 3) {
+            std::cerr << "Input image must be RGB (3 channels)! This image has "
+                      << inputImage.getChannelsCount() << " channels." << std::endl;
+            exit(0);
+        }
 
         CubemapUtils::clamp(inputImage);
 
@@ -404,15 +409,18 @@ int main(int argc, char* argv[]) {
         if ((isPOT(width) && (width * 3 == height * 4)) ||
             (isPOT(height) && (height * 3 == width * 4))) {
             // This is cross cubemap
+            const bool isHorizontal = width > height;
             size_t dim = std::max(height, width) / 4;
-            levels.push_back(Cubemap(dim));
             if (!g_quiet) {
                 std::cout << "Loading cross... " << std::endl;
             }
-            Cubemap& base(levels[0]);
-            CubemapUtils::setAllFacesFromCross(base, inputImage);
-            base.makeSeamless();
-            images.push_back(std::move(inputImage));
+
+            Image temp;
+            Cubemap cml = CubemapUtils::create(temp, dim, isHorizontal);
+            CubemapUtils::copyImage(temp, inputImage);
+            cml.makeSeamless();
+            images.push_back(std::move(temp));
+            levels.push_back(std::move(cml));
         } else if (width == 2 * height) {
             // we assume a spherical (equirectangular) image, which we will convert to a cross image
             size_t dim = g_output_size ? g_output_size : 256;
@@ -429,8 +437,8 @@ int main(int argc, char* argv[]) {
             std::cerr << "Aspect ratio not supported: " << width << "x" << height << std::endl;
             std::cerr << "Supported aspect ratios:" << std::endl;
             std::cerr << "  2:1, lat/long or equirectangular" << std::endl;
-            std::cerr << "  3:4, vertical cross (width must be power of two)" << std::endl;
-            std::cerr << "  4:3, horizontal cross (height must be power of two)" << std::endl;
+            std::cerr << "  3:4, vertical cross (height must be power of two)" << std::endl;
+            std::cerr << "  4:3, horizontal cross (width must be power of two)" << std::endl;
             exit(0);
         }
     } else {