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}
}`

What error? What result are you expecting? What actually happened instead? — Wyck, Mar 29 '23 at 19:53
That line does not create a "`char` variable". That line does not create any variable and does not change the type of `pixel`. — Drew Dormann, Mar 29 '23 at 19:53
@DrewDormann ok, do you know what might be causing the image to process differently. — DJA15, Mar 29 '23 at 19:56
@Wyck i am trying to add to the green channel of an image that is input to my class. But the output image is incorrectly modified. the brighter colors seem to come out unaltered. — DJA15, Mar 29 '23 at 19:57
What does "wrong result" look like? What does "process differently" look like? What does "incorrectly modified" look like? What actually happens? — Drew Dormann, Mar 29 '23 at 19:59
For starters, you don't have any code here that calls `add_green_channel`. — Wyck, Mar 29 '23 at 20:00
Warning: It doesn't happen all that often but nothing guarantees that `char` is 8 bits and `short` is 16 bits. — user4581301, Mar 29 '23 at 20:01
@wyck I am doing that on a different file since i believe my errors are in the classes — DJA15, Mar 29 '23 at 20:02

score 2 · Answer 1 · answered Mar 29 '23 at 20:09

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.

Definitely go `uint8_t`. Otherwise `copy.pixels[i].green + value` overflows a lot sooner than you want, likely rolls over negative, and gets clipped by the `pixel < 0` test. — user4581301, Mar 29 '23 at 20:14

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

1 Answers1