Page 1 of 1

Can only do full cylinders - Why not remove parameter?

Posted: Sun 5. Aug 2012, 08:54
by ted
If anything other than Math.PI is specified in the thetamax parameter, it throws
Exception in thread "main" java.lang.IllegalArgumentException: Can only do full cylinders
at de.jreality.geometry.Primitives.closedCylinder(Primitives.java:484)
at de.jreality.geometry.Primitives.closedCylinder(Primitives.java:479)
at ...
The code responsible in Primitives.Java is a check at the top of the function call:

Code: Select all

	public static SceneGraphComponent closedCylinder(int n,   double r, double R, double zmin, double zmax, double thetamax) {
		if (Math.abs(thetamax - 2*Math.PI) > 10E-4)
			throw new IllegalArgumentException("Can only do full cylinders");
I could be missing something, but it seems this should be deprecated in favor of an identical function without the extra thetamax parameter. If it is to be implemented in the future it could be provisionally deprecated until then like de.jreality.geometry.GeometryMergeFactory.setRespectVertices(boolean).

Here's a patch if you're interested:

Code: Select all

Index: src-core/de/jreality/geometry/Primitives.java
===================================================================
--- src-core/de/jreality/geometry/Primitives.java	(revision 5485)
+++ src-core/de/jreality/geometry/Primitives.java	(working copy)
@@ -475,10 +475,17 @@
 		ifs.setGeometryAttributes(CommonAttributes.RMAN_PROXY_COMMAND, "Cylinder "+r+" "+zmin+" "+zmax+" "+180.0/Math.PI * thetamax);
 		return ifs;
 	}
+	public static SceneGraphComponent closedFullCylinder(int n,   double r, double zmin, double zmax) {
+		return closedCylinder(n,r,r,zmin, zmax, 2*Math.PI);
+	}
+	public static SceneGraphComponent closedFullCylinder(int n,   double r, double R, double zmin, double zmax) {
+		return closedCylinder(n,r,R,zmin,zmax,2*Math.PI);
+	}
+	/** @deprecated thetamax not implemented yet: Use closedFullCylinder */
 	public static SceneGraphComponent closedCylinder(int n,   double r, double zmin, double zmax, double thetamax) {
 		return closedCylinder(n,r,r,zmin, zmax, thetamax);
 	}
-
+	/** @deprecated thetamax not implemented yet: Use closedFullCylinder */
 	public static SceneGraphComponent closedCylinder(int n,   double r, double R, double zmin, double zmax, double thetamax) {
 		if (Math.abs(thetamax - 2*Math.PI) > 10E-4)
 			throw new IllegalArgumentException("Can only do full cylinders");

Re: Can only do full cylinders - Why not remove parameter?

Posted: Mon 6. Aug 2012, 11:57
by gunn
The thetamax parameter is interpreted correctly for the Renderman backend (Primitives.java, line 475):

Code: Select all

		ifs.setGeometryAttributes(CommonAttributes.RMAN_PROXY_COMMAND, "Cylinder "+r+" "+zmin+" "+zmax+" "+180.0/Math.PI * thetamax);
but not by default for the other backends. Your suggestion is probably a good one, since most users aren't using the Renderman backend.