Modeling, view and projection transformation using vector and point in homogenous formSerialize/deserialize a...

How to check the quality of an audio sample?

What would be the ideal melee weapon made of "Phase Metal"?

Interpreting the word "randomly"

How can an advanced civilization forget how to manufacture its technology?

How do Windows version numbers work?

What is the English equivalent of 干物女 (dried fish woman)?

What does "Fotze" really mean?

How to repair a laptop's screen hinges?

How might the United Kingdom become a republic?

Why does the autopilot disengage even when it does not receive pilot input?

Does Google Maps take into account hills/inclines for route times?

Is this floating-point optimization allowed?

Find values of x so that the matrix is invertible

What is temperature on a quantum level?

The monorail explodes before I can get on it

Alternatives to using writing paper for writing practice

What are some symbols representing peasants/oppressed persons fighting back?

Was the Ford Model T black because of the speed black paint dries?

When did the Roman Empire fall according to contemporaries?

What would the EU do if an EU member declared war on another EU member?

Did any of the founding fathers anticipate Lysander Spooner's criticism of the constitution?

Back to the nineties!

School House Points (Python + SQLite)

Why did the Japanese attack the Aleutians at the same time as Midway?



Modeling, view and projection transformation using vector and point in homogenous form


Serialize/deserialize a vector using insertion/extraction operatorsGeneric vector implementation in C using macrosMultiplayer BlackJack using VectorRotation view for std::vectorMaking Math vector implementation using javaGeneric Vector Class using smart_pointersManipulating and maintaining vectorQsort in C++ using vectorSIMD product of a 4×4 matrix and a vectorData Structures for Counting Duplicates and using std::vector::erase






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty{ margin-bottom:0;
}







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 << "nnormalizingn[ " << 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 2n";

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 3n";
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 consolen";
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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the scene.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "nttABOUT TO MULTIPLY THE FOLLOWINGnn";

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


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 << "ttdo 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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage1.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
print(V);
cout << endl;
print(myMatrix);
cout << "nn";*/

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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage2.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
cout << "printing Perspective matrix " << endl;
print(P);
cout << endl;
cout << "stage 2 matrix" << endl;
print(myMatrix);
cout << "nn";*/

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 stacknn";

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 << "nninserting to stackn";

s.push(arr);

cout << "s.size() : " << s.size() << "nn";

}

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









share|improve this question







New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






$endgroup$








  • 2




    $begingroup$
    You should use classes to encapsulate all that global stuff.
    $endgroup$
    – πάντα ῥεῖ
    13 hours ago










  • $begingroup$
    Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough.
    $endgroup$
    – greybeard
    12 hours ago


















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 << "nnormalizingn[ " << 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 2n";

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 3n";
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 consolen";
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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the scene.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "nttABOUT TO MULTIPLY THE FOLLOWINGnn";

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


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 << "ttdo 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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage1.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
print(V);
cout << endl;
print(myMatrix);
cout << "nn";*/

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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage2.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
cout << "printing Perspective matrix " << endl;
print(P);
cout << endl;
cout << "stage 2 matrix" << endl;
print(myMatrix);
cout << "nn";*/

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 stacknn";

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 << "nninserting to stackn";

s.push(arr);

cout << "s.size() : " << s.size() << "nn";

}

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









share|improve this question







New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






$endgroup$








  • 2




    $begingroup$
    You should use classes to encapsulate all that global stuff.
    $endgroup$
    – πάντα ῥεῖ
    13 hours ago










  • $begingroup$
    Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough.
    $endgroup$
    – greybeard
    12 hours ago














2












2








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 << "nnormalizingn[ " << 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 2n";

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 3n";
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 consolen";
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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the scene.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "nttABOUT TO MULTIPLY THE FOLLOWINGnn";

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


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 << "ttdo 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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage1.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
print(V);
cout << endl;
print(myMatrix);
cout << "nn";*/

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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage2.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
cout << "printing Perspective matrix " << endl;
print(P);
cout << endl;
cout << "stage 2 matrix" << endl;
print(myMatrix);
cout << "nn";*/

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 stacknn";

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 << "nninserting to stackn";

s.push(arr);

cout << "s.size() : " << s.size() << "nn";

}

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









share|improve this question







New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






$endgroup$




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 << "nnormalizingn[ " << 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 2n";

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 3n";
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 consolen";
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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the scene.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "nttABOUT TO MULTIPLY THE FOLLOWINGnn";

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


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 << "ttdo 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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage1.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
print(V);
cout << endl;
print(myMatrix);
cout << "nn";*/

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 << "nn~~~~~~~~~~~~~~~~~~~~~~~~ Reading from the stage2.txt file ~~~~~~~~~~~~~~~~~~~~~~~nn";

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 << "ttABOUT TO MULTIPLY THE FOLLOWINGnn";
cout << "printing Perspective matrix " << endl;
print(P);
cout << endl;
cout << "stage 2 matrix" << endl;
print(myMatrix);
cout << "nn";*/

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 stacknn";

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 << "nninserting to stackn";

s.push(arr);

cout << "s.size() : " << s.size() << "nn";

}

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






c++ performance matrix complexity vectors






share|improve this question







New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.










share|improve this question







New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.








share|improve this question




share|improve this question






New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.








asked 14 hours ago









afsara__afsara__

183 bronze badges




183 bronze badges




New contributor



afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




New contributor




afsara__ is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.










  • 2




    $begingroup$
    You should use classes to encapsulate all that global stuff.
    $endgroup$
    – πάντα ῥεῖ
    13 hours ago










  • $begingroup$
    Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough.
    $endgroup$
    – greybeard
    12 hours ago














  • 2




    $begingroup$
    You should use classes to encapsulate all that global stuff.
    $endgroup$
    – πάντα ῥεῖ
    13 hours ago










  • $begingroup$
    Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough.
    $endgroup$
    – greybeard
    12 hours ago








2




2




$begingroup$
You should use classes to encapsulate all that global stuff.
$endgroup$
– πάντα ῥεῖ
13 hours ago




$begingroup$
You should use classes to encapsulate all that global stuff.
$endgroup$
– πάντα ῥεῖ
13 hours ago












$begingroup$
Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough.
$endgroup$
– greybeard
12 hours ago




$begingroup$
Where […] should I be commenting? everywhere an educated guess at what is this? doesn't promise to be enough.
$endgroup$
– greybeard
12 hours ago










3 Answers
3






active

oldest

votes


















4












$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:userspacmaninbwmodelermodeler.cpp(145): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

1>c:userspacmaninbwmodelermodeler.cpp(151): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

1>c:userspacmaninbwmodelermodeler.cpp(157): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
1>c:userspacmaninbwmodelermodeler.cpp(337): warning C4018: '<': signed/unsigned mismatch

1>c:userspacmaninbwmodelermodeler.cpp(400): warning C4018: '<': signed/unsigned mismatch

1>c:userspacmaninbwmodelermodeler.cpp(628): warning C4018: '>': signed/unsigned mismatch

1>c:userspacmaninbwmodelermodeler.cpp(821): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

1>c:userspacmaninbwmodelermodeler.cpp(822): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

1>c:userspacmaninbwmodelermodeler.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.






share|improve this answer









$endgroup$













  • $begingroup$
    0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
    $endgroup$
    – afsara__
    5 hours ago










  • $begingroup$
    @afsara__ No, but some comments about why those particular positions are important might be good.
    $endgroup$
    – pacmaninbw
    5 hours ago





















2












$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.






share|improve this answer









$endgroup$





















    2












    $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.






    share|improve this answer









    $endgroup$
















      Your Answer






      StackExchange.ifUsing("editor", function () {
      StackExchange.using("externalEditor", function () {
      StackExchange.using("snippets", function () {
      StackExchange.snippets.init();
      });
      });
      }, "code-snippets");

      StackExchange.ready(function() {
      var channelOptions = {
      tags: "".split(" "),
      id: "196"
      };
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function() {
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled) {
      StackExchange.using("snippets", function() {
      createEditor();
      });
      }
      else {
      createEditor();
      }
      });

      function createEditor() {
      StackExchange.prepareEditor({
      heartbeatType: 'answer',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader: {
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      },
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      });


      }
      });






      afsara__ is a new contributor. Be nice, and check out our Code of Conduct.










      draft saved

      draft discarded


















      StackExchange.ready(
      function () {
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f224085%2fmodeling-view-and-projection-transformation-using-vector-and-point-in-homogenou%23new-answer', 'question_page');
      }
      );

      Post as a guest















      Required, but never shown

























      3 Answers
      3






      active

      oldest

      votes








      3 Answers
      3






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes









      4












      $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:userspacmaninbwmodelermodeler.cpp(145): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(151): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(157): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
      1>c:userspacmaninbwmodelermodeler.cpp(337): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(400): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(628): warning C4018: '>': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(821): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(822): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.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.






      share|improve this answer









      $endgroup$













      • $begingroup$
        0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
        $endgroup$
        – afsara__
        5 hours ago










      • $begingroup$
        @afsara__ No, but some comments about why those particular positions are important might be good.
        $endgroup$
        – pacmaninbw
        5 hours ago


















      4












      $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:userspacmaninbwmodelermodeler.cpp(145): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(151): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(157): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
      1>c:userspacmaninbwmodelermodeler.cpp(337): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(400): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(628): warning C4018: '>': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(821): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(822): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.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.






      share|improve this answer









      $endgroup$













      • $begingroup$
        0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
        $endgroup$
        – afsara__
        5 hours ago










      • $begingroup$
        @afsara__ No, but some comments about why those particular positions are important might be good.
        $endgroup$
        – pacmaninbw
        5 hours ago
















      4












      4








      4





      $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:userspacmaninbwmodelermodeler.cpp(145): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(151): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(157): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
      1>c:userspacmaninbwmodelermodeler.cpp(337): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(400): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(628): warning C4018: '>': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(821): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(822): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.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.






      share|improve this answer









      $endgroup$



      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:userspacmaninbwmodelermodeler.cpp(145): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(151): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(157): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
      1>c:userspacmaninbwmodelermodeler.cpp(337): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(400): warning C4018: '<': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(628): warning C4018: '>': signed/unsigned mismatch

      1>c:userspacmaninbwmodelermodeler.cpp(821): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.cpp(822): warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data

      1>c:userspacmaninbwmodelermodeler.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.







      share|improve this answer












      share|improve this answer



      share|improve this answer










      answered 9 hours ago









      pacmaninbwpacmaninbw

      6,8232 gold badges17 silver badges40 bronze badges




      6,8232 gold badges17 silver badges40 bronze badges












      • $begingroup$
        0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
        $endgroup$
        – afsara__
        5 hours ago










      • $begingroup$
        @afsara__ No, but some comments about why those particular positions are important might be good.
        $endgroup$
        – pacmaninbw
        5 hours ago




















      • $begingroup$
        0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
        $endgroup$
        – afsara__
        5 hours ago










      • $begingroup$
        @afsara__ No, but some comments about why those particular positions are important might be good.
        $endgroup$
        – pacmaninbw
        5 hours ago


















      $begingroup$
      0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
      $endgroup$
      – afsara__
      5 hours ago




      $begingroup$
      0,1,2,3 indicates the row positions. should i have used define row0 0; define row1 1; ?
      $endgroup$
      – afsara__
      5 hours ago












      $begingroup$
      @afsara__ No, but some comments about why those particular positions are important might be good.
      $endgroup$
      – pacmaninbw
      5 hours ago






      $begingroup$
      @afsara__ No, but some comments about why those particular positions are important might be good.
      $endgroup$
      – pacmaninbw
      5 hours ago















      2












      $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.






      share|improve this answer









      $endgroup$


















        2












        $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.






        share|improve this answer









        $endgroup$
















          2












          2








          2





          $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.






          share|improve this answer









          $endgroup$



          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.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered 3 hours ago









          EdwardEdward

          49.9k3 gold badges83 silver badges222 bronze badges




          49.9k3 gold badges83 silver badges222 bronze badges























              2












              $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.






              share|improve this answer









              $endgroup$


















                2












                $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.






                share|improve this answer









                $endgroup$
















                  2












                  2








                  2





                  $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.






                  share|improve this answer









                  $endgroup$



                  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.







                  share|improve this answer












                  share|improve this answer



                  share|improve this answer










                  answered 3 hours ago









                  Emily L.Emily L.

                  13.2k1 gold badge28 silver badges78 bronze badges




                  13.2k1 gold badge28 silver badges78 bronze badges






















                      afsara__ is a new contributor. Be nice, and check out our Code of Conduct.










                      draft saved

                      draft discarded


















                      afsara__ is a new contributor. Be nice, and check out our Code of Conduct.













                      afsara__ is a new contributor. Be nice, and check out our Code of Conduct.












                      afsara__ is a new contributor. Be nice, and check out our Code of Conduct.
















                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid



                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.


                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function () {
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f224085%2fmodeling-view-and-projection-transformation-using-vector-and-point-in-homogenou%23new-answer', 'question_page');
                      }
                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown







                      Popular posts from this blog

                      Hudson River Historic District Contents Geography History The district today Aesthetics Cultural...

                      The number designs the writing. Feandra Aversely Definition: The act of ingrafting a sprig or shoot of one...

                      Ayherre Geografie Demografie Externe links Navigatiemenu43° 23′ NB, 1° 15′ WL43° 23′ NB, 1°...