Merge pull request #515 from matthew16550/preserveAspectRatio

Fix preserveAspectRatio regression caused in #511

src/net/sourceforge/plantuml/PlainDiagram.java

@@ -46,17 +46,20 @@ import static net.sourceforge.plantuml.ugraphic.ImageBuilder.plainImageBuilder;
 // This class doesnt feel like a wonderful idea, just a stepping stone towards something
 public abstract class PlainDiagram extends AbstractPSystem {
 
+	public ImageBuilder createImageBuilder(FileFormatOption fileFormatOption) throws IOException {
+		final UDrawable drawable = getRootDrawable(fileFormatOption);
+
+		return plainImageBuilder(drawable, fileFormatOption)
+				.margin(getDefaultMargins())
+				.metadata(fileFormatOption.isWithMetadata() ? getMetadata() : null)
+				.seed(seed());
+	}
+
 	@Override
 	protected ImageData exportDiagramNow(OutputStream os, int index, FileFormatOption fileFormatOption)
 			throws IOException {
 
-		final UDrawable drawable = getRootDrawable(fileFormatOption);
+		final ImageBuilder builder = createImageBuilder(fileFormatOption);
-
-		final ImageBuilder builder = plainImageBuilder(drawable, fileFormatOption)
-				.margin(getDefaultMargins())
-				.metadata(fileFormatOption.isWithMetadata() ? getMetadata() : null)
-				.seed(seed());
-
 		return adjustImageBuilder(builder).write(os);
 	}
 

src/net/sourceforge/plantuml/SkinParam.java

@@ -89,6 +89,9 @@ import net.sourceforge.plantuml.ugraphic.color.NoSuchColorException;
 
 public class SkinParam implements ISkinParam {
 
+	// TODO not clear whether SkinParam or ImageBuilder is responsible for defaults
+	public static final String DEFAULT_PRESERVE_ASPECT_RATIO = "none";
+
 	// private String skin = "debug.skin";
 
 	private String skin = "plantuml.skin";
@@ -1056,7 +1059,7 @@ public class SkinParam implements ISkinParam {
 	public String getPreserveAspectRatio() {
 		final String value = getValue("preserveaspectratio");
 		if (value == null) {
-			return "none";
+			return DEFAULT_PRESERVE_ASPECT_RATIO;
 		}
 		return value;
 	}

src/net/sourceforge/plantuml/ugraphic/ImageBuilder.java

@@ -105,6 +105,8 @@ import net.sourceforge.plantuml.ugraphic.tikz.UGraphicTikz;
 import net.sourceforge.plantuml.ugraphic.txt.UGraphicTxt;
 import net.sourceforge.plantuml.ugraphic.visio.UGraphicVdx;
 
+import static net.sourceforge.plantuml.SkinParam.DEFAULT_PRESERVE_ASPECT_RATIO;
+
 public class ImageBuilder {
 
 	private Animation animation;
@@ -152,6 +154,7 @@ public class ImageBuilder {
 	private ImageBuilder(UDrawable drawable, FileFormatOption fileFormatOption) {
 		this.udrawable = drawable;
 		this.fileFormatOption = fileFormatOption;
+		this.preserveAspectRatio = calculatePreserveAspectRatio(fileFormatOption, null);
 
 		if (drawable instanceof TextBlockBackcolored) {
 			backcolor = ((TextBlockBackcolored) drawable).getBackcolor();
@@ -187,6 +190,10 @@ public class ImageBuilder {
 		return this;
 	}
 
+	public String getPreserveAspectRatio() {
+		return preserveAspectRatio;
+	}
+
 	public ImageBuilder randomPixel() {
 		this.randomPixel = true;
 		return this;
@@ -222,8 +229,7 @@ public class ImageBuilder {
 		lengthAdjust = skinParam.getlengthAdjust();
 		margin = calculateMargin(diagram);
 		metadata = fileFormatOption.isWithMetadata() ? diagram.getMetadata() : null;
-		preserveAspectRatio = (fileFormatOption.getPreserveAspectRatio() != null)
+		preserveAspectRatio = calculatePreserveAspectRatio(fileFormatOption, skinParam);
-				? fileFormatOption.getPreserveAspectRatio() : skinParam.getPreserveAspectRatio();
 		scale = diagram.getScale();
 		seed = diagram.seed();
 		svgCharSizeHack = skinParam;
@@ -516,6 +522,16 @@ public class ImageBuilder {
 		return diagram.getDefaultMargins();
 	}
 
+	private static String calculatePreserveAspectRatio(FileFormatOption fileFormatOption, ISkinParam skinParam) {
+		if (fileFormatOption.getPreserveAspectRatio() != null) {
+			return fileFormatOption.getPreserveAspectRatio();
+		} else if (skinParam != null) {
+			return skinParam.getPreserveAspectRatio();
+		} else {
+			return DEFAULT_PRESERVE_ASPECT_RATIO;
+		}
+	}
+
 	private ImageDataSimple createImageData(Dimension2D dim) {
 		return new ImageDataSimple(dim, status);
 	}

src/net/sourceforge/plantuml/wbs/WBSDiagram.java

@@ -57,6 +57,7 @@ import net.sourceforge.plantuml.graphic.TextBlock;
 import net.sourceforge.plantuml.mindmap.IdeaShape;
 import net.sourceforge.plantuml.style.NoStyleAvailableException;
 import net.sourceforge.plantuml.svek.TextBlockBackcolored;
+import net.sourceforge.plantuml.ugraphic.ImageBuilder;
 import net.sourceforge.plantuml.ugraphic.MinMax;
 import net.sourceforge.plantuml.ugraphic.UGraphic;
 import net.sourceforge.plantuml.ugraphic.color.HColor;
@@ -73,11 +74,17 @@ public class WBSDiagram extends UmlDiagram {
 		super(UmlDiagramType.WBS);
 	}
 
+	public ImageBuilder createImageBuilder(FileFormatOption fileFormatOption) {
+		// TODO index should not be -1 here but we currently dont use it anyway,
+		//      the real value we need is "index" in exportDiagramInternal()
+		return styledImageBuilder(this, getTextBlock(), -1, fileFormatOption);
+	}
+
 	@Override
 	protected ImageData exportDiagramInternal(OutputStream os, int index, FileFormatOption fileFormatOption)
 			throws IOException {
 
-		return styledImageBuilder(this, getTextBlock(), index, fileFormatOption)
+		return createImageBuilder(fileFormatOption)
 				.write(os);
 	}
 

test/net/sourceforge/plantuml/ugraphic/ImageBuilderTest.java

@@ -0,0 +1,57 @@
+package net.sourceforge.plantuml.ugraphic;
+
+import net.sourceforge.plantuml.FileFormatOption;
+import net.sourceforge.plantuml.PlainDiagram;
+import net.sourceforge.plantuml.creole.legacy.PSystemCreole;
+import net.sourceforge.plantuml.wbs.WBSDiagram;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import static net.sourceforge.plantuml.FileFormat.DEBUG;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class ImageBuilderTest {
+
+	@ParameterizedTest
+	@CsvSource(
+			value = {
+					// inFileFormatOption  Expected
+					"  NULL,               none",
+					"  foo,                foo",
+			},
+			nullValues = {"NULL"}
+	)
+	public void test_preserveAspectRatio_plainDiagram(String inFileFormatOption, String expected) throws Exception {
+		final PlainDiagram diagram = new PSystemCreole();
+		FileFormatOption fileFormatOption = new FileFormatOption(DEBUG);
+
+		if (inFileFormatOption != null) fileFormatOption = fileFormatOption.withPreserveAspectRatio(inFileFormatOption);
+
+		final ImageBuilder builder = diagram.createImageBuilder(fileFormatOption);
+
+		assertThat(builder.getPreserveAspectRatio()).isEqualTo(expected);
+	}
+
+	@ParameterizedTest
+	@CsvSource(
+			value = {
+					// inSkinParam  inFileFormatOption  Expected
+					"  NULL,        NULL,               none",
+					"  foo,         NULL,               foo",
+					"  NULL,        bar,                bar",
+					"  foo,         bar,                bar",
+			},
+			nullValues = {"NULL"}
+	)
+	public void test_preserveAspectRatio_styledDiagram(String inSkinParam, String inFileFormatOption, String expected) {
+		final WBSDiagram diagram = new WBSDiagram();
+		FileFormatOption fileFormatOption = new FileFormatOption(DEBUG);
+
+		if (inSkinParam != null) diagram.setParam("preserveAspectRatio", inSkinParam);
+		if (inFileFormatOption != null) fileFormatOption = fileFormatOption.withPreserveAspectRatio(inFileFormatOption);
+
+		final ImageBuilder builder = diagram.createImageBuilder(fileFormatOption);
+
+		assertThat(builder.getPreserveAspectRatio()).isEqualTo(expected);
+	}
+}