Refactoring image loading/manipulation class

Question

I have this Image class that I've been using for some time now on personal projects and games I eventually write. It was written in C++98 style and it is starting to show its age. Over time, I added a few C++11 elements here and there, but I think it is now due for a major refactoring.

image.hpp:

#ifndef IMAGE_HPP
#define IMAGE_HPP

// Dependencies:
#include <string>
#include <cstdint>

// ======================================================
// PixelFormat:
// ======================================================

//
// Supported internal image and texture formats.
//
struct PixelFormat
{
    enum Enum
    {
        // RGB images are composed of 3 color components.
        // Each component can be a byte [0,255] or a normalized float [0,1].
        RgbU8,
        RgbF32,

        // RGBA images are composed of 3 color components and
        // 1 component for the transparency level (alpha).
        // Each component can be a byte [0,255] or a normalized float [0,1].
        RgbaU8,
        RgbaF32,

        // Sentinel value. Not a valid pixel format. Internal use.
        Invalid = -1
    };

    // Converts a pixel format enum value to a printable string, for debugging purposes.
    static const char * toString(PixelFormat::Enum pixelFormat);

    // Returns the size in bytes of a single pixel of a given pixel format.
    static size_t sizeBytes(PixelFormat::Enum pixelFormat);

    // Returns the number of color components of a pixel format.
    static uint32_t componentCount(PixelFormat::Enum pixelFormat);
};

// ======================================================
// TPixel templates:
// ======================================================

// RGB pixel:
template<class T>
struct TPixel3
{
    T r;
    T g;
    T b;
};

// RGBA pixel:
template<class T>
struct TPixel4
{
    T r;
    T g;
    T b;
    T a;
};

// ======================================================
// Image:
// ======================================================

//
// Represents and array of pixels.
// Images are most commonly loaded from image files.
//
class Image
{
public:

    // Default constructor; no image loaded.
    Image();

    // Construct from a deep copy.
    Image(const Image & other);

    // Assignment operator creates a deep copy of the source image.
    Image & operator = (const Image & other);

    // Tries to load an image from a file.
    // If the image fails to load and 'errorMessage' is not null, a small error description is returned in it.
    bool loadFromFile(const std::string & filename, std::string * errorMessage = nullptr);

    // Creates an uncompressed image of any size, filled with the given color.
    // Provided data pointer must match pixel format size and type.
    void makeColorFilledImage(uint32_t w, uint32_t h, PixelFormat::Enum pf, const uint8_t * color);

    // Creates a RgbaU8 64x64 checkerboard pattern image.
    // The number of checker squares may range from 2 to 64, as long as it is always a power-of-two.
    void makeCheckerPatternImage(uint32_t numSquares);

    // Allocate image data storage:
    uint8_t * allocImageStorage(size_t dataSize, uint32_t w, uint32_t h, PixelFormat::Enum pf);

    // Free previously allocated image data.
    void freeImageStorage();

    // Get the width of the image in pixels.
    uint32_t getWidth() const { return (width); }

    // Get the height of the image in pixels.
    uint32_t getHeight() const { return (height); }

    // Get the internal pixel format of the image.
    PixelFormat::Enum getFormat() const { return pixelFormat; }

    // Number of color components for this image.
    uint32_t getNumComponents() const { return PixelFormat::componentCount(pixelFormat); }

    // Get the number of bytes allocated for image data.
    size_t getDataSizeBytes() const { return dataSizeBytes; }

    // Grab a pointer to the system memory image data.
    template<class PixelType> PixelType * getDataPtr() const { return reinterpret_cast<PixelType *>(data); }

    // Check if the image width and height are both powers of two.
    bool isPowerOfTwo() const;

    // Validate image dimensions/size/format and data pointers.
    bool isValid() const;

    // Get a pixel at a given coord. 'pixel' must be big enough to hold data of one pixel.
    void getPixelAt(uint32_t x, uint32_t y, uint8_t * pixel) const;

    // Set a pixel at a given coord.
    void setPixelAt(uint32_t x, uint32_t y, const uint8_t * pixel);

    // Swaps the pixel at (x0,y0) with the pixel at (x1,y1).
    void swapPixels(uint32_t x0, uint32_t y0, uint32_t x1, uint32_t y1);

    // Applies the user supplied function to every pixel of this uncompressed image.
    void doForEveryPixel(void (* func)(uint8_t *, PixelFormat::Enum));

    // Flips this image vertically in-place without a copy.
    void flipVInPlace();

    // Flips this image horizontally in-place without a copy.
    void flipHInPlace();

    // Creates a vertically flipped copy of this image.
    void flipV(Image & destImage) const;

    // Creates a horizontally flipped copy of this image.
    void flipH(Image & destImage) const;

    // Resizes this image, in place.
    void resizeInPlace(uint32_t targetWidth, uint32_t targetHeight);

    // Creates a resized copy of this image.
    void resize(Image & destImage, uint32_t targetWidth, uint32_t targetHeight) const;

    // Rounds this image down to its nearest power for 2.
    void roundDownToPowerOfTwo();

    // Swizzle the fist and third color channels of the image,
    // effectively changing it from RGB[A] to BGR[A] and vice versa.
    void swizzleRGB();

    // Discard the alpha component of an RGBA/BGRA image.
    void discardAlphaComponent();

    // Set every byte of the image to the given value.
    // Equivalent to C's memset() function.
    void memSet(int val);

    // Copy a portion of this image to the destination image.
    void copyRect(Image & destImage, uint32_t xOffset, uint32_t yOffset, uint32_t rectWidth, uint32_t rectHeight, bool topLeft) const;

    // Set a portion of this image from a source image.
    void setRect(const Image & srcImage, uint32_t xOffset, uint32_t yOffset, uint32_t rectWidth, uint32_t rectHeight, bool topLeft);

    // setRect() overload that takes a raw pointer as data source.
    // Data is assumed to be of the same pixel format as this image.
    void setRect(const uint8_t * image, uint32_t xOffset, uint32_t yOffset, uint32_t rectWidth, uint32_t rectHeight, bool topLeft);

    // Image destructor releases any allocated memory.
    ~Image();

private:

    // Construct an Image from a copy.
    void initFromCopy(const Image & img);

    // Move 'img' to this image. 'img' is invalidated after that.
    void moveCopy(Image & img);

private:

    // Pointer to in memory image data, compressed or uncompressed.
    uint8_t * data;

    // Size in bytes of the 'data' memory buffer.
    size_t dataSizeBytes;

    // Width and height of image, in pixels.
    uint32_t width, height;

    // Internal format of the image pixels.
    PixelFormat::Enum pixelFormat;
};

#endif // IMAGE_HPP

image.cpp:

#include "image.hpp"
#include <cassert>
#include <cstring>
#include <cstdlib>
#include <algorithm>

// ======================================================
// PixelFormat:
// ======================================================

const char * PixelFormat::toString(const PixelFormat::Enum pixelFormat)
{
    switch (pixelFormat)
    {
    case PixelFormat::Invalid : return "Invalid/Undefined";
    case PixelFormat::RgbU8   : return "RgbU8";
    case PixelFormat::RgbF32  : return "RgbF32";
    case PixelFormat::RgbaU8  : return "RgbaU8";
    case PixelFormat::RgbaF32 : return "RgbaF32";
    default : assert(false && "Invalid pixel format!");
    } // switch (pixelFormat)
}

size_t PixelFormat::sizeBytes(const PixelFormat::Enum pixelFormat)
{
    switch (pixelFormat)
    {
    case PixelFormat::Invalid : return 0; // Null value. Not used
    case PixelFormat::RgbU8   : return sizeof(uint8_t) * 3;
    case PixelFormat::RgbF32  : return sizeof(float)   * 3;
    case PixelFormat::RgbaU8  : return sizeof(uint8_t) * 4;
    case PixelFormat::RgbaF32 : return sizeof(float)   * 4;
    default : assert(false && "Invalid pixel format!");
    } // switch (pixelFormat)
}

uint32_t PixelFormat::componentCount(const PixelFormat::Enum pixelFormat)
{
    switch (pixelFormat)
    {
    case PixelFormat::Invalid : return 0;
    case PixelFormat::RgbU8   : return 3;
    case PixelFormat::RgbF32  : return 3;
    case PixelFormat::RgbaU8  : return 4;
    case PixelFormat::RgbaF32 : return 4;
    default : assert(false && "Invalid pixel format!");
    } // switch (pixelFormat)
}

// ======================================================
// Local helpers:
// ======================================================

namespace {

// STB Image library (header only library).
// (NO_HDR to disable the High Dynamic Range (hdr) image loader).
#define STBI_NO_HDR              1
#define STB_IMAGE_IMPLEMENTATION 1
#include "stb_image.h"

// ======================================================

// A big value, enough to hold the largest pixel size imaginable and more.
// Used internally for allocation-less image resizing/transforming.
// The largest pixel format we deal with is RGBA float, which is just 16bytes wide.
constexpr uint32_t bigPixelSizeBytes = 256;

// Round an integer to a power-of-two that is not greater than 'x'.
template<class T>
T floorPowerOfTwo(const T x)
{
    T p2;
    for (p2 = 1; (p2 * 2) <= x; p2 <<= 1)
    {
        // Next...
    }
    return p2;
}

} // namespace {}

// ======================================================
// Image:
// ======================================================

Image::Image()
    : data(nullptr)
    , dataSizeBytes(0)
    , width(0)
    , height(0)
    , pixelFormat(PixelFormat::Invalid)
{
}

Image::Image(const Image & other)
    : data(nullptr)
    , dataSizeBytes(0)
    , width(0)
    , height(0)
    , pixelFormat(PixelFormat::Invalid)
{
    initFromCopy(other);
}

Image & Image::operator = (const Image & other)
{
    freeImageStorage();
    initFromCopy(other);
    return *this;
}

Image::~Image()
{
    freeImageStorage();
}

bool Image::loadFromFile(const std::string & filename, std::string * errorMessage)
{
    assert(!filename.empty());
    assert(data == nullptr && "Creating new image would cause a memory leak!");

    int w, h, comp;
    data = reinterpret_cast<uint8_t *>(stbi_load(filename.c_str(), &w, &h, &comp, /* req_comp = */ 0));

    if (data == nullptr)
    {
        if (errorMessage != nullptr)
        {
            errorMessage->assign("\'stbi_load()\' failed with error: ");
            errorMessage->append(stbi_failure_reason());
        }
        return false;
    }

    switch (comp)
    {
    case 3 : // RGB
        pixelFormat = PixelFormat::RgbU8;
        break;

    case 4 : // RGBA
        pixelFormat = PixelFormat::RgbaU8;
        break;

    default : // Unsupported
        {
            std::free(data);
            data = nullptr;
            if (errorMessage != nullptr)
            {
                errorMessage->assign(filename);
                errorMessage->append(" image format not supported! comp = ");
                errorMessage->append(std::to_string(comp));
            }
            return false;
        }
    } // switch (comp)

    // Non power-of-2 dimensions check:
    if (((w & (w - 1)) != 0) || ((h & (h - 1)) != 0))
    {
        // NOTE: This should be improved in the future.
        // We should define a configuration parameter that enables
        // downscaling a non-POT image.
        //
        if (errorMessage != nullptr)
        {
            errorMessage->assign(filename);
            errorMessage->append(" image does not have power-of-2 dimensions!");
        }
        // Allow it to continue...
    }

    // Set dimensions/size and we are done.
    width  = w;
    height = h;
    dataSizeBytes = (w * h * comp);

    return true;
}

void Image::makeColorFilledImage(const uint32_t w, const uint32_t h, const PixelFormat::Enum pf, const uint8_t * color)
{
    assert(color != nullptr);

    const size_t pixelSize = PixelFormat::sizeBytes(pf);
    if ((width != w) || (height != h) || (pixelFormat != pf))
    {
        freeImageStorage();
        allocImageStorage((w * h * pixelSize), w, h, pf);
    }

    for (uint32_t y = 0; y < h; ++y)
    {
        for (uint32_t x = 0; x < w; ++x)
        {
            std::memcpy((data + (x + y * w) * pixelSize), color, pixelSize);
        }
    }
}

void Image::makeCheckerPatternImage(const uint32_t numSquares)
{
    assert(numSquares >= 2 && numSquares <= 64);

    const uint32_t imgSize = 64; // Image dimensions (w & h) in pixels.
    const uint32_t checkerSize = (imgSize / numSquares); // Size of one checker square, in pixels.

    // One square black and one white:
    const uint8_t blackWhite[][4] = {
        { 0,   0,   0,   255 },
        { 255, 255, 255, 255 }
    };

    freeImageStorage();
    allocImageStorage((imgSize * imgSize * 4), imgSize, imgSize, PixelFormat::RgbaU8); // RGBA image

    uint32_t startY    = 0;
    uint32_t lastColor = 0;
    uint32_t color, rowX;

    while (startY < imgSize)
    {
        for (uint32_t y = startY; y < (startY + checkerSize); ++y)
        {
            color = lastColor, rowX = 0;

            for (uint32_t x = 0; x < imgSize; ++x)
            {
                if (rowX == checkerSize)
                {
                    // Invert color every time we complete a checker box:
                    color = !color;
                    rowX = 0;
                }

                reinterpret_cast<uint32_t *>(data)[x + y * width] =
                        (*reinterpret_cast<const uint32_t *>(blackWhite[color]));

                ++rowX;
            }
        }

        startY += checkerSize;
        lastColor = !lastColor;
    }
}

bool Image::isPowerOfTwo() const
{
    return (((width & (width  - 1)) == 0)
        && ((height & (height - 1)) == 0));
}

bool Image::isValid() const
{
    return (width != 0) && (height != 0) &&
        (pixelFormat != PixelFormat::Invalid) &&
        (data != nullptr) && (dataSizeBytes != 0);
}

uint8_t * Image::allocImageStorage(const size_t dataSize, const uint32_t w, const uint32_t h, const PixelFormat::Enum pf)
{
    assert(data == nullptr && "freeImageStorage() before allocating new one!");

    // Validate parameters:
    assert(w > 0 && h > 0);
    assert(dataSize != 0);
    assert(pf != PixelFormat::Invalid);

    // Save parameters:
    width         = w;
    height        = h;
    pixelFormat   = pf;
    dataSizeBytes = dataSize;

    // Allocate system memory for image data:
    data = reinterpret_cast<uint8_t *>(std::malloc(dataSizeBytes));
    return data;
}

void Image::freeImageStorage()
{
    if (data != nullptr) // Don't waste time nor print message if no data is present:
    {
        std::free(data);

        data          = nullptr;
        dataSizeBytes = 0;
        width         = 0;
        height        = 0;
        pixelFormat   = PixelFormat::Invalid;
    }
}

void Image::initFromCopy(const Image & img)
{
    // Allocate storage:
    allocImageStorage(img.dataSizeBytes, img.width, img.height, img.pixelFormat);

    // Copy 'img' data to this image:
    std::memcpy(data, img.data, img.dataSizeBytes);
}

void Image::moveCopy(Image & img)
{
    // Move:
    data              = img.data;
    dataSizeBytes     = img.dataSizeBytes;
    width             = img.width;
    height            = img.height;
    pixelFormat       = img.pixelFormat;

    // Invalidate the other:
    img.data          = nullptr;
    img.dataSizeBytes = 0;
    img.width         = 0;
    img.height        = 0;
    img.pixelFormat   = PixelFormat::Invalid;
}

void Image::getPixelAt(const uint32_t x, const uint32_t y, uint8_t * pixel) const
{
    assert(isValid());
    assert(x < width);
    assert(y < height);
    assert(pixel != nullptr);

    switch (pixelFormat)
    {
    case PixelFormat::RgbU8 :
        {
            const TPixel3<uint8_t> * rgb8 = &reinterpret_cast<const TPixel3<uint8_t> *>(data)[x + y * width];
            (*reinterpret_cast<TPixel3<uint8_t> *>(pixel)).r = rgb8->r;
            (*reinterpret_cast<TPixel3<uint8_t> *>(pixel)).g = rgb8->g;
            (*reinterpret_cast<TPixel3<uint8_t> *>(pixel)).b = rgb8->b;
            break;
        }
    case PixelFormat::RgbF32 :
        {
            const TPixel3<float> * rgb32 = &reinterpret_cast<const TPixel3<float> *>(data)[x + y * width];
            (*reinterpret_cast<TPixel3<float> *>(pixel)).r = rgb32->r;
            (*reinterpret_cast<TPixel3<float> *>(pixel)).g = rgb32->g;
            (*reinterpret_cast<TPixel3<float> *>(pixel)).b = rgb32->b;
            break;
        }
    case PixelFormat::RgbaU8 :
        {
            // 4-bytes long, use an unsigned integer instead:
            (*reinterpret_cast<uint32_t *>(pixel)) =
                reinterpret_cast<const uint32_t *>(data)[x + y * width];
            break;
        }
    case PixelFormat::RgbaF32 :
        {
            const TPixel4<float> * rgba32 = &reinterpret_cast<const TPixel4<float> *>(data)[x + y * width];
            (*reinterpret_cast<TPixel4<float> *>(pixel)).r = rgba32->r;
            (*reinterpret_cast<TPixel4<float> *>(pixel)).g = rgba32->g;
            (*reinterpret_cast<TPixel4<float> *>(pixel)).b = rgba32->b;
            (*reinterpret_cast<TPixel4<float> *>(pixel)).a = rgba32->a;
            break;
        }
    default:
        assert(false && "Bad image pixel format!");
    } // switch (pixelFormat)
}

void Image::setPixelAt(const uint32_t x, const uint32_t y, const uint8_t * pixel)
{
    assert(isValid());
    assert(x < width);
    assert(y < height);
    assert(pixel != nullptr);

    switch (pixelFormat)
    {
    case PixelFormat::RgbU8 :
        {
            TPixel3<uint8_t> * rgb8 = &reinterpret_cast<TPixel3<uint8_t> *>(data)[x + y * width];
            rgb8->r = (*reinterpret_cast<const TPixel3<uint8_t> *>(pixel)).r;
            rgb8->g = (*reinterpret_cast<const TPixel3<uint8_t> *>(pixel)).g;
            rgb8->b = (*reinterpret_cast<const TPixel3<uint8_t> *>(pixel)).b;
            break;
        }
    case PixelFormat::RgbF32 :
        {
            TPixel3<float> * rgb32 = &reinterpret_cast<TPixel3<float> *>(data)[x + y * width];
            rgb32->r = (*reinterpret_cast<const TPixel3<float> *>(pixel)).r;
            rgb32->g = (*reinterpret_cast<const TPixel3<float> *>(pixel)).g;
            rgb32->b = (*reinterpret_cast<const TPixel3<float> *>(pixel)).b;
            break;
        }
    case PixelFormat::RgbaU8 :
        {
            // 4-bytes long, use an unsigned integer instead:
            reinterpret_cast<uint32_t *>(data)[x + y * width] =
                (*reinterpret_cast<const uint32_t *>(pixel));
            break;
        }
    case PixelFormat::RgbaF32 :
        {
            TPixel4<float> * rgba32 = &reinterpret_cast<TPixel4<float> *>(data)[x + y * width];
            rgba32->r = (*reinterpret_cast<const TPixel4<float> *>(pixel)).r;
            rgba32->g = (*reinterpret_cast<const TPixel4<float> *>(pixel)).g;
            rgba32->b = (*reinterpret_cast<const TPixel4<float> *>(pixel)).b;
            rgba32->a = (*reinterpret_cast<const TPixel4<float> *>(pixel)).a;
            break;
        }
    default:
        assert(false && "Bad image pixel format!");
    } // switch (pixelFormat)
}

void Image::swapPixels(const uint32_t x0, const uint32_t y0, const uint32_t x1, const uint32_t y1)
{
    uint8_t tmpPixel0[bigPixelSizeBytes];
    uint8_t tmpPixel1[bigPixelSizeBytes];

    getPixelAt(x0, y0, tmpPixel0);
    getPixelAt(x1, y1, tmpPixel1);

    setPixelAt(x0, y0, tmpPixel1);
    setPixelAt(x1, y1, tmpPixel0);
}

void Image::doForEveryPixel(void (* func)(uint8_t *, PixelFormat::Enum))
{
    assert(isValid());
    assert(func != nullptr);

    const size_t pixelCount     = (width * height);
    const size_t pixelSizeBytes = PixelFormat::sizeBytes(pixelFormat);
    uint8_t * pixelPtr          = data;

    for (size_t i = 0; i < pixelCount; ++i)
    {
        func(pixelPtr, pixelFormat);
        pixelPtr += pixelSizeBytes;
    }
}

void Image::flipVInPlace()
{
    assert(isValid());

    for (uint32_t y = 0; y < (height / 2); ++y)
    {
        for (uint32_t x = 0; x < width; ++x)
        {
            swapPixels(x, y, x, ((height - 1) - y));
        }
    }
}

void Image::flipHInPlace()
{
    assert(isValid());

    for (uint32_t y = 0; y < height; ++y)
    {
        for (uint32_t x = 0; x < (width / 2); ++x)
        {
            swapPixels(x, y, ((width - 1) - x), y);
        }
    }
}

void Image::flipV(Image & destImage) const
{
    assert(isValid());

    // Allocate a new image:
    destImage.freeImageStorage();
    destImage.allocImageStorage(dataSizeBytes, width, height, pixelFormat);

    // Flip it:
    uint8_t tmpPixel[bigPixelSizeBytes];
    const uint32_t maxWidthIndex  = (width  - 1);
    const uint32_t maxHeightIndex = (height - 1);

    for (uint32_t y = 0; y < height; ++y)
    {
        for (uint32_t x = 0; x < width; ++x)
        {
            getPixelAt((maxWidthIndex - x), y, tmpPixel);
            destImage.setPixelAt((maxWidthIndex - x), (maxHeightIndex - y), tmpPixel);
        }
    }
}

void Image::flipH(Image & destImage) const
{
    assert(isValid());

    // Allocate a new image:
    destImage.freeImageStorage();
    destImage.allocImageStorage(dataSizeBytes, width, height, pixelFormat);

    // Flip it:
    uint8_t tmpPixel[bigPixelSizeBytes];
    for (uint32_t y = 0; y < height; ++y)
    {
        for (uint32_t x = 0; x < width; )
        {
            getPixelAt(x, y, tmpPixel);
            x = x + 1;
            destImage.setPixelAt((width - x), y, tmpPixel);
        }
    }
}

void Image::resizeInPlace(const uint32_t targetWidth, const uint32_t targetHeight)
{
    if ((width == targetWidth) && (height == targetHeight))
    {
        return; // Nothing to be done here...
    }

    // Resulting image may be bigger or smaller.
    // We always need a copy:
    Image resizedImage;
    resize(resizedImage, targetWidth, targetHeight);
    freeImageStorage();
    moveCopy(resizedImage);
}

void Image::resize(Image & destImage, const uint32_t targetWidth, const uint32_t targetHeight) const
{
    assert(isValid());
    assert(targetWidth  > 0);
    assert(targetHeight > 0);

    if ((width == targetWidth) && (height == targetHeight))
    {
        return; // Nothing to be done here...
    }

    const size_t pixelSize = PixelFormat::sizeBytes(pixelFormat);
    destImage.freeImageStorage();
    destImage.allocImageStorage((targetWidth * targetHeight * pixelSize), targetWidth, targetHeight, pixelFormat);

    // Quick resample with no filtering applied:
    const double scaleWidth  = static_cast<double>(targetWidth)  / static_cast<double>(width);
    const double scaleHeight = static_cast<double>(targetHeight) / static_cast<double>(height);
    uint8_t tmpPixel[bigPixelSizeBytes];

    for (uint32_t y = 0; y < targetHeight; ++y)
    {
        for (uint32_t x = 0; x < targetWidth; ++x)
        {
            getPixelAt(static_cast<uint32_t>(x / scaleWidth), static_cast<uint32_t>(y / scaleHeight), tmpPixel);
            destImage.setPixelAt(x, y, tmpPixel);
        }
    }
}

void Image::roundDownToPowerOfTwo()
{
    assert(isValid());

    // Minimum size for rounding.
    // If an image's w|h power of 2 is less than 16, we clamp it to 16.
    constexpr uint32_t minSize = 16;

    if (isPowerOfTwo())
    {
        return; // Nothing to do...
    }

    // Round down sizes:
    uint32_t targetWidth = floorPowerOfTwo(width);
    if (targetWidth < minSize) // Never go lower than the minimum:
    {
        targetWidth = minSize;
    }

    uint32_t targetHeight = floorPowerOfTwo(height);
    if (targetHeight < minSize) // Never go lower than the minimum:
    {
        targetHeight = minSize;
    }

    resizeInPlace(targetWidth, targetHeight);
}

void Image::swizzleRGB()
{
    assert(isValid());

    // Swizzle every pixel:
    const size_t pixelCount         = (width * height);
    const size_t componentsPerPixel = PixelFormat::componentCount(pixelFormat);
    uint8_t * pixelPtr              = data;

    for (size_t i = 0; i < pixelCount; ++i)
    {
        std::swap(pixelPtr[0], pixelPtr[2]);
        pixelPtr += componentsPerPixel;
    }
}

void Image::discardAlphaComponent()
{
    assert(isValid());
    if ((pixelFormat != PixelFormat::RgbaU8) && (pixelFormat != PixelFormat::RgbaF32))
    {
        return;
    }

    // Resulting image will be smaller. We need a new one:
    Image rgbImage;
    rgbImage.allocImageStorage((width * height * PixelFormat::sizeBytes(pixelFormat)), width, height,
            (pixelFormat == PixelFormat::RgbaU8) ? PixelFormat::RgbU8 : PixelFormat::RgbF32);

    uint8_t tmpPixel[bigPixelSizeBytes];
    for (uint32_t y = 0; y < height; ++y)
    {
        for (uint32_t x = 0; x < width; ++x)
        {
            getPixelAt(x, y, tmpPixel);
            rgbImage.setPixelAt(x, y, tmpPixel); // Since rgbImage is RGB, the alpha will be dropped by setPixelAt.
        }
    }

    freeImageStorage();
    moveCopy(rgbImage);
}

void Image::memSet(const int val)
{
    assert(isValid());
    std::memset(data, val, dataSizeBytes);
}

void Image::copyRect(Image & destImage, const uint32_t xOffset, const uint32_t yOffset,
                     const uint32_t rectWidth, const uint32_t rectHeight, const bool topLeft) const
{
    assert(isValid());
    assert(rectHeight != 0 && rectWidth != 0);

    if ((destImage.pixelFormat != pixelFormat) ||
        (destImage.width  < rectWidth)         ||
        (destImage.height < rectHeight))
    {
        destImage.freeImageStorage();
        destImage.allocImageStorage((rectWidth * rectHeight * PixelFormat::sizeBytes(pixelFormat)),
                rectWidth, rectHeight, pixelFormat);
    }

    uint8_t tmpPixel[bigPixelSizeBytes];

    if (topLeft)
    {
        // Use the top-left corner of the image as the (0,0) origin.
        // Invert Y:
        uint32_t maxY = (getHeight() - 1);
        for (uint32_t y = 0; y < rectHeight; ++y)
        {
            for (uint32_t x = 0; x < rectWidth; ++x)
            {
                getPixelAt(x + xOffset, maxY - yOffset, tmpPixel);
                destImage.setPixelAt(x, y, tmpPixel);
            }
            --maxY;
        }
    }
    else
    {
        // Use the bottom-left corner of the image as the (0,0) origin.
        // Default Y:
        for (uint32_t y = 0; y < rectHeight; ++y)
        {
            for (uint32_t x = 0; x < rectWidth; ++x)
            {
                getPixelAt(x + xOffset, y + yOffset, tmpPixel);
                destImage.setPixelAt(x, y, tmpPixel);
            }
        }
    }
}

void Image::setRect(const Image & srcImage, const uint32_t xOffset, const uint32_t yOffset,
                    const uint32_t rectWidth, const uint32_t rectHeight, const bool topLeft)
{
    // Validation:
    assert(isValid());
    assert(srcImage.isValid());
    assert(rectHeight != 0 && rectWidth != 0);

    uint8_t tmpPixel[bigPixelSizeBytes];

    if (topLeft)
    {
        // Use the top-left corner of the image as the (0,0) origin.
        // Invert Y:
        uint32_t maxY = (getHeight() - 1);
        for (uint32_t y = 0; y < rectHeight; ++y)
        {
            for (uint32_t x = 0; x < rectWidth; ++x)
            {
                srcImage.getPixelAt(x, y, tmpPixel);
                setPixelAt(x + xOffset, maxY - yOffset, tmpPixel);
            }
            --maxY;
        }
    }
    else
    {
        // Use the bottom-left corner of the image as the (0,0) origin.
        // Default Y:
        for (uint32_t y = 0; y < rectHeight; ++y)
        {
            for (uint32_t x = 0; x < rectWidth; ++x)
            {
                srcImage.getPixelAt(x, y, tmpPixel);
                setPixelAt(x + xOffset, y + yOffset, tmpPixel);
            }
        }
    }
}

void Image::setRect(const uint8_t * __restrict image, const uint32_t xOffset, const uint32_t yOffset,
                    const uint32_t rectWidth, const uint32_t rectHeight, const bool topLeft)
{
    // Validation:
    assert(isValid());
    assert(image != nullptr);
    assert(rectHeight != 0 && rectWidth != 0);

    const size_t pixelSize = PixelFormat::sizeBytes(pixelFormat);
    uint8_t tmpPixel[bigPixelSizeBytes];

    if (topLeft)
    {
        // Use the top-left corner of the image as the (0,0) origin.
        // Invert Y:
        uint32_t maxY = (getHeight() - 1);
        for (uint32_t y = 0; y < rectHeight; ++y)
        {
            for (uint32_t x = 0; x < rectWidth; ++x)
            {
                std::memcpy(tmpPixel, (image + (x + y * rectWidth) * pixelSize), pixelSize);
                setPixelAt(x + xOffset, maxY - yOffset, tmpPixel);
            }
            --maxY;
        }
    }
    else
    {
        // Use the bottom-left corner of the image as the (0,0) origin.
        // Default Y:
        for (uint32_t y = 0; y < rectHeight; ++y)
        {
            for (uint32_t x = 0; x < rectWidth; ++x)
            {
                std::memcpy(tmpPixel, (image + (x + y * rectWidth) * pixelSize), pixelSize);
                setPixelAt(x + xOffset, y + yOffset, tmpPixel);
            }
        }
    }
}

I'm aware of some of its flaws, such as the lack of parameterized constructors, move constructor, lots of raw pointer manipulation, type casting and manual memory management.

Please feel free to comment on how to improve those but, particularly, I'm interested in:

• Suggestions on the architectural aspects of it. How do you guys handle image processing in your programs?

• How do you deal with different image formats / pixel formats? My solution was parameterization, which lead to switch statements in a few places. I would like to improve that.

Also, note that the manual memory management is pretty much forced on me by the underlaying library I'm using (STBI) which is written in a C-style and returns a pointer allocated with malloc. Not sure if there's any room for improvement there.

Other comments on code style and practices are also very welcome. Forgive the lengthy code, I don't think it would have made sense to post just fragments of it.

Answer 1

I'll chime in on helping you get some switch statements out. I can see that the PixelFormat is an enum that is apart of the Image class. I also notice that PixelFormat is a reference to a collection of data and even methods.

The best bet would be to remove the enum from the image class and create some PixelFormat objects. Taking with it all the data that makes up a pixel format. All code below is pseudo-ish in nature.

IPixelFormat.hpp
PixelFormatRBGU8.hpp
PixelFormatRBGU8.cpp
...etc for each format

Here is what your interface might look like. I tried to pull out the various methods and data that were really just related to the pixel format, but I might've missed some and the parameters may need to change based off of data inherent to the image class (like data and width for the getPixelAt(...)).

#pragma once
#include<cstdint>

class IPixelFormat
{
public:
     enum PixelFormat
    {
        RgbU8,
        RgbF32,
        RgbaU8,
        RgbaF32
    };
    virtual PixelFormat getPixelFormatType() const =0;
    virtual const char* toString() const = 0;
    virtual  size_t sizeBytes() const = 0;
    virtual uint32_t componentCount()const = 0;
    virtual void setPixelAt(const uint32_t x, const uint32_t y, const uint8_t * pixel) const  =0;
    virtual void getPixelAt(const uint32_t x, const uint32_t y, uint8_t * pixel) const =0;

};

Modify your image class to use a reference to an IPixelFormat class, make sure to add those getters/setters:

void setPixelFormat(IPixelFormat* pixelFormat);
IPixelFormat* getPixelFormat() const;

IPixelFormat* pixelFormat;

Now remove the switches and just reference the PixelFormat, here is an example of your new setPixelAt() inside the image class:

void Image::setPixelAt(const uint32_t x, const uint32_t y, const uint8_t * pixel)
{
   this->pixelFormat->setPixelAt(x,y,pixel);   
}

This will help reduce the size of your image class, while segmenting out the specific functionality/data inherent to each PixelFormat. If you want to add a new pixel format you just add a new class and make sure its referenced in the right format lookup spot. No more needing to touch 5 or 6 spots in the image class to update switch blocks.

You could even get rid of that switch on the comp inside of:

bool Image::loadFromFile(...)

Instead of passing in comp, pass in a reference to the PixelFormat, and do a simple if(..) on the pixel format value to see if its null. If its not, great, set it! If it is null, then just do exactly what your default case does in the former switch.

Hope this has been helpful! I'm sure some more knowledgeable people will drop on by to help cleanup my pseudo-ish code.

Answer 2

If this code is working. Then DON'T do any major refactoring without unit tests.

You can add your standard modernizations that are easy to show work:

Use Copy and Swap Idiom on the assignment operator:

Image & Image::operator = (const Image & other) 
{
    freeImageStorage();   // This is not exception safe.
    initFromCopy(other);  // If the initFromCopy() throws then you are in a bad position.
    return *this;
}

Easy re-write:

Image & Image::operator = (Image copy) 
{
    copy.swap(*this);
    return *this;
}
void swap(Image& other) noexcept
{
    using std::swap;
    swap(data,          other.data);
    swap(dataSizeBytes, other.dataSizeBytes);
    swap(width,         other.width);
    swap(height,        other.height);
    swap(pixelFormat,   other.pixelFormat);
}

You could also add Move Semantics. (I did this to some old code and got a 30% boost in speed by doing nothing else but recompiling with C++11 on).

Image(Image&& other) noexcept
    : data(null)
{
    other.swap(*this);
}
Image& Image::operator=(Image&& other) noexcept
{
    other.swap(*this);
    return *this;
}

Answer 3

You are passing around many integers that represent sizes, positions and rectangles. Honestly, you could group these parameters into small utility classes. That would improve your functions signatures:

struct Size
{
    uint32_t width  = 0;
    uint32_t height = 0;
};

struct Position
{
    uint32_t x = 0;
    uint32_t y = 0;
};

struct Rectangle
{
    Position origin;
    Size size;
};

The three classes above are the most simple ones in the world (ok, it may be a little bit superlative), yet they make your functions signatures easier to read and reduce the risk to pass a size to a position parameter for example. Also, you can return them while you cannot return two integers. Moreover, these classes are so small that any modern compiler should be able to inline the code and make it as fast as if you were only passing integers around.

Note that I considered the classes simple enough to be structs with public members. I did not write any method, but methods such as isFlat on Size could be useful. You can also meaningfully overload some operators that will make the code using these classes easier to write and no harder to read.