diff --git a/core/src/net/sf/openrocket/rocketcomponent/BoxBounded.java b/core/src/net/sf/openrocket/rocketcomponent/BoxBounded.java new file mode 100644 index 000000000..700d2c9d4 --- /dev/null +++ b/core/src/net/sf/openrocket/rocketcomponent/BoxBounded.java @@ -0,0 +1,15 @@ +package net.sf.openrocket.rocketcomponent; + +import net.sf.openrocket.util.BoundingBox; + +public interface BoxBounded { + + /** + * Get a bounding box for a single instance of this component, from its own reference point. + * This is expected to be compbined with a InstanceContext for bounds in the global / rocket frame. + * + * @return BoundingBox from the instance's reference point. + */ + BoundingBox getInstanceBoundingBox(); + +} diff --git a/core/src/net/sf/openrocket/rocketcomponent/FinSet.java b/core/src/net/sf/openrocket/rocketcomponent/FinSet.java index e276782de..220ae5518 100644 --- a/core/src/net/sf/openrocket/rocketcomponent/FinSet.java +++ b/core/src/net/sf/openrocket/rocketcomponent/FinSet.java @@ -25,12 +25,8 @@ import net.sf.openrocket.util.Coordinate; import net.sf.openrocket.util.MathUtil; import net.sf.openrocket.util.Transformation; -public abstract class FinSet extends ExternalComponent implements RingInstanceable, AxialPositionable { +public abstract class FinSet extends ExternalComponent implements AxialPositionable, BoxBounded, RingInstanceable { private static final Translator trans = Application.getTranslator(); - - @SuppressWarnings("unused") - private static final Logger log = LoggerFactory.getLogger(FinSet.class); - /** * Maximum allowed cant of fins. @@ -735,13 +731,14 @@ public abstract class FinSet extends ExternalComponent implements RingInstanceab } - public BoundingBox getBoundingBox() { + public BoundingBox getInstanceBoundingBox(){ BoundingBox singleFinBounds= new BoundingBox().update(getFinPoints()); final double finLength = singleFinBounds.max.x; final double finHeight = singleFinBounds.max.y; Coordinate[] locations = getInstanceLocations(); double[] angles = getInstanceAngles(); + BoundingBox finSetBox = new BoundingBox(); /* diff --git a/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java b/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java index bd9298e0d..0cff35040 100644 --- a/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java +++ b/core/src/net/sf/openrocket/rocketcomponent/FlightConfiguration.java @@ -550,59 +550,77 @@ public class FlightConfiguration implements FlightConfigurableParameter> entry : map.entrySet()) { RocketComponent component = entry.getKey(); + BoundingBox componentBounds = new BoundingBox(); List contexts = entry.getValue(); - - Collection coordinates = new ArrayList(); + /* FinSets already provide a bounding box, so let's use that. */ - if (component instanceof FinSet) { - bounds.update(((FinSet) component).getBoundingBox()); - continue; - } 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 BoxBounded) { + for (InstanceContext context : 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 ((component instanceof AxialStage) || (component instanceof BodyTube) || - (component instanceof PodSet)) { - tc = c; - } else { - tc = ctxt.transform.transform(c); + if (!context.active) { + continue; + } + + BoundingBox instanceBounds = ((BoxBounded) component).getInstanceBoundingBox(); + if(instanceBounds.isEmpty()) { + // this component is probably non-physical (like an assembly) or has invalid bounds. Skip. + // i.e. 'break' out of the per-instance-context loop, and let the per-component loop continue + break; + } + componentBounds = instanceBounds.transform(context.transform); + } + }else if ((component instanceof AxialStage) || (component instanceof BodyTube) || (component instanceof PodSet)) { + // Legacy Case #1: + // 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. + componentBounds.update(component.getComponentBounds()); + } else { + // Legacy Case #2: + // These components do not implement + Collection instanceCoordinates = component.getComponentBounds(); + + for (InstanceContext context : 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 (!context.active) { + continue; + } + + Collection transformedCoords = new ArrayList<>(instanceCoordinates); + // mutating. Transforms coordinates in place. + context.transform.transform(instanceCoordinates); + + for (Coordinate tc : transformedCoords) { + componentBounds.update(tc); } - componentBox.update(tc); - transformedCoords.add(tc); } } - - bounds.update(componentBox); + + rocketBounds.update(componentBounds); } boundsModID = rocket.getModID(); - cachedLength = bounds.span().x; + cachedLength = rocketBounds.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. @@ -610,7 +628,7 @@ public class FlightConfiguration implements FlightConfigurableParameter max.x) || + (min.y > max.y) || + (min.z > max.z)){ + return true; + }else{ + return false; + } + } + + /** + * return a copy of this bounding box, transformed by the given transformation matrix + * @return + */ + public BoundingBox transform(Transformation transform){ + return new BoundingBox(transform.transform(this.min), transform.transform(this.max)); + } + private void update_x_min( final double xVal) { if( min.x > xVal) min = min.setX( xVal ); @@ -104,6 +121,9 @@ public class BoundingBox { } public BoundingBox update( BoundingBox other ) { + if(other.isEmpty()){ + return this; + } update_x_min(other.min.x); update_y_min(other.min.y); update_z_min(other.min.y); @@ -121,7 +141,7 @@ public class BoundingBox { } public Collection toCollection(){ - Collection toReturn = new ArrayList(); + Collection toReturn = new ArrayList<>(); toReturn.add( this.max); toReturn.add( this.min); return toReturn; diff --git a/swing/src/net/sf/openrocket/gui/figure3d/geometry/FinRenderer.java b/swing/src/net/sf/openrocket/gui/figure3d/geometry/FinRenderer.java index 3dca72e65..ddd3acf66 100644 --- a/swing/src/net/sf/openrocket/gui/figure3d/geometry/FinRenderer.java +++ b/swing/src/net/sf/openrocket/gui/figure3d/geometry/FinRenderer.java @@ -19,7 +19,7 @@ public class FinRenderer { public void renderFinSet(final GL2 gl, FinSet finSet ) { - BoundingBox bounds = finSet.getBoundingBox(); + BoundingBox bounds = finSet.getInstanceBoundingBox(); gl.glMatrixMode(GL.GL_TEXTURE); gl.glPushMatrix(); gl.glScaled(1 / (bounds.max.x - bounds.min.x), 1 / (bounds.max.y - bounds.min.y), 0);