Can only do full cylinders - Why not remove parameter?

Found a bug? Post here.
Post Reply
ted
Posts: 57
Joined: Wed 25. Jul 2012, 22:53

Can only do full cylinders - Why not remove parameter?

Post by ted » Sun 5. Aug 2012, 08:54

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");

User avatar
gunn
Posts: 323
Joined: Thu 14. Dec 2006, 09:56
Location: TU Berlin
Contact:

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

Post by gunn » Mon 6. Aug 2012, 11:57

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.
jReality core developer

Post Reply