2
\$\begingroup\$

I know this is horrible code. This program is for performing modeling, view and projection transformation.

The program reads input from scene.txt and outputs the modeling transformation in stage1.txt, view transformation in stage2.txt, projection transformation in stage3.txt. The files are added below.

I haven't utilized the functionalities of C++ and the writing style is clumsy. I want to know how I could have written this same code with better time,space complexity, data structure and also with better elegance.

I have commented but someone pointed out that it was not enough. Where else should I be commenting?

scene.txt

0.0 0.0 50.0
0.0 0.0 0.0 
0.0 1.0 0.0
80.0 1.0 1.0 100.0
triangle
0.0 0.0 0.0
5.0 0.0 0.0
0.0 5.0 0.0
push
scale
2.0 2.0 2.0
triangle
0.0 0.0 0.0
5.0 0.0 0.0
0.0 5.0 0.0
translate
10.0 0.0 0.0
triangle
0.0 0.0 0.0
5.0 0.0 0.0
0.0 5.0 0.0
rotate
90.0 0.0 0.0 1.0
triangle
0.0 0.0 0.0
5.0 0.0 0.0
0.0 5.0 0.0
pop
triangle
0.0 0.0 0.0
20.0 0.0 0.0
0.0 20.0 0.0
end

main.cpp

//
// Created by afsara on 7/11/19.
//

#include <iostream>
#include <stack>
#include <vector>
#include <cstdio>
#include <fstream>
#include <cmath>
#include <cstring>
#include <string>
#include <sstream>
#include <iomanip>

using namespace std;
const double PI = acos(-1.0);
const double EPS = 1e-4;

ifstream infile, infile2, infile3;
ofstream outfile, outfile2, outfile3;
int sz = 4;

stack<float (*)[10]> s;
stack<int> Size;


struct Point {
    float x, y, z, w = 1;

    Point() {}
};

struct Vector {
    float x, y, z, w = 0;

    Vector() {
    x = x;
    y = y;
    z = z;
    };

    Vector(float vx, float vy, float vz) {

    x = vx;
    y = vy;
    z = vz;
    w = 0;
    }

};

Vector eye(0, 0, 0), look(0, 0, 0), up(0, 0, 0);
float fovY, aspectRatio, near, far;

/*  functions   */

float (*matrixMultiplication(float firstMatrix[][10], float secondMatrix[][10],
                         int rowFirst,
                         int columnFirst, int rowSecond, int columnSecond))[10];


float (*makeIdentityMatrix(int identity_sz))[10];

void showstack(stack<float (*)[10]> s);

void insertToStack(float arr[][10]);

float (*makeTranslationMatrix(float transX, float transY, float transZ))[10];

float (*makeScalingMatrix(float scaleX, float scaleY, float scaleZ))[10];

inline double degToRad(double ang);

static inline bool isNearlyEqual(const double &a, const double &b);

float Cos(float angle);

float Sin(float angle);

float Tan(float angle);

Vector crossProduct(const Vector &vec1, const Vector &vec2);

float dotProduct(const Vector &vec1, const Vector &vec2);

Vector normalize(Vector a);

Vector multiply(Vector v, float scalar);

Vector add(Vector v1, Vector v2);

Vector subtract(Vector v1, Vector v2);

Vector rotateRod(Vector x, Vector rotateAxis, float rotateAngle);

void printMatrix(float (*matrix)[10]);

void printMatrix2(float (*matrix)[10]);

void printMatrix3(float (*matrix)[10]);

void print(float (*matrix)[10]);

void printTokens(vector<string> tokens[100], int line_num);

void readFromFile();

void readFromStage1File();

void readFromStage2File();


int main() {


    float (*identityMatrix)[10] = makeIdentityMatrix(sz);
    insertToStack(identityMatrix);

    outfile.open("stage1.txt");
    readFromFile();
    outfile.close();

    outfile2.open("stage2.txt");
    readFromStage1File();
    outfile2.close();

    outfile3.open("stage3.txt");
    readFromStage2File();
    outfile3.close();

    return 0;
}


inline double degToRad(double ang) {
    return ang * PI / 180.0;
}

static inline bool isNearlyEqual(const double &a, const double &b) {
    return abs(a - b) < EPS;
}

float Cos(float angle) {
    float var = cos(degToRad(angle));
    if (isNearlyEqual(var, 0)) var = 0;
    return var;
}

float Sin(float angle) {
    float var = sin(degToRad(angle));
    if (isNearlyEqual(var, 0)) var = 0;
    return var;
}

float Tan(float angle) {
    float var = tan(degToRad(angle));
    if (isNearlyEqual(var, 0)) var = 0;
    return var;
}


Vector crossProduct(const Vector &vec1, const Vector &vec2) {

    Vector res;
    res.x = vec1.y * vec2.z - vec2.y * vec1.z;
    res.y = vec1.z * vec2.x - vec2.z * vec1.x;
    res.z = vec1.x * vec2.y - vec2.x * vec1.y;

    return res;
}


float dotProduct(const Vector &vec1, const Vector &vec2) {

    float res;

    res += vec1.x * vec2.x + vec1.y * vec2.y + vec1.z * vec2.z;
    if (isNearlyEqual(res, 0)) res = 0;

    return res;
}

Vector normalize(Vector a) {

    float val = sqrt(a.x * a.x + a.y * a.y + a.z * a.z);

    Vector p;
    p.x = a.x / val;
    p.y = a.y / val;
    p.z = a.z / val;

    //cout << "\nnormalizing\n[ " << p.x << " " << p.y << " " << p.z << " " << p.w << " ] \n";
    return p;
}

Vector multiply(Vector v, float scalar) {
    // cout << "scalar is " << scalar << endl;
    v.x = v.x * scalar;
    v.y = v.y * scalar;
    v.z = v.z * scalar;
    return v;
}

Vector add(Vector v1, Vector v2) {
    Vector ret(0, 0, 0);
    ret.x = v1.x + v2.x;
    ret.y = v1.y + v2.y;
    ret.z = v1.z + v2.z;
    return ret;
}


Vector subtract(Vector v1, Vector v2) {
    Vector ret;
    ret.x = v1.x - v2.x;
    ret.y = v1.y - v2.y;
    ret.z = v1.z - v2.z;
    return ret;

}

Vector rotateRod(Vector x, Vector rotateAxis, float rotateAngle) {


    Vector temp1 = multiply(x, Cos(rotateAngle)); //cos(theta)*x ; x is a vector
    //cout << "\ntemp1 " << temp1.x << " " << temp1.y << " " << temp1.z << endl;

    Vector temp2 = crossProduct(rotateAxis, x); // a cross x
    // cout << "a cross x : " << rotateAxis.x << " " << rotateAxis.y << " " << rotateAxis.z << " cross " << x.x << " "
    // << x.y << " " << x.z << endl;
    //cout << "\ntemp2 " << temp2.x << " " << temp2.y << " " << temp2.z << endl;

    Vector temp3 = multiply(temp2, Sin(rotateAngle)); //sin(theta) * (a cross x)
    //cout << "\ntemp3 " << temp3.x << " " << temp3.y << " " << temp3.z << endl;

    Vector temp4 = add(temp1, temp3); // cos(theta)*x + sin(theta) * (a cross x)
    cout << "\ntemp4 " << temp4.x << " " << temp4.y << " " << temp4.z << endl;


    float temp5 = dotProduct(rotateAxis, x); // a dot x
    //cout << "\ntemp5 " << temp5 << endl;

    Vector temp6 = multiply(rotateAxis, temp5); // (a dot x)*a
    //cout << "\ntemp6 " << temp6.x << " " << temp6.y << " " << temp6.z << endl;

    Vector temp7 = multiply(temp6, (1 - Cos(rotateAngle))); // (1-cos(theta)) * (a dot x)*a
    //cout << "\ntemp7 " << temp7.x << " " << temp7.y << " " << temp7.z << endl;

    Vector finalR = add(temp4,
                    temp7); // cos(theta)*x + sin(theta) * (a cross x) + (1-cos(theta)) * (a dot x)*a
    //cout << "\nfinal  " << finalR.x << " " << finalR.y << " " << finalR.z << endl;

    return finalR;
}


void printMatrix(float (*matrix)[10]) {
    cout << "\n in print matrix \n";

    for (int i = 0; i < sz - 1; ++i) {
    for (int j = 0; j < sz - 1; ++j) {
        cout << matrix[i][j] << " ";
        outfile << setprecision(7) << fixed << matrix[j][i] << " ";
    }
    cout << endl;
    outfile << endl;
    }
    outfile << endl;
}

void printMatrix2(float (*matrix)[10]) {
    cout << "\n in print matrix 2\n";

    for (int i = 0; i < sz - 1; ++i) {
    for (int j = 0; j < sz - 1; ++j) {
        cout << matrix[i][j] << " ";
        outfile2 << setprecision(7) << fixed << matrix[j][i] << " ";
    }
    cout << endl;
    outfile2 << endl;
    }
    outfile2 << endl;

}

void printMatrix3(float (*matrix)[10]) {

    cout << "\n in print matrix 3\n";
    for (int i = 0; i < sz - 1; ++i) {
    for (int j = 0; j < sz - 1; ++j) {
        cout << matrix[i][j] << " ";
        outfile3 << setprecision(7) << fixed << matrix[j][i] << " ";
    }
    cout << endl;
    outfile3 << endl;
    }
    cout<<endl;
    outfile3 << endl;
}

void print(float (*matrix)[10]) {

    cout << "\nprint in console\n";
    for (int i = 0; i < sz; ++i) {
    for (int j = 0; j < sz; ++j) {
        cout << matrix[i][j] << " ";
    }
    cout << endl;
    }
    cout<<endl;
}


float (*balance_W(float (*m)[10]))[10] {

    float w1 = m[sz - 1][0];
    float w2 = m[sz - 1][1];
    float w3 = m[sz - 1][2];

    for (int i = 0; i < sz; ++i) {
    for (int j = 0; j < sz; ++j) {
        if (j == 0) m[i][j] = m[i][j] / w1;
        if (j == 1) m[i][j] = m[i][j] / w2;
        if (j == 2) m[i][j] = m[i][j] / w3;
    }
    }

    return m;

}

void printTokens(vector<string> tokens[100], int line_num) {
    //printing the file content as a 2d array
    for (int j = 0; j < line_num; ++j) {

    for (int i = 0; i < tokens[j].size(); ++i) {

        cout << j << " : " << i << " " << tokens[j][i] << " \n";

    }
    cout << endl;
    }
}

void readFromFile() {
    // open a file in read mode.
    cout << "\n\n~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the scene.txt file  ~~~~~~~~~~~~~~~~~~~~~~~\n\n";

    string line;

    infile.open("scene.txt");

    if (!infile.is_open()) {
    perror("Error open");
    exit(EXIT_FAILURE);
    }

    int line_num = 0;
    vector<string> tokens[100];     // Create vector to hold the words
    while (getline(infile, line)) {

    string buf;                     // Have a buffer string
    stringstream ss(line);       // Insert the string into a stream

    while (ss >> buf)
        tokens[line_num].push_back(buf);

    line_num++;
    }

    //printTokens(tokens, line_num);

    string command;
    for (int i = 0; i < line_num; ++i) {

    if (i == 0) {
        eye.x = stof(tokens[i][0].c_str());
        eye.y = stof(tokens[i][1].c_str());
        eye.z = stof(tokens[i][2].c_str());
    } else if (i == 1) {
        look.x = stof(tokens[i][0].c_str());
        look.y = stof(tokens[i][1].c_str());
        look.z = stof(tokens[i][2].c_str());
    } else if (i == 2) {
        up.x = stof(tokens[i][0].c_str());
        up.y = stof(tokens[i][1].c_str());
        up.z = stof(tokens[i][2].c_str());
    } else if (i == 3) {
        fovY = stof(tokens[i][0].c_str());
        aspectRatio = stof(tokens[i][1].c_str());
        near = stof(tokens[i][2].c_str());
        far = stof(tokens[i][3].c_str());
    } else {

        for (int itr = 0; itr < tokens[i].size(); ++itr) {

            command = tokens[i][itr];

            //start parsing commands
            if (command == "triangle") {

                //go to next line
                i++;

                cout << "found a triangle " << endl;

                //input three points
                struct Point firstPoint, secondPoint, thirdPoint;

                firstPoint.x = stof(tokens[i][itr].c_str());
                firstPoint.y = stof(tokens[i][itr + 1].c_str());
                firstPoint.z = stof(tokens[i][itr + 2].c_str());

                i++;
                secondPoint.x = stof(tokens[i][itr].c_str());
                secondPoint.y = stof(tokens[i][itr + 1].c_str());
                secondPoint.z = stof(tokens[i][itr + 2].c_str());

                i++;
                thirdPoint.x = stof(tokens[i][itr].c_str());
                thirdPoint.y = stof(tokens[i][itr + 1].c_str());
                thirdPoint.z = stof(tokens[i][itr + 2].c_str());

                float myMatrix[10][10];

                vector<float> temp;
                temp.push_back(firstPoint.x);
                temp.push_back(secondPoint.x);
                temp.push_back(thirdPoint.x);
                temp.push_back(1);

                temp.push_back(firstPoint.y);
                temp.push_back(secondPoint.y);
                temp.push_back(thirdPoint.y);
                temp.push_back(1);

                temp.push_back(firstPoint.z);
                temp.push_back(secondPoint.z);
                temp.push_back(thirdPoint.z);
                temp.push_back(1);

                temp.push_back(1);
                temp.push_back(1);
                temp.push_back(1);
                temp.push_back(1);


                for (int j = 0; j < sz; ++j) {

                    for (int k = 0; k < sz; ++k) {
                        myMatrix[j][k] = temp.at(j * 4 + k);
                    }
                }


                /*cout << "\n\t\tABOUT TO MULTIPLY THE FOLLOWING\n\n";

                print(s.top());
                cout << endl;
                print(myMatrix);
                cout << "\n\n";*/


                float (*resultant)[10];

                resultant = matrixMultiplication(s.top(), myMatrix, sz, sz, sz, sz);
                resultant = balance_W(resultant);

                printMatrix(resultant); //T*I

                print(resultant);

                //showstack(s);

            } else if (command == "scale") {
                // input scaling factors
                // generate the corresponding scaling matrix T
                // S.push(product(S.top,T))

                //go to next line
                i++;

                cout << "do scaling " << endl;
                struct Point scaleFactor;

                //parsing values
                scaleFactor.x = stof(tokens[i][itr].c_str());
                scaleFactor.y = stof(tokens[i][itr + 1].c_str());
                scaleFactor.z = stof(tokens[i][itr + 2].c_str());


                float (*scaleMatrix)[10] = new float[10][10];
                scaleMatrix = makeScalingMatrix(scaleFactor.x, scaleFactor.y, scaleFactor.z);
                //print(scaleMatrix);


                float (*prev)[10];
                float (*New)[10];
                prev = s.top();
                New = matrixMultiplication(prev, scaleMatrix, sz, sz, sz, sz);
                s.push(New);

                //showstack(s);

            } else if (command == "translate") {

                // input translation amounts
                // generate the corresponding translation matrix T
                // S.push(product(S.top,T))

                //go to next line
                i++;

                cout << "do translate " << endl;
                struct Point t;

                //parsing values
                t.x = stof(tokens[i][itr].c_str());
                t.y = stof(tokens[i][itr + 1].c_str());
                t.z = stof(tokens[i][itr + 2].c_str());


                float (*T)[10] = new float[10][10];
                T = makeTranslationMatrix(t.x, t.y, t.z);
                //printMatrix(T);

                float (*prev)[10];
                float (*New)[10];
                prev = s.top();
                New = matrixMultiplication(prev, T, sz, sz, sz, sz);
                s.push(New);

            } else if (command == "rotate") {
                // input rotation angle and axis
                // generate the corresponding rotation matrix T
                // S.push(product(S.top,T))

                //go to next line
                i++;

                cout << "\t\tdo rotate " << endl;
                struct Vector rotateAxis;
                float rotateAngle;

                //parsing values
                rotateAngle = stof(tokens[i][itr].c_str());
                //rotateAngle = degToRad(rotateAngle);

                rotateAxis.x = stof(tokens[i][itr + 1].c_str());
                rotateAxis.y = stof(tokens[i][itr + 2].c_str());
                rotateAxis.z = stof(tokens[i][itr + 3].c_str());

                rotateAxis = normalize(rotateAxis);

                Vector c1, c2, c3;

                Vector iHat(1, 0, 0), jHat(0, 1, 0), kHat(0, 0, 1);

                c1 = rotateRod(iHat, rotateAxis, rotateAngle);
                c2 = rotateRod(jHat, rotateAxis, rotateAngle);
                c3 = rotateRod(kHat, rotateAxis, rotateAngle);

                /*cout << "c1 : " << c1.x << " " << c1.y << " " << c1.z << " " << endl;
                cout << "c2 : " << c2.x << " " << c2.y << " " << c2.z << " " << endl;
                cout << "c3 : " << c3.x << " " << c3.y << " " << c3.z << " " << endl;*/

                float R[10][10];

                vector<float> temp;
                temp.push_back(c1.x);
                temp.push_back(c2.x);
                temp.push_back(c3.x);
                temp.push_back(0);

                temp.push_back(c1.y);
                temp.push_back(c2.y);
                temp.push_back(c3.y);
                temp.push_back(0);

                temp.push_back(c1.z);
                temp.push_back(c2.z);
                temp.push_back(c3.z);
                temp.push_back(0);

                temp.push_back(0);
                temp.push_back(0);
                temp.push_back(0);
                temp.push_back(1);

                for (int j = 0; j < sz; j++) {

                    for (int k = 0; k < sz; k++) {
                        R[j][k] = temp.at(j * 4 + k);
                    }
                }

                cout << "printing rotation matrix" << endl;
                print(R);

                float (*prev)[10];
                float (*New)[10];
                prev = s.top();
                New = matrixMultiplication(prev, R, sz, sz, sz, sz);
                s.push(New);

            } else if (command == "push") {

                cout << "PUSH" << endl;
                Size.push(s.size());

            } else if (command == "pop") {


                cout << "POP" << endl;
                if (s.size() == 1) continue;
                int l = Size.top();
                Size.pop();
                while (s.size() > l) {
                    s.pop();
                }
            } else if (command == "end") {
                break;
            }
        }
    }
    }
    infile.close();
}


void readFromStage1File() {

    // open a file in read mode.
    cout << "\n\n~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage1.txt file  ~~~~~~~~~~~~~~~~~~~~~~~\n\n";

    string line;

    infile2.open("stage1.txt");

    if (!infile2.is_open()) {
    perror("Error open");
    exit(EXIT_FAILURE);
    }

    int line_num = 0;
    vector<string> tokens[200]; // Create vector to hold the words

    while (getline(infile2, line)) {

    string buf;                 // Have a buffer string
    stringstream ss(line);       // Insert the string into a stream

    while (ss >> buf)
        tokens[line_num].push_back(buf);

    line_num++;

    }

    //printing the file content as a 2d array
    //printTokens(tokens, line_num);

    Vector l(0, 0, 0), r(0, 0, 0), u(0, 0, 0);

    l = subtract(look, eye);    //l = look - eye
    l = normalize(l);           //l.normalize()
    r = crossProduct(l, up);    //r = l X up
    r = normalize(r);           // r.normalize()
    u = crossProduct(r, l);     // u = r X l


    //Apply the following translation T to move the eye/camera to origin.
    float (*T)[10] = new float[10][10];

    T = makeIdentityMatrix(sz);
    T[0][sz - 1] = -eye.x;
    T[1][sz - 1] = -eye.y;
    T[2][sz - 1] = -eye.z;
    T[3][sz - 1] = 1;


    //Apply the following rotation R such that the l aligns with the -Z axis, r with X axis, and u with Y axis.
    float (*R)[10] = new float[10][10];
    vector<float> temp = {r.x, r.y, r.z, 0, u.x, u.y, u.z, 0, -l.x, -l.y, -l.z, 0, 0, 0, 0, 1};

    for (int j = 0; j < sz; ++j) {

    for (int k = 0; k < sz; ++k) {
        R[j][k] = temp.at(j * 4 + k);
    }
    }


    float (*V)[10] = new float[10][10];
    V = matrixMultiplication(R, T, sz, sz, sz, sz);
    //printMatrix2(V);

    //V*matrix
    int itr = 0;
    for (int i = 0; i < line_num; i++) {

    if (i == line_num - 1) {
        cout << "breaking" << endl;
        break;
    };
    if (tokens[i].size() == 0) { i++; }

    //input three points
    struct Point firstPoint, secondPoint, thirdPoint;


    firstPoint.x = stof(tokens[i][itr].c_str());
    firstPoint.y = stof(tokens[i][itr + 1].c_str());
    firstPoint.z = stof(tokens[i][itr + 2].c_str());


    i++;
    secondPoint.x = stof(tokens[i][itr].c_str());
    secondPoint.y = stof(tokens[i][itr + 1].c_str());
    secondPoint.z = stof(tokens[i][itr + 2].c_str());

    i++;
    thirdPoint.x = stof(tokens[i][itr].c_str());
    thirdPoint.y = stof(tokens[i][itr + 1].c_str());
    thirdPoint.z = stof(tokens[i][itr + 2].c_str());

    float myMatrix[10][10];

    vector<float> temp;
    temp.push_back(firstPoint.x);
    temp.push_back(secondPoint.x);
    temp.push_back(thirdPoint.x);
    temp.push_back(1);

    temp.push_back(firstPoint.y);
    temp.push_back(secondPoint.y);
    temp.push_back(thirdPoint.y);
    temp.push_back(1);

    temp.push_back(firstPoint.z);
    temp.push_back(secondPoint.z);
    temp.push_back(thirdPoint.z);
    temp.push_back(1);

    temp.push_back(1);
    temp.push_back(1);
    temp.push_back(1);
    temp.push_back(1);


    for (int j = 0; j < sz; ++j) {

        for (int k = 0; k < sz; ++k) {
            myMatrix[j][k] = temp.at(j * 4 + k);
        }
    }

    /*cout << "\t\tABOUT TO MULTIPLY THE FOLLOWING\n\n";
    print(V);
    cout << endl;
    print(myMatrix);
    cout << "\n\n";*/

    float (*resultant)[10];

    resultant = matrixMultiplication(V, myMatrix, sz, sz, sz, sz);

    print(resultant);

    printMatrix2(resultant);

    }
    infile2.close();
}


void readFromStage2File() {
    // open a file in read mode.
    cout << "\n\n~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage2.txt file  ~~~~~~~~~~~~~~~~~~~~~~~\n\n";

    string line;

    infile3.open("stage2.txt");

    if (!infile3.is_open()) {
    perror("Error open");
    exit(EXIT_FAILURE);
    }

    int line_num = 0;
    vector<string> tokens[200]; // Create vector to hold the words

    while (getline(infile3, line)) {

    string buf;                 // Have a buffer string
    stringstream ss(line);       // Insert the string into a stream

    while (ss >> buf)
        tokens[line_num].push_back(buf);

    line_num++;

    }


    //printing the file content as a 2d array
    //printTokens(tokens, line_num);

    float fovX = fovY * aspectRatio;        //fovX = fovY * aspectRatio
    float t = near * Tan(fovY / 2.0);       //t = near * tan(fovY/2)
    float div_r = near * Tan(fovX / 2.0);   //r = near * tan(fovX/2)


    float (*P)[10] = new float[10][10];
    vector<float> temp = {near / div_r, 0, 0, 0, 0, near / t, 0, 0, 0, 0, (-(far + near) / (far - near)),
                      (-(2 * far * near) / (far - near)), 0, 0, -1, 0};

    for (int j = 0; j < sz; ++j) {

    for (int k = 0; k < sz; ++k) {
        P[j][k] = temp.at(j * 4 + k);
    }
    }

    print(P);

    //P*matrix
    int itr = 0;
    for (int i = 0; i < line_num; i++) {

    if (i == line_num - 1) {
        cout << "breaking" << endl;
        break;
    };

    if (tokens[i].size() == 0) { i++; }

    //input three points
    struct Point firstPoint, secondPoint, thirdPoint;


    firstPoint.x = stof(tokens[i][itr].c_str());
    firstPoint.y = stof(tokens[i][itr + 1].c_str());
    firstPoint.z = stof(tokens[i][itr + 2].c_str());

    i++;
    secondPoint.x = stof(tokens[i][itr].c_str());
    secondPoint.y = stof(tokens[i][itr + 1].c_str());
    secondPoint.z = stof(tokens[i][itr + 2].c_str());

    i++;
    thirdPoint.x = stof(tokens[i][itr].c_str());
    thirdPoint.y = stof(tokens[i][itr + 1].c_str());
    thirdPoint.z = stof(tokens[i][itr + 2].c_str());

    float myMatrix[10][10];

    vector<float> temp;
    temp.push_back(firstPoint.x);
    temp.push_back(secondPoint.x);
    temp.push_back(thirdPoint.x);
    temp.push_back(1);

    temp.push_back(firstPoint.y);
    temp.push_back(secondPoint.y);
    temp.push_back(thirdPoint.y);
    temp.push_back(1);

    temp.push_back(firstPoint.z);
    temp.push_back(secondPoint.z);
    temp.push_back(thirdPoint.z);
    temp.push_back(1);

    temp.push_back(1);
    temp.push_back(1);
    temp.push_back(1);
    temp.push_back(1);


    for (int j = 0; j < sz; ++j) {

        for (int k = 0; k < sz; ++k) {
            myMatrix[j][k] = temp.at(j * 4 + k);
        }
    }

    /*cout << "\t\tABOUT TO MULTIPLY THE FOLLOWING\n\n";
    cout << "printing Perspective matrix " << endl;
    print(P);
    cout << endl;
    cout << "stage 2 matrix" << endl;
    print(myMatrix);
    cout << "\n\n";*/

    float (*resultant)[10];

    resultant = matrixMultiplication(P, myMatrix, sz, sz, sz, sz);
    resultant = balance_W(resultant); //have to make w=1
    //print(resultant);

    printMatrix3(resultant); //T*I

    }

}


float
(*matrixMultiplication(float firstMatrix[][10], float secondMatrix[][10], int rowFirst,
                   int columnFirst,
                   int rowSecond, int columnSecond))[10] {

    //cout << "first matrix is :\n";
    // print(firstMatrix);

    //cout << "second matrix is :\n";
    // print(secondMatrix);
    float (*resultantMatrix)[10] = new float[10][10]();
    int i, j, k;

    // multiplying firstMatrix and secondMatrix and storing in array resultantMatrix.
    for (i = 0; i < rowFirst; ++i) {
    for (j = 0; j < columnSecond; ++j) {
        for (k = 0; k < columnFirst; ++k) {
            resultantMatrix[i][j] += firstMatrix[i][k] * secondMatrix[k][j];
        }
    }
    }

    //printing
    for (int l = 0; l < sz; ++l) {
    for (int m = 0; m < sz; ++m) {
        // cout << " [ " << l << "] [" << m << " ] " << resultantMatrix[l][m] << " ";
    }
    //cout << endl;
    }
    return resultantMatrix;
}


float (*makeIdentityMatrix(int identity_sz))[10] {

    float (*identity)[10] = new float[10][10];
    int row, col;

    for (row = 0; row < identity_sz; row++) {
    for (col = 0; col < identity_sz; col++) {

        // Checking if row is equal to column
        if (row == col) {
            identity[row][col] = 1;
        } else {
            identity[row][col] = 0;
        }
    }
    }
    return identity;
}


void showstack(stack<float (*)[10]> s) {

    cout << "\nprinting the full stack\n\n";

    float (*temp)[10];
    stack<float (*)[10]> tempStack;

    tempStack = s;
    while (!tempStack.empty()) {

    temp = tempStack.top();
    printMatrix(temp);
    tempStack.pop();
    }

}

void insertToStack(float (*arr)[10]) {

    cout << "\n\ninserting to stack\n";

    s.push(arr);

    cout << "s.size() : " << s.size() << "\n\n";

}

float (*makeScalingMatrix(float scaleX, float scaleY, float scaleZ))[10] {

    float (*myMatrix)[10] = new float[10][10];

    for (int i = 0; i < sz; ++i) {
    for (int j = 0; j < sz; ++j) {

        if (i == j && i == 0) myMatrix[i][j] = scaleX;
        else if (i == j && i == 1) myMatrix[i][j] = scaleY;
        else if (i == j && i == 2) myMatrix[i][j] = scaleZ;
        else if (i == j && i == 3) myMatrix[i][j] = 1;
        else
            myMatrix[i][j] = 0;

    }
    }
    return myMatrix;
}


float (*makeTranslationMatrix(float transX, float transY, float transZ))[10] {

    float (*myMatrix)[10] = new float[10][10];

    for (int i = 0; i < sz; ++i) {
    for (int j = 0; j < sz; ++j) {

        if (i == j) {
            myMatrix[i][j] = 1;

        } else {
            if (i == 0 && j == 3) myMatrix[i][j] = transX;
            else if (i == 1 && j == 3) myMatrix[i][j] = transY;
            else if (i == 2 && j == 3) myMatrix[i][j] = transZ;
            else myMatrix[i][j] = 0;
        }

    }
    }
    return myMatrix;
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ You should use classes to encapsulate all that global stuff. \$\endgroup\$ Jul 13, 2019 at 10:42
  • 1
    \$\begingroup\$ Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough. \$\endgroup\$
    – greybeard
    Jul 13, 2019 at 11:31

3 Answers 3

5
\$\begingroup\$

General - it might be better to create a matrix class and your own vector class in a namespace.

Allow The Tools to Help You Improve the Code
There are compiler settings that can help you improve your code, these can be specific the the c++ compiler you are using or they can be common. A common c++ compiler switch is -Wall which indicates a errors and warnings should be reported. When I compiled this program there was an error reported as well as many warnings.

1>modeler.cpp
1>c:\users\pacmaninbw\modeler\modeler.cpp(145): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
1>c:\users\pacmaninbw\modeler\modeler.cpp(151): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
1>c:\users\pacmaninbw\modeler\modeler.cpp(157): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data 1>c:\users\pacmaninbw\modeler\modeler.cpp(337): warning C4018: '<': signed/unsigned mismatch
1>c:\users\pacmaninbw\modeler\modeler.cpp(400): warning C4018: '<': signed/unsigned mismatch
1>c:\users\pacmaninbw\modeler\modeler.cpp(628): warning C4018: '>': signed/unsigned mismatch
1>c:\users\pacmaninbw\modeler\modeler.cpp(821): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data
1>c:\users\pacmaninbw\modeler\modeler.cpp(822): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data
1>c:\users\pacmaninbw\modeler\modeler.cpp(178): error C4700: uninitialized local variable 'res' used
1>Done building project "modeler.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

The error reported is in this function:

float dotProduct(const Vector &vec1, const Vector &vec2) {

    float res;

    res += vec1.x * vec2.x + vec1.y * vec2.y + vec1.z * vec2.z;        // ERROR ON THIS LINE.
    if (isNearlyEqual(res, 0)) res = 0;

    return res;
}

The variable res is not initialized prior to being used. The variable res is being used because the += operator says add the following to this variable. There are 2 ways to correct this, either change the += to = or assign zero in the declaration of res.

    float res = 0;

In C++ none of the variables on the stack (local variables in functions and methods) are initialized by the compiler. A good practice is to initialize the variable in the declaration.

Possible Floating Point Errors
As you seem to be aware of with the isNearlyEqual() function due to the binary nature of computer data floating point numbers can't be completely represented on computers, there can always be some very small error. The error can be increased by switching back and forth between types, int to double, double to int, float to double, and double to float. This is why most financial institutions will use separate integers to represent dollars and cents.

It might be better to choose one of the two types for everything, either stick with double or stick with float. I generally use just double because it provides greater precision. The only time that a float variable might be a good choice is if it is a member of a class or struct and space is an issue.

When you do convert from one to the other use a static_cast to do the conversion, that will remove the warning messages and possibly improve accuracy.

Here are two references on floating point numbers and related errors, the first is why floating point numbers can be problems and the second is on floating point error mitigation.

signed/unsigned mismatch
In the for loops where an integer value is being compared to container.size() there is a type mismatch, container.size() is declared as size_t which is currently defined as unsigned int. It might be better to define loop control variables as size_t when they will be compared with container.size().

Avoid using "using namespace std;"
Names spaces were invented to prevent collisions of class and function names from different libraries and modules. This code already introduces a struct/type that could conflict with the std names space (struct Vector). It would be better to get into the habit of prefixing objects from different namespaces with the namespace so that others can maintain the code if necessary. You might also want to make your Vector struct a class and create a namespace for it. A better discussion of this can be found on stackoverflow.com.

Using inline Function Declarations
Using inline in function declarations is generally obsolete. The inline declaration was created as an optimization in the early years of C++. Most modern C++ compilers will properly inline functions as necessary when compiling with -O3. There are times when inlining is not optimal due to cache restrictions and other reasons.

Debugging Code on Code Review
It is generally a good idea to remove debugging code before posting on Code Review, rather than just commenting it out. When you do comment out debugging code it might be better to comment out the for loops as well as the cout statements. This will improve the performance of the program. What might be even better is to move the debugging code that prints an entire matrix into a function where it can be called from multiple functions.

float
(*matrixMultiplication(float firstMatrix[][10], float secondMatrix[][10], int rowFirst,
    int columnFirst,
    int rowSecond, int columnSecond))[10]{

        //cout << "first matrix is :\n";
        // print(firstMatrix);

        //cout << "second matrix is :\n";
        // print(secondMatrix);
        float(*resultantMatrix)[10] = new float[10][10]();
        int i, j, k;

        // multiplying firstMatrix and secondMatrix and storing in array resultantMatrix.
        for (i = 0; i < rowFirst; ++i) {
        for (j = 0; j < columnSecond; ++j) {
            for (k = 0; k < columnFirst; ++k) {
                resultantMatrix[i][j] += firstMatrix[i][k] * secondMatrix[k][j];
            }
        }
        }

        //printing
        for (int l = 0; l < sz; ++l) {
        for (int m = 0; m < sz; ++m) {
            // cout << " [ " << l << "] [" << m << " ] " << resultantMatrix[l][m] << " ";
        }
        //cout << endl;
        }
        return resultantMatrix;
}

For performance reasons it would be better to use "\n" over std::endl especially in loops as shown above. "\n" just inserts a new line, std::endl flushes the output after the new line which means it is making a system call and that really does slow things down.

Indentation
The code above is representative of all the code, a standard practice is to indent the inner loop of all nested loops. Indentation can be an indication of code complexity and can help a code author figure out where they need additional functions. Indentation also helps reviewers and maintainers of the code to read the code.

Code Complexity and Readability
The following code is a little too complex and readability could be improved:

float (*makeTranslationMatrix(float transX, float transY, float transZ))[10] {

    float (*myMatrix)[10] = new float[10][10];

    for (int i = 0; i < sz; ++i) {
    for (int j = 0; j < sz; ++j) {

        if (i == j) {
            myMatrix[i][j] = 1;

        } else {
            if (i == 0 && j == 3) myMatrix[i][j] = transX;
            else if (i == 1 && j == 3) myMatrix[i][j] = transY;
            else if (i == 2 && j == 3) myMatrix[i][j] = transZ;
            else myMatrix[i][j] = 0;
        }

    }
    }
    return myMatrix;
}

In the else clause of the major if statement all of the subsidarary if statements could be made simpler by creating and outer if statement:

        if (j == 3)
        {
            if (i == 0)
            {
                myMatrix[i][j] = transX;
            }
            else if (i == 1)
            {
                myMatrix[i][j] = transY;
            }
            else if (i == 2) 
            {
                myMatrix[i][j] = transZ;
            }
            else
            {
                myMatrix[i][j] = 0;
            }
        }
        else
        {
            myMatrix[i][j] = 0;
        }

This might also improve performance by removing one comparison if the compiler hasn't already optimized it out.

Note: For maintainability it might be better to always put braces {} after if (condition) and else so that maintainers can expand code as necessary without introducing new bugs.

The numbers 0, 1, 2 and 3 aren't clear, it may be better to create symbolic constants with names that indicate what the actual condition is.

\$\endgroup\$
2
  • \$\begingroup\$ 0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ? \$\endgroup\$
    – afsara__
    Jul 13, 2019 at 18:42
  • \$\begingroup\$ @afsara__ No, but some comments about why those particular positions are important might be good. \$\endgroup\$
    – pacmaninbw
    Jul 13, 2019 at 19:11
6
\$\begingroup\$

Lots of good comments already given.

I'll just point out that you should most likely not be writing your own math primitives. It's easy to get wrong, it takes time away from actually creating what you're trying to create, you'll tear your hair out fixing hard to spot bugs and your code (contrary to what most people who write their own math primitives seem to think) will not be faster. Turns out, the people who write math libraries are experienced and have had much longer time than you to debug, design, test and optimize the libraries than you have.

That said, there are two good reasons to write math primitives: you're doing it to learn the maths or libraries or functions you need are not available for your platform, licensing requirements, algorithms you need are missing etc.

There are many good libraries out there, personally I like to use Eigen. I have no affiliation with the project; I just like their API, they have good performance and don't need to deal with distributing libraries or linking.

\$\endgroup\$
3
\$\begingroup\$

I see some things that I think could help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Eliminate global variables where practical

Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for error. Eliminating global variables where practical is always a good idea, and can be done here by moving them as local variables to the only place they're used, as with infile and/or pass them as parameters.

Use constexpr for values that could be computed at compile time

The values of PI and EPS could be declared constexpr or probably better would be static constexpr. See Con.5.

Be careful with signed and unsigned

In several places, the code compares an int i with tokens[j].size() or similar. However, tokens[j].size() is unsigned and i is signed. For consistency, it would be better to declare i as std::size_t which is the type returned by size().

Eliminate unused parameters

The rowSecond parameter to matrixMultiplication is unused and should be deleted.

Don't define a default constructor that only initializes data members

The Vector constructor is currently this:

Vector() {
x = x;
y = y;
z = z;
};

Not only does this not make sense, better would be to use in-class member initializers and delete this constructor in favor of Vector() = default;. See C.45

Consider the user

Instead of having hardcoded filenames, it might be nice to allow the user to control the name and location of the input and output files. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.

Use better names

I would expect a function named readFromFile to read... something... from a file. No more and no less. However, what this function actually does is to read the file and perform some operation on that data and write that resulting data to yet another file. I'd suggest breaking each of those into its own function and then naming each piece more appropriately.

Fix the bug

The dotProduct() function includes these lines:

float dotProduct(const Vector &vec1, const Vector &vec2) {
    float res;
    res += vec1.x * vec2.x + vec1.y * vec2.y + vec1.z * vec2.z;

The problem is that by using += the code is using an uninitialized variable. Better would be to use =.

Eliminate "magic numbers"

This code has a number of inscrutable "magic numbers," that is, unnamed constants such as 10, 100, 200, etc. Generally it's better to avoid that and give such constants meaningful names. That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "10" and then trying to determine if this particular 10 is relevant to the desired change or if it is some other constant that happens to have the same value.

Don't Repeat Yourself (DRY)

The various printMatrix routines are exactly alike except for the file they write to. That is a strong indicator they should instead be a single function with the ostream passed as a parameter. When you consolidate them, you will see that there's a subtle difference in the way one of them prints to the console.

Don't leak memory

At the moment, the several calls to new have no corresponding calls to delete which is a memory leak. That should be fixed. See C.31

Don't use std::endl if you don't really need it

The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.

Define operations using operators

The code includes functions like this:

Vector add(Vector v1, Vector v2) {
    Vector ret(0, 0, 0);
    ret.x = v1.x + v2.x;
    ret.y = v1.y + v2.y;
    ret.z = v1.z + v2.z;
    return ret;
}

This makes more sense to me expressed as an operator member function of Vector. First define operator+=:

Vector &operator+=(const Vector& other) {
    x += other.x;
    y += other.y;
    z += other.z;
    return *this;
}

Then define a free standing functions using that function:

Vector operator+(Vector a, const Vector& b) {
    return a += b;
}

Now instead of this:

Vector temp4 = add(temp1, temp3);

We can write this:

auto temp4 = temp1 + temp3;

And instead of

l = subtract(look, eye);    //l = look - eye

We can simply write:

l = look - eye;

and render the comment completely redundant because the code itself is clear. This also allows considerable simplification elsewhere. For example the rotateRod function becomes this rather than the current 33 line function:

Vector rotateRod(Vector x, Vector rotateAxis, float rotateAngle) {
    auto ra{rotateAxis};
    rotateAxis *= rotateAxis.dotProduct(x) * (1 - Cos(rotateAngle)); 
    return x*Cos(rotateAngle) + ra.cross(x)*Sin(rotateAngle) + rotateAxis;
}

Make better use of objects

Everywhere that something like (float (*matrix)[10]) occurs is probably much better expressed as an object. I'd also suggest minimally using a std::matrix<float, 10> rather than a raw array because the latter type is quite stupid and doesn't even know its own size.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.