3
\$\begingroup\$

I am starting to work on a basic 3D renderer for fun, so right now I have made a triangle fill function to be used when I draw faces on models:

public void fillTriangle(Triangle2d t, Color color) {
    // triangle points are already sorted
    Line2d ab = t.getAB(), bc = t.getBC(), ac = t.getAC();
    Line2d leftEdge = ab.getEnd().getX() < ac.getEnd().getX() ? ab : ac;
    Line2d rightEdge = ab.getEnd().getX() > ac.getEnd().getX() ? ab : ac;
    Line2d bottomEdge = bc;

    // horizontal lines
    int startY = (int) t.getA().getY(), endY = (int) t.getC().getY();
    for (int y = startY; y <= endY; y++) {
        int startX, endX = (int) rightEdge.getX(y);
        // we are now below the middle (vertically) point, so to avoid going past the bottom edge,
        // we use it to get the startX
        if (y > leftEdge.getEnd().getY())
            startX = (int) bottomEdge.getX(y);
        else
            startX = (int) leftEdge.getX(y);

        for (int x = startX; x <= endX; x++)
            set(x, y, color);
    }

    set((int) t.getA().getX(), (int) t.getA().getY(), Color.RED);
    set((int) t.getB().getX(), (int) t.getB().getY(), Color.RED);
    set((int) t.getC().getX(), (int) t.getC().getY(), Color.RED);
}
  • set sets a pixel to a specific color
  • Triangle2d's points are already sorted vertically, so A is above B and B is above C (unless they have equal y-values)
  • Triangle2d#getAB returns a line going from the first point, A to the second point B
  • Line2d#getX returns the X-value for a given Y-value (and likewise for Line2d#getY)

The idea is that I draw horizontal lines going from the top to the bottom.

\$\endgroup\$
2
\$\begingroup\$

This is overall pretty reasonable code. It's very straightforward. Here are some things that could be improved:

Readabality

I find a few things hard to read. For example, putting not only multiple variable declarations on the same line, but assigning each one to a function call result is not fun to read. I would write each on its own line so it's more clear:

Line2d ab = t.getAB();
Line2d bc = t.getBC();
Line2d ac = t.getAC();

Same with the startY and endY variables:

int startY = (int) t.getA().getY();
int endY = (int) t.getC().getY();

Also, even though it's legal to omit the braces on single line if and for statements, it's a good idea to include them to avoid future bugs. I've seen plenty of code where someone added a line to a 1-line if statement without the braces and that line ended up always being executed. It ends up looking like this:

if (foobar)
    callFunction()
    callOtherFunction()

Because there are no braces, callOtherFunction() is always called regardless of the value of foobar.

Naming

Most of your function and variable naming is pretty good, but I would rename the set() function to setPixelColor() to make it more clear what you're setting. The names set() and get() are very overused in programming today, so it helps to be more specific.

Use Points in set() Function

Why separate out the x and y values of the point when calling set()? You should just make it accept a point parameter and do the getX(), getY() and type casting inside the function.

In fact, you do actually pass separate x and y values that are calculated inside the loop, so you might even want to overload it and create 2 versions, 1 that takes a point, and one that takes 2 coordinates, like this:

void setPixelColor(int x, int y, Color pixColor);

and

void setPixelColor(Point2d p, Color pixColor)
{
    setPixelColor((int)p.getX(), (int)p.getY(), pixColor);
}

Renderer Stuff

Most 3D renderers, like OpenGL, DirectX, Metal, etc. make a guarantee to never draw the same pixel twice for triangles that share an edge. I notice that your loops are including the start and end points of each row and column of pixels in the triangle. You might want to change the end condition to just < instead of <= to get this same sort of functionality.

Also, why are you coloring the corners of every triangle red? Was that just for debugging purposes?

\$\endgroup\$
  • \$\begingroup\$ Sorry forgot to say the red corners were for debugging. Thanks for the help! \$\endgroup\$ – MCMastery Aug 19 '17 at 18:10

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.