1
\$\begingroup\$

This method repositions vertices of an object in such a way that the center of the object is at the origin of the coordinate system. The task is the commented part. While my code does the job I feel that it is very basic. I am looking for advice on how to make it faster and more elegant, concise and efficient. Any comments are welcome.

import java.nio.FloatBuffer;

public class ObjModifier {
    public static void centerVertices(FloatBuffer buf) {
        // We often get 3D models which are not positioned at the origin of the coordinate system. Placing such models
        // on a 3D map is harder for users. One solution is to reposition such a model so that its center is at the
        // origin of the coordinate system. Your task is to implement the centering.
        //
        // FloatBuffer stores the vertices of a mesh in a continuous array of floats (see below)
        // [x0, y0, z0, x1, y1, z1, ..., xn, yn, zn]
        // This kind of layout is common in low-level 3D graphics APIs.
        // TODO: Implement your solution here

        float[] a = new float[buf.capacity()];
        buf.get(a);

        float[] sums = new float[3];
        float[] avgs = new float[3];

        for (int i = 0 ; i < a.length; i+=3) {
            sums[0] += a[i];
            sums[1] += a[i+1];
            sums[2] += a[i+2];           

        }

        for (int i = 0; i < avgs.length; i++) {
            avgs[i] = sums[i]/(a.length/3);
        }

        for (int i = 0 ; i < a.length; i+=3) {
            a[i] -= avgs[0];
            a[i+1] -= avgs[1];
            a[i+2] -= avgs[2];         

        }

        for (int i = 0; i < a.length; i++) {
            buf.put(i, a[i]);
        }
        //

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

I would create a separate method for calculating averages and use variables for them, so that the code is more legible.

I don't personally like the use of arrays such as sum[] and avg[] because looping over them makes it hard to tell what you are trying to access. If you do want to use them, I would save their indices (or offsets) in static final fields so that the accesses looks like this:

    sum[i + X_OFFSET] = ...;
    sum[i + Y_OFFSET] = ...;
    sum[i + Z_OFFSET] = ...;

I used the stream API for convenience - a loop would be fine too.

private static final int X_OFFSET = 0;
private static final int Y_OFFSET = 1;
private static final int Z_OFFSET = 2;

public static void centerVertices(FloatBuffer buf) {

    float[] objVertices = new float[buf.capacity()];
    buf.get(objVertices);

    float xAvg = (float) getAverage(objVertices, X_OFFSET);
    float yAvg = (float) getAverage(objVertices, Y_OFFSET);
    float zAvg = (float) getAverage(objVertices, Z_OFFSET);

    for (int i = 0; i < objVertices.length; i += 3) {
        objVertices[i + X_OFFSET] -= xAvg;
        objVertices[i + Y_OFFSET] -= yAvg;
        objVertices[i + Z_OFFSET] -= zAvg;
    }

    for (int i = 0; i < objVertices.length; i++) {
        buf.put(i, objVertices[i]);
    }
}

/**
 * Get the average of every third element in the array (with offset)
 */
private static double getAverage(float[] arr, int offset) {
    return IntStream //
            .range(0, arr.length) //
            .filter(i -> i % 3 == offset) //
            .mapToDouble(i -> arr[i]).average() //
            .getAsDouble();
}
```
\$\endgroup\$
2
\$\begingroup\$

Hello, you can store the value a.length in a variable and reuse it in all your loops.

int n = a.length;

You can rewrite the first loop of your code introducing a new index k for the array sums in this way:

for (int i = 0 ; i < n; i+=3) {
    for (int k = 0; k < 3; ++k) {
        sums[k] += a[i + k];
    }
}

Similarly, you can rewrite the second loop:

int d = n / 3;
for (int i = 0; i < 3; ++i) {
    avgs[i] = sums[i] / d;
}

The third one is similar to the first one:

for (int i = 0 ; i < n; i+=3) {
    for (int k = 0; k < 3; ++k) {
        a[i + k] -= avgs[k];
    }
}

In the forth one, you can substitute a.length with n.

\$\endgroup\$
  • \$\begingroup\$ thank you. is there a reason to use ++k instead of k++? \$\endgroup\$ – Jan Pisl Aug 11 at 18:19
  • 1
    \$\begingroup\$ @JanPisl: you can use pre-increment or post-increment operator in a loop and you will obtain the same result, but usually the preincrement opeator is preferred. If you read the thread at the stackoverflow.com/questions/4706199/… there is the full explanation of why it is preferred to use preincrement over postincrement operator and a lot of debates about it. \$\endgroup\$ – dariosicily Aug 12 at 7:56

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.