C++ Modifying a TGA file and adding to a certain channel

Question

this is what i am getting this is the image i am supposed to output I am attempting to add a number to the green channel of a loaded TGA file, but I am getting the wrong result, also when I add zero, it returns a modified image.

Here is my code and the function i am using.

I have believe the error might be on how i am structuring the code, but other than that maybe its my code structure.

`#ifndef PROJECT2_TGA_READER_H
#define PROJECT2_TGA_READER_H

#include <iostream>
#include <fstream>
#include <vector>

using namespace std;
struct TGA_HeaderData{
    char idLength;
    char colorMapType;
    char dataTypeCode;
    short colorMapOrigin;
    short colorMapLength;
    char colorMapDepth;
    short xOrigin;
    short yOrigin;
    short width;
    short height;
    char bitsPerPixel;
    char imageDescriptor;
};
struct Pixel_Data{
    char blue;
    char green;
    char red;
};


class TGA_Reader {
public:
    TGA_Reader(string file_name);

    TGA_HeaderData HeaderData{};

    vector<Pixel_Data> pixels;

    void DeclareFile(string file_name);

    void createFile(const char *name, const TGA_HeaderData &headerData, const vector<Pixel_Data> &pixels);

    TGA_Reader add_green_channel(int value);
};

#include "TGA_Reader.h"

TGA_Reader::TGA_Reader(string file_name) {
    DeclareFile(file_name);
}

void TGA_Reader::DeclareFile(string file_name) {
    ifstream file(file_name, ios::binary);
    if (!file){
        cout<<"not open"<<endl;
        return;
    }
    file.read((char*)&HeaderData.idLength, sizeof(HeaderData.idLength));
    file.read((char*)&HeaderData.colorMapType, sizeof(HeaderData.colorMapType));
    file.read((char*)&HeaderData.dataTypeCode,  sizeof(HeaderData.dataTypeCode));
    file.read((char*)&HeaderData.colorMapOrigin, sizeof(HeaderData.colorMapOrigin));
    file.read((char*)&HeaderData.colorMapLength, sizeof(HeaderData.colorMapLength));
    file.read((char*)&HeaderData.colorMapDepth, sizeof(HeaderData.colorMapDepth));
    file.read((char*)&HeaderData.xOrigin, sizeof(HeaderData.xOrigin));
    file.read((char*)&HeaderData.yOrigin, sizeof(HeaderData.yOrigin));
    file.read((char*)&HeaderData.width, sizeof(HeaderData.width));
    file.read((char*)&HeaderData.height, sizeof(HeaderData.height));
    file.read((char*)&HeaderData.bitsPerPixel, sizeof(HeaderData.bitsPerPixel));
    file.read((char*)&HeaderData.imageDescriptor, sizeof(HeaderData.imageDescriptor));

    pixels.reserve(HeaderData.width*HeaderData.height);



    for (int i = 0; i < HeaderData.width*HeaderData.height; i++) {
        Pixel_Data pixel;
        file.read((char*)&pixel.blue, sizeof(unsigned char));
        file.read((char*)&pixel.green, sizeof(unsigned char));
        file.read((char*)&pixel.red, sizeof(unsigned char));
        pixels.push_back(pixel);
        }

    file.close();
    }


void TGA_Reader::createFile(const char *name,
                            const TGA_HeaderData &headerData,
                            const vector<Pixel_Data> &pixels) {
    ofstream outfile;

    outfile.open("/Users/diegogomez/CLionProjects/Project2/cmake-build-debug/Hello3.tga",ios::binary);

    outfile.write((char*)&headerData.idLength, sizeof(headerData.idLength));
    outfile.write((char*)&headerData.colorMapType, sizeof(headerData.colorMapType));
    outfile.write((char*)&headerData.dataTypeCode,  sizeof(headerData.dataTypeCode));
    outfile.write((char*)&headerData.colorMapOrigin, sizeof(headerData.colorMapOrigin));
    outfile.write((char*)&headerData.colorMapLength, sizeof(headerData.colorMapLength));
    outfile.write((char*)&headerData.colorMapDepth, sizeof(headerData.colorMapDepth));
    outfile.write((char*)&headerData.xOrigin, sizeof(headerData.xOrigin));
    outfile.write((char*)&headerData.yOrigin, sizeof(headerData.yOrigin));
    outfile.write((char*)&headerData.width, sizeof(headerData.width));
    outfile.write((char*)&headerData.height, sizeof(headerData.height));
    outfile.write((char*)&headerData.bitsPerPixel, sizeof(headerData.bitsPerPixel));
    outfile.write((char*)&headerData.imageDescriptor, sizeof(headerData.imageDescriptor));

---


    cout<<pixels.size()<<endl;


    for (int i = 0; i < pixels.size(); i++) {
        outfile.write((char*)&pixels[i].blue, sizeof(unsigned char));
        outfile.write((char*)&pixels[i].green, sizeof(unsigned char));
        outfile.write((char*)&pixels[i].red, sizeof(unsigned char));
    }
    outfile.close();
}

TGA_Reader TGA_Reader::add_green_channel(int value) {

    TGA_Reader copy(*this); // make a copy of the object
    for (int i = 0; i < copy.pixels.size(); i++) {
        int pixel = (int) (copy.pixels[i].green + value);
        if (pixel < 0) {
            int pixel = 0;
        }
        if (pixel > 255) {
            int pixel = 255;
        }
        (unsigned char)(pixel);
        copy.pixels[i].green = pixel;

    }
    return copy; // return the modified copy}
}`

Answer 1

This section of code is a mess:

        int pixel = (int) (copy.pixels[i].green + value);
        if (pixel < 0) {
            int pixel = 0;
        }
        if (pixel > 255) {
            int pixel = 255;
        }
        (unsigned char)(pixel);
        copy.pixels[i].green = pixel;

By doing int pixel = 0; you're declaring a new variable called pixel that is hiding the variable declared on the first line of the snippet above. You do so again in the next if block. you should drop int from both those lines.

Furthermore, (unsigned char)(pixel); is a do-nothing statement with no side effects. You probably meant:

        int pixel = (int) (copy.pixels[i].green + value);
        if (pixel < 0) {
            pixel = 0;
        }
        if (pixel > 255) {
            pixel = 255;
        }
        copy.pixels[i].green = (unsigned char)pixel;

There are many more things that are poor. Such as, you're casting to unsigned char when the target variable .green is declared as just (signed) char. You should probably use a uint8_t for these types.