From 486a7f94e1a0cd081ccad3273ce89a1538ab298b Mon Sep 17 00:00:00 2001
From: Billy Olsen <billy.olsen@gmail.com>
Date: Sun, 12 Apr 2020 20:29:14 -0700
Subject: [PATCH] Fix TubeFinSet 2D rendering

The TubeFinSetShapes were applying the supplied transform too many
times, resulting in the the shapes being offset off the viewing area.
The offset transform is already applied to the component instance
that is provided to the TubeFinSetShapes methods, so applying the
transform resulted in the offset being moved to the right.

This change removes the transform being applied to the component
locations when retrieved and only applies a linear transformation
to the component locations when applying the base rotational
transform. Using the linear transform avoids applying the offset
in addition to the rest of the transform.

The problem existed in both the side view and the back view, however
the back view was not as obvious due to the nature of the view. The
common code between both functions applying the transform is
refactored into a common location to reduce the amount of code.
Additionally, documentation was added explaining the transform process
that is being applied to the component locations for the final shapes.

Fixes #605

Signed-off-by: Billy Olsen <billy.olsen@gmail.com>
---
 .../gui/rocketfigure/TubeFinSetShapes.java    | 130 +++++++++++-------
 1 file changed, 80 insertions(+), 50 deletions(-)

diff --git a/swing/src/net/sf/openrocket/gui/rocketfigure/TubeFinSetShapes.java b/swing/src/net/sf/openrocket/gui/rocketfigure/TubeFinSetShapes.java
index f3eb9e8d3..1b00fd5f4 100644
--- a/swing/src/net/sf/openrocket/gui/rocketfigure/TubeFinSetShapes.java
+++ b/swing/src/net/sf/openrocket/gui/rocketfigure/TubeFinSetShapes.java
@@ -10,72 +10,102 @@ import net.sf.openrocket.util.Coordinate;
 import net.sf.openrocket.util.Transformation;
 
 
+/**
+ * The TubeFinSetShapes is used for retrieving shapes of the TubeFins on a
+ * Rocket from multiple view points. The returned shapes will be translated
+ * and transformed to the correct locations for rendering the rocket in the
+ * 2D view space.
+ */
 public class TubeFinSetShapes extends RocketComponentShape {
 	
+	/**
+	 * Returns an array of RocketcomponentShapes that describe the shape of
+	 * the TubeFinSet when viewed from the side of the rocket. TubeFins will
+	 * appear as a Rectangle from the side view
+	 * 
+	 * @param component the TubeFinSet to get the shapes for
+	 * @param transformation the transformation to apply to the shapes
+	 * @return an array of RocketComponentShapes that are used to draw the
+	 *         TubeFinSet from the side.
+	 */
 	public static RocketComponentShape[] getShapesSide( final RocketComponent component, final Transformation transformation) {
 
-		TubeFinSet finset = (net.sf.openrocket.rocketcomponent.TubeFinSet)component;
+		TubeFinSet finSet = (TubeFinSet) component;
 
-		int fins = finset.getFinCount();
-		double length = finset.getLength();
-		double outerRadius = finset.getOuterRadius();
-		double bodyRadius = finset.getBodyRadius();
-		// old version - Oct, 19 2015
-		//Coordinate[] instanceOffsets = new Coordinate[]{ transformation.transform( componentAbsoluteLocation )};
-		//instanceOffsets = component.shiftCoordinates(instanceOffsets);
-		
-		// new version
-		Coordinate[] start = transformation.transform( component.getLocations());
+		final double outerRadius = finSet.getOuterRadius();
+		final double length = finSet.getLength();
+		Coordinate[] locations = transformLocations(finSet, transformation);
+		Transformation finRotation = finSet.getFinRotationTransformation();
 
-		Transformation baseRotation = finset.getBaseRotationTransformation();
-		Transformation finRotation = finset.getFinRotationTransformation();
-
-		// Translate & rotate the coordinates
-		for (int i=0; i<start.length; i++) {
-			start[i] = baseRotation.transform(transformation.transform(start[i].add(0,bodyRadius+outerRadius,0)));
+		Shape[] shapes = new Shape[finSet.getFinCount()];
+		for (int i=0; i < shapes.length; i++) {
+			shapes[i] = new Rectangle2D.Double(locations[0].x, (locations[0].y-outerRadius), length, 2*outerRadius);
+			locations = finRotation.transform(locations);
 		}
-
-		//start = baseRotation.transform(start);
 		
-		Shape[] s = new Shape[fins];
-		for (int i=0; i<fins; i++) {
-			s[i] = new Rectangle2D.Double(start[0].x,(start[0].y-outerRadius),length,2*outerRadius);
-			start = finRotation.transform(start);
-		}
-		return RocketComponentShape.toArray(s, component);
+		return RocketComponentShape.toArray(shapes, component);
 	}
-	
 
+	/**
+	 * Returns an array of RocketComponentShapes that describe the shape of
+	 * the TubeFinSet when viewed from the rear or the rocket. TubeFins will
+	 * appear as an Ellipse/Circle from the back view.
+	 * 
+	 * @param component the TubeFinSet to get the shapes for
+	 * @param transformation the transformation to apply to the shapes
+	 * @return an array of RocketComponentShapes that are used to draw the
+	 *         TubeFinSet from the back
+	 */
 	public static RocketComponentShape[] getShapesBack( final RocketComponent component, final Transformation transformation) {
 			
-		TubeFinSet finset = (net.sf.openrocket.rocketcomponent.TubeFinSet)component;
+		TubeFinSet finSet = (TubeFinSet) component;
 		
-		int fins = finset.getFinCount();
-		double outerradius = finset.getOuterRadius();
-		double bodyradius = finset.getBodyRadius();
-		
-		// old version - Oct, 19 2015
-		//Coordinate[] instanceOffsets = new Coordinate[]{ transformation.transform( componentAbsoluteLocation )};
-		//instanceOffsets = component.shiftCoordinates(instanceOffsets);
-		
-		// new version
-		Coordinate[] start = transformation.transform( component.getLocations());
+		final double outerRadius = finSet.getOuterRadius();
+		Coordinate[] locations = transformLocations(finSet, transformation);
+		Transformation finRotation = finSet.getFinRotationTransformation();
 
-		Transformation baseRotation = finset.getBaseRotationTransformation();
-		Transformation finRotation = finset.getFinRotationTransformation();
+		Shape[] shapes = new Shape[finSet.getFinCount()];
+		for (int i=0; i < shapes.length; i++) {
+			shapes[i] = new Ellipse2D.Double((locations[0].z - outerRadius), (locations[0].y - outerRadius), (2 * outerRadius), (2 * outerRadius));
+			locations = finRotation.transform(locations);
+		}
 		
-		// Translate & rotate the coordinates
-		for (int i=0; i<start.length; i++) {
-			start[i] = baseRotation.transform(transformation.transform(start[i].add(0,bodyradius+outerradius,0)));
-		}
-
-		Shape[] s = new Shape[fins];
-		for (int i=0; i < fins; i++) {
-			s[i] = new Ellipse2D.Double((start[0].z-outerradius),(start[0].y-outerradius),2*outerradius,2*outerradius);
-			start = finRotation.transform(start);
-		}
-		return RocketComponentShape.toArray(s, component);
+		return RocketComponentShape.toArray(shapes, component);
 	}
 	
+	/**
+	 * Translates and rotates the coordinates as follows:
+	 * 
+	 * 1. Ensure the coordinate accounts for the body and outer radius. This
+	 *    adjusts the Y value of the coordinate to place it in the correct
+	 *    position relative to the body tube.
+	 * 
+	 * 2. Perform a linear transformation of the coordinate using the supplied
+	 *    transform. Using the linear transform ensures the coordinate is
+	 *    rotated per the view, but avoids applying an offset translation
+	 *    since that is already applied in the locations retrieved from the
+	 *    TubeFinSet.
+	 * 
+	 * 3. Apply the base rotational transform described by the TubeFinSet
+	 *    component itself.
+	 *    
+	 * @param finSet the TubeFinSet to apply the transformation to
+	 * @param transformation the Transformation to apply to the TubeFinSet
+	 */
+	private static Coordinate[] transformLocations(final TubeFinSet finSet, final Transformation transformation) {
+		final double outerRadius = finSet.getOuterRadius();
+		final double bodyRadius = finSet.getBodyRadius();
+        Coordinate[] locations = finSet.getComponentLocations();
+		Transformation baseRotation = finSet.getBaseRotationTransformation();
+		
+		for (int i=0; i < locations.length; i++) {
+			Coordinate c = locations[i].add(0, (bodyRadius + outerRadius), 0);
+			c = transformation.linearTransform(c);
+			c = baseRotation.transform(c);
+			locations[i] = c;
+		}
+		
+		return locations;
+	}
 	
 }