12
\$\begingroup\$

This is something I put together from a 3D Soft Renderer I previously wrote in Java. I have less experience in C++, but I thought that being able to render 3D shapes in a console simply by printing characters (Like Perspective ASCII Art) would be a fun challenge.

This was mainly for educational and learning purposes and less of a performance concern. How do you all think this program accomplishes the task from a more novice C++ programmer's approach? I personally feel accomplished that I was able to do this, but am curious as to if I am making some horrible mistakes somewhere in my code. I would like to learn more about proper object oriented programming, along with good C++ programming practices. Any code-cleanup suggestions?

Edit: Here are a few screenshots https://imgur.com/a/5C1eG

Main.cpp

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#include <iostream>
#include "Renderer.h"


using namespace std;

int main(){
    Renderer r;
    cin.get();
    return 0;
}

Renderer.cpp

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#include "Renderer.h"

#define PI 3.14159265358979323846264

Renderer::Renderer(void){
    viewpoint = new Vector(0, 0, 0);
    viewangle = new Vector(0, 0, PI);
    c0 = new Cube(0, 0, 0, 10);
    c1 = new Cube(10, 10, 10, 10);

    buildMatrix();

    HANDLE hInput = GetStdHandle(STD_INPUT_HANDLE);
    DWORD NumInputs = 0;
    DWORD InputsRead = 0;
    INPUT_RECORD irInput;
    GetNumberOfConsoleInputEvents(hInput, &NumInputs);

    while(true){
        ReadConsoleInput(hInput, &irInput, 1, &InputsRead);
        switch(irInput.Event.KeyEvent.wVirtualKeyCode){
            case 0x57: //W
                if(flight){
                    viewpoint->y += sin(viewangle->y) * movement;
                    viewpoint->x += cos(viewangle->x + (PI / 2.0)) * (cos(viewangle->y) * movement);
                    viewpoint->z += sin(viewangle->x + (PI / 2.0)) * (cos(viewangle->y) * movement);
                }else{
                    viewpoint->x += cos(viewangle->x + (PI / 2.0)) * movement;
                    viewpoint->z += sin(viewangle->x + (PI / 2.0)) * movement;
                }
                render();
                break;
            case 0x41: //A
                viewpoint->x += cos(viewangle->x) * movement;
                viewpoint->z += sin(viewangle->x) * movement;
                render();
                break;
            case 0x53: //S
                if(flight){
                    viewpoint->y -= sin(viewangle->y) * movement;
                    viewpoint->x += cos(viewangle->x - (PI / 2.0)) * (cos(viewangle->y) * movement);
                    viewpoint->z += sin(viewangle->x - (PI / 2.0)) * (cos(viewangle->y) * movement);
                }else{
                    viewpoint->x += cos(viewangle->x - (PI / 2.0)) * movement;
                    viewpoint->z += sin(viewangle->x - (PI / 2.0)) * movement;
                }
                render();
                break;
            case 0x44: //D
                viewpoint->x -= cos(viewangle->x) * movement;
                viewpoint->z -= sin(viewangle->x) * movement;
                render();
                break;
            case 0x45: //E
                viewpoint->y += 1;
                render();
                break;
            case 0x51: //Q
                viewpoint->y -= 1;
                render();
                break;
            case VK_LEFT:
                viewangle->x -= 0.1;
                viewangle->x = fmod(viewangle->x, 2.0 * PI);
                render();
                break;
            case VK_RIGHT:
                viewangle->x += 0.1;
                viewangle->x = fmod(viewangle->x, 2.0 * PI);
                render();
                break;
            case VK_UP:
                if(viewangle->y < (PI / 2.0)){
                    viewangle->y += 0.1;
                }
                render();
                break;
            case VK_DOWN:
                if(viewangle->y > -(PI / 2.0)){
                    viewangle->y -= 0.1;
                }
                render();
                break;
            case 0x52: //R
                viewpoint->x = 0.0;
                viewpoint->y = 0.0;
                viewpoint->z = 0.0;
                viewangle->x = 0.0;
                viewangle->y = 0.0;
                viewangle->z = PI;
                render();
                break;
        }
    }
}

void Renderer::buildMatrix(void){
    double frustumDepth =  (_far - _near);
    double oneOverDepth = 1.0 / frustumDepth;

    matrix[1] = 0.0;
    matrix[2] = 0.0;
    matrix[3] = 0.0;
    matrix[4] = 0.0;
    matrix[5] = 1.0 / tan(0.5 * FOV);

    matrix[0] = matrix[5] / aspectRatio;

    matrix[6] = 0.0;
    matrix[7] = 0.0;
    matrix[8] = 0.0;
    matrix[9] = 0.0;
    matrix[10] = _far * oneOverDepth;
    matrix[11] = 1.0;
    matrix[12] = 0.0;
    matrix[13] = 0.0;
    matrix[14] = (-_far * _near) * oneOverDepth;
    matrix[15] = 0.0;

    return;
}

Vector Renderer::toPerspective(Vector point){
    Vector vect;

    double ax = viewangle->x;
    double ay = viewangle->y;
    double az = viewangle->z;

    double ix = point.x - viewpoint->x;
    double iy = point.y - viewpoint->y;
    double iz = point.z - viewpoint->z;
    double iw = point.w - viewpoint->w;

    double x1=iz*sin(ax)+ix*cos(ax);
    double y1=iy;
    double z1=iz*cos(ax)-ix*sin(ax);

    double x11=x1;
    double y11=y1*cos(ay)-z1*sin(ay);
    double z11=y1*sin(ay)+z1*cos(ay);

    double x111=y11*sin(az)+x11*cos(az);
    double y111=y11*cos(az)-x11*sin(az);
    double z111=z11;

    ix = x111;
    iy = y111;
    iz = z111;

    double ox = matrix[0] * ix + matrix[4] * iy + matrix[8] * iz + matrix[12] * iw;
    double oy = matrix[1] * ix + matrix[5] * iy + matrix[9] * iz + matrix[13] * iw;
    double oz = matrix[2] * ix + matrix[6] * iy + matrix[10] * iz + matrix[14] * iw;
    double ow = matrix[3] * ix + matrix[7] * iy + matrix[11] * iz + matrix[15] * iw;

    vect.x = ((ox * WIDTH) / (2.0 * ow) + (WIDTH / 2.0));
    vect.y = ((oy * HEIGHT) / (2.0 * ow) + (HEIGHT / 2.0));
    vect.z = (oz);
    vect.w = (ow);

    return vect;
}

void Renderer::render(void){
    for(int x = 0; x < WIDTH; x++){
        for(int y = 0; y < HEIGHT; y++){
            set(x,y, " ");
        }
    }

    c0->draw(this);
    c1->draw(this);

    return;
}

void Renderer::makeLine(Vector pointA, Vector pointB){
    Vector pointAProjected = toPerspective(pointA);
    Vector pointBProjected = toPerspective(pointB);
    if(pointAProjected.z >= this->_near && pointBProjected.z >= this->_near){
        this->drawLine(pointAProjected.x, pointAProjected.y, pointBProjected.x, pointBProjected.y);
    }else if(pointAProjected.z >= this->_near && pointBProjected.z < this->_near){
        double n = (pointAProjected.w - this->_near) / (pointAProjected.w - pointBProjected.w);
        double xc = (n * pointAProjected.x) + ((1-n) * pointBProjected.x);
        double yc = (n * pointAProjected.y) + ((1-n) * pointBProjected.y);
        double zc = (n * pointAProjected.z) + ((1-n) * pointBProjected.z);
        pointBProjected.x = xc;
        pointBProjected.y = yc;
        pointBProjected.z = zc;
        this->drawLine(pointAProjected.x, pointAProjected.y, pointBProjected.x, pointBProjected.y);
    }else if(pointAProjected.z < this->_near && pointBProjected.z >= this->_near){
        double n = (pointBProjected.w - this->_near) / (pointBProjected.w - pointAProjected.w);
        double xc = (n * pointBProjected.x) + ((1-n) * pointAProjected.x);
        double yc = (n * pointBProjected.y) + ((1-n) * pointAProjected.y);
        double zc = (n * pointBProjected.z) + ((1-n) * pointAProjected.z);
        pointAProjected.x = xc;
        pointAProjected.y = yc;
        pointAProjected.z = zc;
        this->drawLine(pointAProjected.x, pointAProjected.y, pointBProjected.x, pointBProjected.y);
    }
    return;
}

void Renderer::drawLine(double x1, double y1, double x2, double y2){
    // Bresenham's line algorithm
    //cout << x1 << ":" << y1 << " # " << x2 << ":" << y2 << endl;
    const bool steep = (fabs(y2 - y1) > fabs(x2 - x1));

    if(steep){
        swap(x1, y1);
        swap(x2, y2);
    }

    if(x1 > x2){
        swap(x1, x2);
        swap(y1, y2);
    }

    const double dx = x2 - x1;
    const double dy = fabs(y2 - y1);

    double error = dx / 2.0f;
    const int ystep = (y1 < y2) ? 1 : -1;
    int y = (int)y1;

    const int maxX = (int)x2;

    for(int x=(int)x1; x<maxX; x++){
        if(steep){
            set(y,x, "X");//.c_str());
        }else{
            set(x,y, "X");//.c_str());
        }

        error -= dy;
        if(error < 0){
            y += ystep;
            error += dx;
        }
    }
    return;
}

void Renderer::set(int x, int y, string s){
    if(x < WIDTH && x >= 0 && y < HEIGHT && y >= 0){
        COORD p;
        p.X = x;
        p.Y = y;
        if(SetConsoleCursorPosition(GetStdHandle( STD_OUTPUT_HANDLE ), p) == 0) cout << "Something broke yo!";
        cout << s;
    }
    return;
}

Renderer.h

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#ifndef RENDERER
#define RENDERER

#include <math.h>
#include <iostream>
#include <windows.h>
#include <stdio.h>
#include "Vector.h"
#include "Cube.h"
#include <string>
#include <stdarg.h>

class Cube;
class Vector;

using namespace std;

class Renderer{
    public:
        static const double FOV = 0.6;
        static const int HEIGHT = 25;
        static const int WIDTH = 40;
        static const double _near = 0.0;
        static const double _far = 40.0;
        static const double aspectRatio = (1.0 * WIDTH) / (1.0 * HEIGHT);
        static const double movement = 1.0;
        static const bool flight = true;

        double matrix[16];

        Vector *viewpoint;
        Vector *viewangle;

        Cube *c0;
        Cube *c1;

        Renderer(void);
        void buildMatrix(void);
        Vector toPerspective(Vector point);
        void render(void);
        void makeLine(Vector pointA, Vector pointB);
        void drawLine(double x1, double y1, double x2, double y2);
        void set(int x, int y, string s);
};

#endif

Vector.cpp

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#include "Vector.h"

Vector::Vector(void){
    this->x = 0;
    this->y = 0;
    this->z = 0;
    this->w = 1;
}
Vector::Vector(double x, double y, double z){
    this->x = x;
    this->y = y;
    this->z = z;
    this->w = 1;
}
Vector::Vector(double x, double y, double z, double w){
    this->x = x;
    this->y = y;
    this->z = z;
    this->w = w;
}
Vector Vector::normalize(void){
    Vector vect;
    vect.x = this->x / this->getLength();
    vect.y = this->y / this->getLength();
    vect.z = this->z / this->getLength();
    return vect;
}
double Vector::getLength(void){
    return sqrt(this->x * this->x + this->y * this->y + this->z * this->z);
}

Vector.h

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#ifndef VECTOR
#define VECTOR

#include <iostream>
#include <math.h>

using namespace std;

class Vector {
    public:
        Vector(void);
        Vector(double x, double y, double z);
        Vector(double x, double y, double z, double w);
        Vector normalize(void);
        double getLength(void);
        double x;
        double y;
        double z;
        double w;
};

#endif

Cube.cpp

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#include "Cube.h"

Cube::Cube(double x, double y, double z, double len){
    s0.x = x;
    s0.y = y;
    s0.z = z;

    s1.x = x + len;
    s1.y = y;
    s1.z = z;

    s2.x = x;
    s2.y = y + len;
    s2.z = z;

    s3.x = x;
    s3.y = y;
    s3.z = z + len;

    s4.x = x + len;
    s4.y = y + len;
    s4.z = z;

    s5.x = x;
    s5.y = y + len;
    s5.z = z + len;

    s6.x = x + len;
    s6.y = y;
    s6.z = z + len;

    s7.x = x + len;
    s7.y = y + len;
    s7.z = z + len;
}
void Cube::draw(Renderer* r){
    r->makeLine(s0, s1);
    r->makeLine(s1, s4);
    r->makeLine(s4, s2);
    r->makeLine(s2, s0);
    r->makeLine(s0, s3);
    r->makeLine(s3, s5);
    r->makeLine(s5, s7);
    r->makeLine(s7, s4);
    r->makeLine(s5, s2);
    r->makeLine(s7, s6);
    r->makeLine(s6, s3);
    r->makeLine(s6, s1);
    return;
}

Cube.h

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#ifndef CUBE
#define CUBE

#include <iostream>
#include "Renderer.h"
#include "Vector.h"

class Renderer;
class Vector;

using namespace std;

class Cube {
    public:
        Cube(double x, double y, double z, double w);
        void draw(Renderer* r);
        Vector s0;
        Vector s1;
        Vector s2;
        Vector s3;
        Vector s4;
        Vector s5;
        Vector s6;
        Vector s7;
};

#endif
\$\endgroup\$
  • 10
    \$\begingroup\$ Aww, I was sooo hoping for a screenshot! :-) \$\endgroup\$ – Mathieu Guindon Dec 22 '15 at 18:27
  • 1
    \$\begingroup\$ I would like to thank everyone for their suggestions and help. It is difficult to choose one correct answer for this review as both answers give their own unique takes for improvement. \$\endgroup\$ – Mr Public Dec 22 '15 at 19:42
  • 1
    \$\begingroup\$ @Mat'sMug imgur.com/a/5C1eG Here are a few screenshots. \$\endgroup\$ – Mr Public Dec 22 '15 at 19:52
7
\$\begingroup\$

Seeing that you've come from Java, there are three main critiques that I have for you:

Encapsulation

It seems that you're exposing many of your class' data members unnecessarily. In particular, for your Renderer class, none of the marked public data members are used (or supposed to be used) by outside code. All of these should be marked private.

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#ifndef RENDERER
#define RENDERER

#include <math.h>
#include <iostream>
#include <windows.h>
#include <stdio.h>
#include "Vector.h"
#include "Cube.h"
#include <string>
#include <stdarg.h>

class Cube;
class Vector;

using namespace std;

class Renderer{
private:
        static const double FOV = 0.6;
        static const int HEIGHT = 25;
        static const int WIDTH = 40;
        static const double _near = 0.0;
        static const double _far = 40.0;
        // can't this be calculated ahead of time?
        static const double aspectRatio = (1.0 * WIDTH) / (1.0 * HEIGHT);
        static const double movement = 1.0;
        static const bool flight = true;

        double matrix[16];

        Vector *viewpoint;
        Vector *viewangle;

        Cube *c0;
        Cube *c1;
public:
        Renderer(void);
        void buildMatrix(void);
        Vector toPerspective(Vector point);
        void render(void);
        void makeLine(Vector pointA, Vector pointB);
        void drawLine(double x1, double y1, double x2, double y2);
        void set(int x, int y, string s);
};

#endif

Constructor Initializer List and Marking Member Functions const

These should be used when constructors are being used to trivially initialize its class' data members. Also, the use of (void) to indicate there are no parameters to the function isn't necessary in C++. Member functions of a class that don't modify the state of the class should be marked const (your getLength() function). Example is in your Vector code (although Cube and Renderer could use similar fixes:

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#include "Vector.h"

Vector::Vector()
    : x(0), y(0), z(0), w(1) {}

Vector::Vector(double xx, double yy, double zz)
    : x(xx), y(yy), z(zz), w(1) { }

Vector::Vector(double xx, double yy, double zz, double ww)
    : x(xx), y(yy), z(zz), w(ww)  {}

Vector Vector::normalize() const {
    Vector vect;
    const double length = getLength(); // no need to recalculate this
    vect.x = x / length
    vect.y = y / length
    vect.z = z / length
    return vect;
}

double Vector::getLength() const {
    return sqrt(x * x + y * y + z * z);
}

Memory Management

Since this is C++ and there is no GC, you need to get used to the habit of managing your memory early. One way to quickly do that is to avoid the use of new as much as possible (unless new is in the constructor of a class and there's a corresponding delete for it in the destructor). As it is now, your code leaks two Vectors and two Cubes worth of memory. This isn't a big deal for this particular application since your OS will just clean up that memory for you when the process exits but it's something to note. In order to remedy this, remove your raw pointers from your code and instead take a look at the <memory> header from C++. You should be using std::unique_ptr in this situation. With std::unique_ptr, you no longer need keyword new and memory management is done automatically for you.

Possible code modification:

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#ifndef RENDERER
#define RENDERER

#include <math.h>
#include <iostream>
#include <windows.h>
#include <stdio.h>
#include "Vector.h"
#include "Cube.h"
#include <string>
#include <stdarg.h>
#include <memory>

class Cube;
class Vector;

using namespace std;

class Renderer{
private:
        static const double FOV = 0.6;
        static const int HEIGHT = 25;
        static const int WIDTH = 40;
        static const double _near = 0.0;
        static const double _far = 40.0;
        static const double aspectRatio = (1.0 * WIDTH) / (1.0 * HEIGHT);
        static const double movement = 1.0;
        static const bool flight = true;

        double matrix[16];

        std::unique_ptr<Vector> viewpoint;
        std::unique_ptr<Vector> viewangle;

        std::unique_ptr<Cube> c0;
        std::unique_ptr<Cube> c1;

public:
        Renderer(void);
        void buildMatrix(void);
        Vector toPerspective(Vector point);
        void render(void);
        void makeLine(Vector pointA, Vector pointB);
        void drawLine(double x1, double y1, double x2, double y2);
        void set(int x, int y, string s);
};

#endif

For implementation:

/*
LEEJAE KARINJA
C++ Programming
Copyright (C) 2015 Leejae Karinja
*/

#include "Renderer.h"

Renderer::Renderer(void)
    : viewpoint(make_unique<Vector>(0, 0, 0)),
    : viewangle(make_unique<Vecotr>(0, 0, PI)),
    : c0(make_unique<Cube>(0,0,0,10),
    : c1(make_unique<Cube>(10, 10, 10, 10)
{
    // ......
}

Macros

Avoid the use of macros when a static const variable would be better. In this case PI can also be made declared as static const double PI = //... in Renderer.cpp

EDIT:

Now that I think about it, I don't see the need for your Vector and Cube objects to exist on the heap at all. They're small enough objects that they shouldn't cause a stackoverflow (see what I did there ;) ). I would just get rid of the pointers completely and just keep them as objects on the stack.

\$\endgroup\$
6
\$\begingroup\$

All in all your overall class design looks good. Though there are a few points to improve:

Use an interface for general 3d figures

You should introduce an interface for 3d shapes/figures

struct Shape3d {
    virtual void draw(Renderer& r) = 0;
    virtual ~Shape3d() {}
};

class Cube : public Shape3d {
public:
    Cube(double x, double y, double z, double w);
    virtual void draw(Renderer& r);
    // ...
};

This would allow to handle multiple different 3d figures in more complex scenes easier.

Don't dereference from this as long its not ambiguous

You don't need to dereference the this pointer within the class scope like in

return sqrt(this->x * this->x + this->y * this->y + this->z * this->z);

and all the other statements where you do so.

You can just write

return sqrt(x * x + y * y + z * z);

for the example above.

Rather use an array for your 3d figures points

Instead of declaring 8 member variables to describe your Cube figures point, just use an array like

std::array<Vector,8> s;

Don't use using namespace std; in header files

Rather be explicit using the scope operator std::, doing so may prevent you from unexpected compilation problems.

Use references rather than pointers

With a forward declaration you can use references equally well as pointers, like e.g. in

void draw(Renderer* r);

=>

void draw(Renderer& r);

As long you don't expect to handle nullptr values, a reference is clearer and safer.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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