Unsure how to declare and then access an int array in heap memory

Question

I have a giant array that's being used to temporarily store pixel data (before it's written to a file). I've been having "random" crashes and someone much more proficient with C++ than myself suggested that I should put my pixel array in the heap than stack. These were all new words to me but I've read up and it makes sense but I'm struggling to set this up.

Declaring the heap array:

int *heap = new int[MAX_WIDTH * MAX_HEIGHT * 3];

This is then used by my Canvas class, which I want to hold a pointer to a specific heap:

class Canvas {
    public:
    Canvas(int w, int h, bool fill_transparent, int* heap);
    Canvas();
    
    int width;
    int height;
    
    int* heap;
}

Canvas::Canvas(int w, int h, bool fill_transparent, int* heap) {
    if (w>MAX_WIDTH) w = MAX_WIDTH;
    if (h>MAX_HEIGHT) h = MAX_HEIGHT;
    
    width = w;
    height = h;
    heap = heap;
    
    if (fill_transparent) {
        for(int x=0; x<width; ++x) {
            for(int y=0; y<height; ++y) {
                set(x, y, transparent);
                //heap->pixels[x][y] = transparent;
            }
        }
    }
}

Creating a new canvas and passing the heap along:

Canvas new_texture(int w, int h, int* heap) {

  Canvas canvas(w, h, true, heap);

  // Do some things with it, using set(x,y);

  return canvas;

}

And then attempting to set pixel data (Pixel is a struct with {int r, int g, int b}:

void Canvas::set(int x, int y, Pixel colour) {
    if (x>=width || y>=height || x<0 || y <0) return;
    
    // Set the heap :C
    *heap[(x * (MAX_HEIGHT*3)) + y] = colour.r;
    *heap[(x * (MAX_HEIGHT*3)) + y + 1] = colour.g;
    *heap[(x * (MAX_HEIGHT*3)) + y + 2] = colour.b;
}

This is wrong! I just can't figure out how I should be declaring the array, storing the pointer in a class, and then accessing it. int *heap[] makes more sense to me but also doesn't work.

I'm learning on the job, and am really struggling to find an overview of how I should be doing this.

I've tried most variations that I can think of, I'm struggling to get something that works in all three places!

Answer 1

You would access the dynamically allocated memory using
heap[y * width * 3 + x * 3 + color] but it's both inconvenient and error prone.

Your class is also lacking the special member functions to follow the rule of five which makes future bugs very likely.

I recommend using a std::vector which will manage the dynamically allocated memory for you and you also store Pixels directly in the std::vector instead of just multiplying the number of ints needed by 3. The struct and the transparent constant could look like this:

struct Pixel {
    int r = 0;
    int g = 0;
    int b = 0;
};

// replace with the constant values you want for transparent:
inline constexpr Pixel transparent{255, 255, 255};

int for the individual channels in Pixel may be overdoing it a bit. Consider using a smaller type, like uint8_t for example.

In your Canvas class, you could create operator() overloads for accessing the Pixels, that you then store in a std::vector<Pixel>:

class Canvas {
public:
    explicit Canvas(size_t h, size_t w, bool fill_transparent);

    Pixel& operator()(size_t h, size_t w);
    const Pixel& operator()(size_t h, size_t w) const;

    size_t Height() const { return height; }
    size_t Width() const { return width; }

private:
    size_t height;
    size_t width;

    std::vector<Pixel> pixels;
};

And the implementation of the operator() overloads becomes a bit simpler than with your dynamically allocated ints. The constructor also tells the std::vector<Pixel> how many pixels there should be and if they should be transparent or have the default constructed Pixel:

Canvas::Canvas(size_t h, size_t w, bool fill_transparent)
    : height(std::min(h, MAX_HEIGHT)),
      width(std::min(w, MAX_WIDTH)),
      pixels(height * width, fill_transparent ? transparent : Pixel{}) {}

Pixel& Canvas::operator()(size_t h, size_t w) {
    return pixels[h * width + w]; 
}

const Pixel& Canvas::operator()(size_t h, size_t w) const {
    return pixels[h * width + w];
}

Demo

Answer 2

Main issue

*heap[(x * (MAX_HEIGHT*3)) + y] = colour.r;

First of all, syntactically this should be:

heap[(x * (MAX_HEIGHT*3)) + y] = colour.r

Putting a * there is just wrong in this case.

Next if width < MAX_WIDTH or height < MAX_HEIGHT, then you'd obviously be completely off!

This should be heap[x*(height + 3) + y], not MAX_HEIGHT.

And then you're still missing the 3 factor from your y term here, so actually this should be

heap[3*(x*height + y)]

General Notes

So this will be part "code review", not just addressing the issue.

int *heap = new int[MAX_WIDTH * MAX_HEIGHT * 3];

Two things here: In C++ it's way easier to just use std::vector when allocating a dynamically sized array; That way you get the memory management part 'automatically'.

std::vector<int> heap(MAX_WIDTH * MAX_HEIGHT * 3);

Moving on, it'd be a bit weird to allocate the memory outside of the Canvas: You'd normally expect the canvas to "own" the memory, so you'd actually just write:

// assuming you have your `heap` member as `std::vector<int>` instead of `int *`.
// we know longer take the 'heap' as a parameter, instead we just construct it directly inside of our object
Canvas::Canvas(int w, int h, bool fill_transparent)
// this : thing is a relatively unique thing to C++, this means
// "initialise this field with this expression"; In C++ class
// member initialisation happens *before* we actually enter the constructor body.
// This is called an initialiser list, you should look that term up for more details
: heap(w * h * 3)
{
  // rest of your constructor code here
}

This way you don't need to worry about all this setup outside your canvas, and as a bonus you're now only allocating as much memory as you need, not as much memory as you might maximally need. There are situations where you might want to manage the memory outside the canvas struct, or always allocate the maximum amount of memory you may ever need, but unless you know you need this it's not a good idea to add these complications.

Finally, if your Pixel is just a simple RGB struct you could just put that into the array directly, no need to have it "unrolled" into integers unless you need to force a particular in-memory representation (while fortunately most C++ implementations have a pretty predictable way they layout structs, technically it's unspecified in the C++ standard so if you don't know what implementations you're targeting you unfortunately might have to skip this step, but this will be fine for most cases):

std::vector<Pixel> heap; // inside the class

// in the initialiser list:
: heap(w*h) // note no more *3 here

// and in the method, note we don't have the weird 3* term in here anymore

heap[x*height + y] = colour;

Also, while this is more a matter of opinion, heap is a bit of a weird name for this. You could just call it pixels; then you'd write

pixels[x * height + y] = colour;

which at least to me seems clearer.