Unneeded if-else statement in Torus

archive contribution-depot-jme2
1

So while I was poking around at the documentation holes in jME, I stumbled across this:



for (i = 0; i < radialSamples; i++, index += 6) {
                if (true) {
                    getIndexBuffer().put(i0++);
                    getIndexBuffer().put(i2);
                    getIndexBuffer().put(i1);
                    getIndexBuffer().put(i1++);
                    getIndexBuffer().put(i2++);
                    getIndexBuffer().put(i3++);
                } else {
                    getIndexBuffer().put(i0++);
                    getIndexBuffer().put(i1);
                    getIndexBuffer().put(i2);
                    getIndexBuffer().put(i1++);
                    getIndexBuffer().put(i3++);
                    getIndexBuffer().put(i2++);
                }
            }



you'll notice that the last two put's in the if and else are swapped, but the if-else is never going to be called anyway then the conditional is pretty useless.

Here's the fix, plus a description of what a Torus is for those not so familiar with 3D primitives

Index: src/com/jme/scene/shape/Torus.java
===================================================================
--- src/com/jme/scene/shape/Torus.java   (revision 4630)
+++ src/com/jme/scene/shape/Torus.java   (working copy)
@@ -45,7 +45,8 @@
 import com.jme.util.geom.BufferUtils;
 
 /**
- * An ordinary (single holed) torus.
+ * An ordinary (single holed) torus.  Similar
+ * in shape to a donut.
  * <p>
  * The center is by default the origin.
  *
@@ -197,21 +198,13 @@
             int i2 = connectionStart;
             int i3 = i2 + 1;
             for (i = 0; i < radialSamples; i++, index += 6) {
-                if (true) {
-                    getIndexBuffer().put(i0++);
-                    getIndexBuffer().put(i2);
-                    getIndexBuffer().put(i1);
-                    getIndexBuffer().put(i1++);
-                    getIndexBuffer().put(i2++);
-                    getIndexBuffer().put(i3++);
-                } else {
-                    getIndexBuffer().put(i0++);
-                    getIndexBuffer().put(i1);
-                    getIndexBuffer().put(i2);
-                    getIndexBuffer().put(i1++);
-                    getIndexBuffer().put(i3++);
-                    getIndexBuffer().put(i2++);
-                }
+               getIndexBuffer().put(i0++);
+               getIndexBuffer().put(i2);
+               getIndexBuffer().put(i1);
+               getIndexBuffer().put(i1++);
+               getIndexBuffer().put(i2++);
+               getIndexBuffer().put(i3++);
+
             }
         }
     }

2

I approve. These statements are probably to switch from CW culling from CCW but is quite useless since most games seem to use CCW culling.

3

Similar stipulations seemed to be in most of the primitive classes…  It’s a shame too… it took me a few minutes to read through the lines and really wrap my head around what was going on, someone put some brain crunch into the logic :’(

4

fwiw, a lot of those were there to allow for a "flip inside out" mode.  (iow, the primitive could be meant to be viewed from the inside or from the outside.)  At the time, it was decided not to enable that, but the logic stayed there in case that was changed in the future.

5
renanse said:

fwiw, a lot of those were there to allow for a "flip inside out" mode.  (iow, the primitive could be meant to be viewed from the inside or from the outside.)  At the time, it was decided not to enable that, but the logic stayed there in case that was changed in the future.


ah ok, thanks..  I could see this being useful for sphere and dome... I'd elect to keep it in then.. does anyone have a use for a feature like this that it should be implemented?  It looks to be ready to go for most, if not all, of the shapes
6

The same behaviour can be achieved by using CullStates.setPolygonWind() right?

So maybe those additional logic in the primitive classes is really not needed.