Skip to main content
1 of 3
Xirema
  • 247
  • 2
  • 7

Can my code for saving image data be simplified?

Given a heap of pixel data, and a width and height associated with that pixel data, I've written code which will save that data as an image, and it more-or-less works as expected.

namespace filesystem = std::experimental::filesystem;

//'heap' is an object wrapping around a std::vector<uint8_t> which allows me to query its 
//width and height, as opposed to its "size"
void save_image(filesystem::path path, font_heap const& heap) {
    uint32_t width = heap.get_width(), height = heap.get_height();
    FIBITMAP * image = FreeImage_Allocate(width, height, 8);
    
    BYTE * bits = (BYTE*)FreeImage_GetBits(image);
    auto it = stdext::make_checked_array_iterator(bits, width * height);
    //BEGIN CONCERNING CODE
    for (size_t y = 0; y < height; y++) {
        it = std::transform(
            heap.begin() + (height - y - 1) * width,
            heap.begin() + (height - y) * width,
            it,
            [](uint8_t val) {return 255 - val; }
        );
    }
    //END CONCERNING CODE
    
    FIMEMORY * memory = FreeImage_OpenMemory();
    FreeImage_SaveToMemory(FIF_PNG, image, memory);
    BYTE * raw_memory;
    DWORD size;
    FreeImage_AcquireMemory(memory, &raw_memory, &size);
    std::ofstream file(path, std::ios_base::binary);
    file.write(reinterpret_cast<char *>(raw_memory), size);
    FreeImage_CloseMemory(memory);
    FreeImage_Unload(image);
}

But, aside from some trivial optimizations in surrounding parts of the code (which don't concern me because they make up a tiny proportion of CPU time), the code in the for loop is most troubling to me:

auto it = stdext::make_checked_array_iterator(bits, width * height);
for (size_t y = 0; y < height; y++) {
    it = std::transform(
        heap.begin() + (height - y - 1) * width,
        heap.begin() + (height - y) * width,
        it,
        [](uint8_t val) {return 255 - val; }
    );
}

What needs to happen in this code is

  • All the rows need to be swapped, because images are saved x→, y↓, but the data is saved in memory as x→, y↑
  • The colors need to be inverted, because the image is a raw grayscale image, and the image should be black with a white background (but is saved in data as white with a black background).

The code does what I'm expecting, but it requires a rather significant amount of CPU overhead to do it, and I'd like to know if there's a better solution.

  • Are there STL algorithms that I could/should be using instead of std::transform? Maybe stuff to use alongside std::transform?
  • Is there a way to eliminate the surrounding for-loop? I can't use reverse iterators, because the row ordering needs to be reversed, but the column ordering needs to stay the same.
  • Are there FreeImage functions/options I could/should be using to make this code faster? Have I included needlessly redundant steps in my process of saving the image to disk?
  • Can this be multithreaded without the overhead outweighing the performance gains? (Also, this code is meant to be used in an environment where there already exist other threads, and I don't want to introduce significant slowdowns to those other threads)
  • Are there any CUDA/OpenCL/Vulkan Compute solutions that won't add significant overhead to this problem?
Xirema
  • 247
  • 2
  • 7