trying to write a ppm image in c++

Question

I want to read and write a ppm image and calculate the average number of color.

My read method works fine and also the average number of each color is correct, but my write method gives me a wrong result.

The buffer which holds the data has type Vec3<float>*. Every 3D vector holds the r, g and b values.

Here is the read method:

Image * ReadPPM(const char * filename) {

    Image* image;

    ifstream file;

    unsigned int width, height;
    string version;
    float maxvalue;

    file.open(filename, ios::in | ios::binary);

    if (!file) {
        cerr << "file could not be open" << endl;
        exit(EXIT_FAILURE);
    }

    // read the header
    file >> version;

    // check the header
    // PPM "header" is valid
    if (version.compare("P6") != 0)
    {
        cout << "Invalid image format (must be 'P6')";
        exit(EXIT_FAILURE);
    }

    file >> width >> height >> maxvalue;


    size_t size = height * width*3;

    unsigned char * buffer = new unsigned char[size];
    Vec3<float>* finalBuffer = new Vec3<float>[size / 3];
    file.get();
    file.read( (char *)buffer, size);
    int j = 0;
    for (int i = 0; i < size; i+=3) {
        Vec3<float> vec(
            (int)buffer[i]/maxvalue,
            (int)buffer[i+1] /  maxvalue,
            (int)buffer[i+2] / maxvalue
        );
        finalBuffer[j] = vec;
        j++;
    }
    file.clear();
    file.close();

    image = new Image(width, height, finalBuffer);
    return image;
}

Here is the write method:

bool Image::operator >> (string filename) {
    ofstream file;
    file.open(filename,ios::out | ios::binary);
    if (!file) {
        cerr << "Cannot open file" << endl;
        return false;
    }

    file << "P6" <<"\n";
    file << getWidth() << "\n";
    file << getHeight() << "\n";
    file << "255" << "\n";
    size_t size = height * width;
    void*img = getRawDataPtr(); //returns void* buffer 
    unsigned char * temp = new unsigned char[size*3];
    Vec3<float>* buff = (Vec3<float>*)img;

    for (int i = 0; i<size; i++) {
        Vec3<float> vector;
        vector[0] =buff[i].x;
        vector[1] = buff[i].y;
        vector[2] = buff[i].z; //  /255.f;
        buff[i] = vector;
    }
    for (int i = 0; i < size; i++) {
        temp[i * 3] = static_cast<unsigned char>(buff[i].x);
        temp[i * 3 + 1] = static_cast<unsigned char>(buff[i].y);
        temp[i * 3 + 2] = static_cast<unsigned char>(buff[i].z);
    }
    file.write((char *)temp, size * 3);

    if (file.fail()) {
        cerr << "Could not write data" << endl;
        return false;
    }

    file.clear();
    file.close();

    return true;
    delete[] buff;
    delete[] temp;
}

Answer 1

Your values are normalized between 0 and 1 when reading the file. When writing, you just cast the float values into unsigned char, so you can only obtain 0 and 1 values: I guess you final image is filled with black.

    void*img = getRawDataPtr(); //returns void* buffer 
    unsigned char * temp = new unsigned char[size*3];
    Vec3<float>* buff = (Vec3<float>*)img;

Why creating a dynamic array you will have to manage? You are coding in C++, so use a container like std::vector.

    for (int i = 0; i<size; i++) {
        Vec3<float> vector;
        vector[0] =buff[i].x;
        vector[1] = buff[i].y;
        vector[2] = buff[i].z; //  /255.f;
        buff[i] = vector;
    }

What is the purpose of this loop? You are creating a Vec3<float> filled without transformation from the Vec3<float> in the current pixel in which you assign without transformation your temporary value: this just does nothing.

    return true;
    delete[] buff;
    delete[] temp;

As I said in the comments, the code after return is not executed, so you will never release the memory you allocated for temp. Use a std::vector and you will not have to do it. buff is your original image: you should not delete it here.

Once cleaned, your code may become:

    std::vector<unsigned char> temp(size*3);
    Vec3<float>* buff = static_cast<Vec3<float>*>(getRawDataPtr());

    for (int i = 0; i < size; i++) {
        temp[i * 3] = static_cast<unsigned char>(buff[i].x * 255);
        temp[i * 3 + 1] = static_cast<unsigned char>(buff[i].y * 255);
        temp[i * 3 + 2] = static_cast<unsigned char>(buff[i].z * 255);
    }
    file.write(reinterpret_cast<char*>(&temp[0]), size * 3);

    if (file.fail()) {
        cerr << "Could not write data" << endl;
        return false;
    }

    file.close();

    return true;