From 3f17363807da21bcb39f640fb251b69b84004a3e Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Sun, 22 Mar 2020 20:38:55 -0700 Subject: [PATCH 1/5] Use FlightConfiguration.getLength() in RocketPanel Use the curConfig.getLength() for the length displayed in the RocketPanel rather than calculating it based off the bounds. The length method provided by the FlightConfiguration takes everything into account that is needed (active stages, etc) to calculate the length. Closes #452 Signed-off-by: Billy Olsen --- .../openrocket/gui/scalefigure/RocketPanel.java | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/swing/src/net/sf/openrocket/gui/scalefigure/RocketPanel.java b/swing/src/net/sf/openrocket/gui/scalefigure/RocketPanel.java index 33a014531..fee80d6c1 100644 --- a/swing/src/net/sf/openrocket/gui/scalefigure/RocketPanel.java +++ b/swing/src/net/sf/openrocket/gui/scalefigure/RocketPanel.java @@ -606,20 +606,8 @@ public class RocketPanel extends JPanel implements TreeSelectionListener, Change figure3d.setCG(cg); figure3d.setCP(cp); - // Length bound is assumed to be tight - double length = 0; - Collection bounds = curConfig.getBounds(); - if (!bounds.isEmpty()) { - double minX = Double.POSITIVE_INFINITY, maxX = Double.NEGATIVE_INFINITY; - for (Coordinate c : bounds) { - if (c.x < minX) - minX = c.x; - if (c.x > maxX) - maxX = c.x; - } - length = maxX - minX; - } - + double length = curConfig.getLength(); + double diameter = Double.NaN; for (RocketComponent c : curConfig.getCoreComponents()) { if (c instanceof SymmetricComponent) { From 33e912833f15125688170991fd605a548fb28b85 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Sun, 22 Mar 2020 20:46:13 -0700 Subject: [PATCH 2/5] Fix stage selection toggle display Fixes the stage selection buttons to only be active when the stage is active and properly reflect the displayed stage state. Signed-off-by: Billy Olsen --- .../src/net/sf/openrocket/gui/components/StageSelector.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/swing/src/net/sf/openrocket/gui/components/StageSelector.java b/swing/src/net/sf/openrocket/gui/components/StageSelector.java index 81accf8b8..f5972ad28 100644 --- a/swing/src/net/sf/openrocket/gui/components/StageSelector.java +++ b/swing/src/net/sf/openrocket/gui/components/StageSelector.java @@ -10,21 +10,17 @@ import javax.swing.JPanel; import javax.swing.JToggleButton; import net.miginfocom.swing.MigLayout; -import net.sf.openrocket.l10n.Translator; import net.sf.openrocket.rocketcomponent.AxialStage; import net.sf.openrocket.rocketcomponent.ComponentChangeEvent; import net.sf.openrocket.rocketcomponent.FlightConfiguration; import net.sf.openrocket.rocketcomponent.Rocket; import net.sf.openrocket.rocketcomponent.RocketComponent; -import net.sf.openrocket.startup.Application; import net.sf.openrocket.util.StateChangeListener; @SuppressWarnings("serial") public class StageSelector extends JPanel implements StateChangeListener { - private static final Translator trans = Application.getTranslator(); - private final Rocket rocket; private List buttons = new ArrayList(); @@ -41,7 +37,7 @@ public class StageSelector extends JPanel implements StateChangeListener { this.removeAll(); for(AxialStage stage : configuration.getRocket().getStageList()){ JToggleButton button = new JToggleButton(new StageAction(stage)); - button.setSelected(true); + button.setSelected(configuration.isStageActive(stage.getStageNumber())); this.add(button); buttons.add(button); } From 25475a92fbdccb790199a1c1d235adb11742b6e6 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Sun, 5 Apr 2020 21:01:50 -0700 Subject: [PATCH 3/5] Rework the FlightConfiguration.calculateBounds() for instances Use the information provided in the getActiveInstances() in order to calculate the length of the rocket. This change takes each component instance and uses the component bounds and the instance's transform in order to determine the instance's actual bounds. The length is then calculated as the difference between the min and max X values, using the BoundingBox. Note, this particular change special-cases a few of the components in order to get the right length. It is preferred to revisit each special case in subsequent patches in order to keep this patch set minimal for the time being. The length calculations are already stressed during unit tests, but the results are a bit more accurate and thus the unit tests are updated to reflect the new values. Fixes #452 Signed-off-by: Billy Olsen --- .../rocketcomponent/FlightConfiguration.java | 66 +++++++++++++++++-- .../FlightConfigurationTest.java | 4 +- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java b/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java index 803013df3..7a6257f46 100644 --- a/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java +++ b/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java @@ -1,6 +1,7 @@ package net.sf.openrocket.rocketcomponent; import java.util.ArrayDeque; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -551,16 +552,71 @@ public class FlightConfiguration implements FlightConfigurableParameter> entry : map.entrySet()) { + RocketComponent component = entry.getKey(); + List contexts = entry.getValue(); + + Collection coordinates = new ArrayList(); + /* FinSets are a bit different in how they store their bounds, + * so we'll use the fin points for calculations. + */ + if (component instanceof FinSet) { + coordinates.addAll(Arrays.asList(((FinSet) component).getFinPoints())); + } else { + coordinates.addAll(component.getComponentBounds()); + } + BoundingBox componentBox = new BoundingBox(); + List transformedCoords = new ArrayList(); + for (InstanceContext ctxt : contexts) { + /* + * If the instance is not active in the current context, then + * skip the bound calculations. This is mildly confusing since + * getActiveInstances() implies that it will only return the + * instances that are active, but it returns all instances and + * the context indicates if it is active or not. + */ + if (!ctxt.active) { + continue; + } + for (Coordinate c : coordinates) { + Coordinate tc = null; + /* These components do not need the transform performed in + * order to provide the proper coordinates for length calculation. + * The transformation will cause the values to be calculated + * incorrectly. This should be fixed in the appropriate places + * not handled as one-offs in here. + */ + if ((component instanceof AxialStage) || (component instanceof BodyTube) || + (component instanceof PodSet)) { + tc = c; + } else { + tc = ctxt.transform.transform(c); + } + componentBox.update(tc); + transformedCoords.add(tc); + } + } + + bounds.update(componentBox); + } + boundsModID = rocket.getModID(); cachedLength = bounds.span().x; + /* Special case for the scenario that all of the stages are removed and are + * inactive. Its possible that this shouldn't be allowed, but it is currently + * so we'll just adjust the length here. + */ + if (cachedLength == Double.POSITIVE_INFINITY || cachedLength == Double.NEGATIVE_INFINITY) { + cachedLength = 0; + } cachedBounds.update( bounds ); } diff --git a/core/test/net/sf/openrocket/rocketcomponent/FlightConfigurationTest.java b/core/test/net/sf/openrocket/rocketcomponent/FlightConfigurationTest.java index fe11df0dc..ebec46523 100644 --- a/core/test/net/sf/openrocket/rocketcomponent/FlightConfigurationTest.java +++ b/core/test/net/sf/openrocket/rocketcomponent/FlightConfigurationTest.java @@ -44,7 +44,7 @@ public class FlightConfigurationTest extends BaseTestCase { // preconditions assertThat("active stage count doesn't match", config.getActiveStageCount(), equalTo(2)); - final double expectedLength = 0.33; + final double expectedLength = 0.335; final double calculatedLength = config.getLength(); assertEquals("source config length doesn't match: ", expectedLength, calculatedLength, EPSILON); @@ -70,7 +70,7 @@ public class FlightConfigurationTest extends BaseTestCase { int expectedMotorCount = 2; int actualMotorCount = config1.getActiveMotors().size(); assertThat("active motor count doesn't match", actualMotorCount, equalTo(expectedMotorCount)); - double expectedLength = 0.33; + double expectedLength = 0.335; assertEquals("source config length doesn't match: ", expectedLength, config1.getLength(), EPSILON); double expectedReferenceLength = 0.024; assertEquals("source config reference length doesn't match: ", expectedReferenceLength, config1.getReferenceLength(), EPSILON); From 355bfb61c1fa7934a6c58f29a172bd872de8b6c9 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Sun, 19 Apr 2020 19:15:14 -0700 Subject: [PATCH 4/5] Use FinSet.getBoundingBox() when computing bounds Use the FinSet.getBoundingBox() when computing the bounds for the current configuration in FlightConfiguration.calculateBounds(). The FinSet already contains a BoundingBox method, however it returns a larger BoundingBox than necessary to encapsulate the component. Change the FinSet.getBoundingBox() to create a bounding box of the smallest size to ensure a tight bounds for the FinSet. This is done by taking each of the locations for the component and determining where the fin tip point is and updating the BoundingBox with this location. Signed-off-by: Billy Olsen --- .../sf/openrocket/rocketcomponent/FinSet.java | 29 ++++++++++++++++--- .../rocketcomponent/FlightConfiguration.java | 8 ++--- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/core/src/net/sf/openrocket/rocketcomponent/FinSet.java b/core/src/net/sf/openrocket/rocketcomponent/FinSet.java index 33b6e0e2e..3d77c09bc 100644 --- a/core/src/net/sf/openrocket/rocketcomponent/FinSet.java +++ b/core/src/net/sf/openrocket/rocketcomponent/FinSet.java @@ -714,11 +714,32 @@ public abstract class FinSet extends ExternalComponent implements RingInstanceab final double finLength = singleFinBounds.max.x; final double finHeight = singleFinBounds.max.y; - BoundingBox compBox = new BoundingBox().update(getComponentLocations()); + Coordinate[] locations = getInstanceLocations(); + double[] angles = getInstanceAngles(); + BoundingBox finSetBox = new BoundingBox(); - BoundingBox finSetBox = new BoundingBox( compBox.min.sub( 0, finHeight, finHeight ), - compBox.max.add( finLength, finHeight, finHeight )); - return finSetBox; + /* + * The fins themselves will be offset by the location itself to match + * the outside radius of a body tube. The bounds encapsulate the outer + * portion of all the tips of the fins. + * + * The height of each fin along the Y axis can be determined by: + * yHeight = cos(angle) * finHeight + * + * The height (depth?) of each fin along the Z axis can be determined by: + * zHeight = sin(angle) * finHeight + * + * The boundingBox should be that box which is the smallest box where + * a convex hull will contain all Coordinates. + */ + for (int i = 0; i < locations.length; i++) { + double y = Math.cos(angles[i]) * finHeight; + double z = Math.sin(angles[i]) * finHeight; + finSetBox.update(locations[i].add(0, y, z)); + finSetBox.update(locations[i].add(finLength, y, z)); + } + + return finSetBox; } /** diff --git a/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java b/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java index 7a6257f46..239e03d04 100644 --- a/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java +++ b/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java @@ -565,11 +565,11 @@ public class FlightConfiguration implements FlightConfigurableParameter contexts = entry.getValue(); Collection coordinates = new ArrayList(); - /* FinSets are a bit different in how they store their bounds, - * so we'll use the fin points for calculations. + /* FinSets already provide a bounding box, so let's use that. */ if (component instanceof FinSet) { - coordinates.addAll(Arrays.asList(((FinSet) component).getFinPoints())); + bounds.update(((FinSet) component).getBoundingBox()); + continue; } else { coordinates.addAll(component.getComponentBounds()); } @@ -614,7 +614,7 @@ public class FlightConfiguration implements FlightConfigurableParameter Date: Sun, 19 Apr 2020 19:15:54 -0700 Subject: [PATCH 5/5] Correctly calculate the maximum height for side view. When calculating the subject dimensions, use the height as the radius of the circle which intersects all four corners of the BoundingBox in the Y, Z plane for both the rear view and the side view. The rear view was already doing this, but it was non-obvious that the side view should also be using this to calculate the height. It becomes obvious if we think about a winged rocket, i.e. a rocket who's fins in the Z direction are larger than those in the Y direction. In this configuration, a height based off of the fins in along the Y-axis alone will result in a "Fit" scaling which encapuslates the Y-axis fins. However, rotating that rocket will move the larger wings (the Z-axis fins) into the view, but the height will not allow for the wings to draw properly on the screen. Thusly, using the radius from the circle intersecting the bounding box along the Y, Z plane will ensure that the widest point from the body will be able to fit within the side view, regardless of the rotation around the X axis. Signed-off-by: Billy Olsen --- .../src/net/sf/openrocket/gui/scalefigure/RocketFigure.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/swing/src/net/sf/openrocket/gui/scalefigure/RocketFigure.java b/swing/src/net/sf/openrocket/gui/scalefigure/RocketFigure.java index 5775390a1..188d70ad2 100644 --- a/swing/src/net/sf/openrocket/gui/scalefigure/RocketFigure.java +++ b/swing/src/net/sf/openrocket/gui/scalefigure/RocketFigure.java @@ -419,13 +419,14 @@ public class RocketFigure extends AbstractScaleFigure { protected void updateSubjectDimensions() { // calculate bounds, and store in class variables final BoundingBox bounds = rocket.getSelectedConfiguration().getBoundingBox(); + final double maxR = Math.max(Math.hypot(bounds.min.y, bounds.min.z), + Math.hypot(bounds.max.y, bounds.max.z)); switch (currentViewType) { case SideView: - subjectBounds_m = new Rectangle2D.Double(bounds.min.x, bounds.min.y, bounds.span().x, bounds.span().y); + subjectBounds_m = new Rectangle2D.Double(bounds.min.x, -maxR, bounds.span().x, 2 * maxR); break; case BackView: - final double maxR = Math.max(Math.hypot(bounds.min.y, bounds.min.z), Math.hypot(bounds.max.y, bounds.max.z)); subjectBounds_m = new Rectangle2D.Double(-maxR, -maxR, 2 * maxR, 2 * maxR); break; default: