Valgrind問題

[Ouzheee]

在跑記憶體檢測時很大一部分是因為GrayImage 的LoadImage出了問題

畢竟DataLoader本身沒有刪除它allocated的momories，可是連接該3D-dinamic array的pointer: pixels已經在GrayImage的destructor delete了....

May be I should edit the DataLoader

[coherent17]

I don't think so, your program should be responsible to delete the memory that dynamic allocate by the data_loader.

[Ouzheee]

We still have some question on memory check. Some function will return int ***, which means we cannot release it in the function. So we delete it in main.cpp like this:

    delete img1;
    img1 = nullptr;
    delete img2;
    img2 = nullptr;

And the destructor of GRAYImage and RGBImage delete the pixels of Image. Also we check the pixels int LoadImage. But valgrind still marks that there is still momory leak as above pictures show. Is there anything we lost?

bool RGBImage::LoadImage(string filename){
    loadfilename = filename;
    if (pixels){
        for (int i=0; i<height; ++i){
            for (int j=0; j<width; ++j){
                delete [] pixels[i][j];
            }
        }
        for (int i=0; i<height; ++i){
            delete [] pixels[i];
        }
        delete [] pixels;
    }
    pixels = data_loader.Load_RGB(filename, &width, &height);
    return true;
}

GrayImage::~GrayImage(){
    if (pixels){
        for (int i=0; i<height; i++){
            delete [] pixels[i];
        }
        delete [] pixels;
        pixels = nullptr;
    }
}

RGBImage::~RGBImage(){
    for (int i=0; i<height; ++i){
        for (int j=0; j<width; ++j){
            delete [] pixels[i][j];
        }
    }
    for (int i=0; i<height; ++i){
        delete [] pixels[i];
    }
    delete [] pixels;
}

[coherent17]

in your example, if img1 & img2 is from Image class, then the virtual destructor should be implemented.

[Ouzheee]

Sorry TA, but we still can't solve the problem. What's the worse, after we finished the Photo Mosaic and try to run the program. It turns output following error message:

we have delete the vector<int ***> in the destructor of PhotoMosaic. The last memory allocating is in createImageGrid

int ***PhotoMosaic::createImageGrid(const vector<int ***>& images, int subWidth, int subHeight, int width, int height) {
    int ***grid = new int **[height];
    for (int i = 0; i < height; i++) {
        grid[i] = new int *[width];
        for (int j = 0; j < width; j++) {
            grid[i][j] = new int[3];
        }
    }

    for (int y = 0; y < width / subWidth; y++) {
        for (int x = 0; x < height / subHeight; x++) {
            int ***subImage = images[y * width / subWidth + x];
            for (int i = 0; i < subHeight; i++) {
                for (int j = 0; j < subWidth; j++) {
                    grid[y * subHeight + i][x * subWidth + j][0] = subImage[i][j][0];
                    grid[y * subHeight + i][x * subWidth + j][1] = subImage[i][j][1];
                    grid[y * subHeight + i][x * subWidth + j][2] = subImage[i][j][2];
                }
            }
        }
    }
    return grid;
}

and the Grid is used in Generate_Mosaic

RGBImage PhotoMosaic::Generate_Mosaic(){
    int subHeight = small_image[0].get_height(), subWidth = small_image[0].get_width();
    int height = image.get_height(), width = image.get_width();
    RGBImage Mosaic(height, width);
    int ***m_pixels = Mosaic.get_3D_pixels();
    int BestMatchIndex;
    vector<int ***> Grid_v;
    for (int y = 0; y < height / subHeight; y++) {
        for (int x = 0; x < width / subWidth; x++) {
            CalculateAverage(splited_photo[y+x]); //calculate the Average of the y+xth splited photo
            BestMatchIndex = getBestMatchIndex(splited_photo[y+x], small_avg);
            Grid_v.push_back(small_image[BestMatchIndex].get_3D_pixels());

        }
    }
    m_pixels = createImageGrid(Grid_v, subWidth, subHeight, width, height);
    return Mosaic;
}

We are not sure whether the m_pixels would auto delete by destructor of RBGImage?

[coherent17]

I think you can refactor this code, it's not that hard for performing photo mosaic algorithm. Write it clean.

If you want to make sure which destructor is called, you might want to add some log (eg cout sth) in the implementation of destructor.