From 5c58b612782f13028a743617a654edec6688e5c3 Mon Sep 17 00:00:00 2001 From: matthew16550 Date: Fri, 2 Apr 2021 17:09:04 +1100 Subject: [PATCH] Fix preserveAspectRatio regression caused in #511 - ensure that "plain" diagrams use a default value of "none". --- .../sourceforge/plantuml/PlainDiagram.java | 17 +++--- src/net/sourceforge/plantuml/SkinParam.java | 5 +- .../plantuml/ugraphic/ImageBuilder.java | 20 ++++++- .../sourceforge/plantuml/wbs/WBSDiagram.java | 9 ++- .../plantuml/ugraphic/ImageBuilderTest.java | 57 +++++++++++++++++++ 5 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 test/net/sourceforge/plantuml/ugraphic/ImageBuilderTest.java diff --git a/src/net/sourceforge/plantuml/PlainDiagram.java b/src/net/sourceforge/plantuml/PlainDiagram.java index 4c28ca5d9..95d6da0a6 100644 --- a/src/net/sourceforge/plantuml/PlainDiagram.java +++ b/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 = plainImageBuilder(drawable, fileFormatOption) - .margin(getDefaultMargins()) - .metadata(fileFormatOption.isWithMetadata() ? getMetadata() : null) - .seed(seed()); - + final ImageBuilder builder = createImageBuilder(fileFormatOption); return adjustImageBuilder(builder).write(os); } diff --git a/src/net/sourceforge/plantuml/SkinParam.java b/src/net/sourceforge/plantuml/SkinParam.java index 7421f15ad..5b498a705 100644 --- a/src/net/sourceforge/plantuml/SkinParam.java +++ b/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; } diff --git a/src/net/sourceforge/plantuml/ugraphic/ImageBuilder.java b/src/net/sourceforge/plantuml/ugraphic/ImageBuilder.java index bad920c32..088b75f02 100644 --- a/src/net/sourceforge/plantuml/ugraphic/ImageBuilder.java +++ b/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) - ? fileFormatOption.getPreserveAspectRatio() : skinParam.getPreserveAspectRatio(); + preserveAspectRatio = calculatePreserveAspectRatio(fileFormatOption, skinParam); 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); } diff --git a/src/net/sourceforge/plantuml/wbs/WBSDiagram.java b/src/net/sourceforge/plantuml/wbs/WBSDiagram.java index 199938c73..fae88af93 100644 --- a/src/net/sourceforge/plantuml/wbs/WBSDiagram.java +++ b/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); } diff --git a/test/net/sourceforge/plantuml/ugraphic/ImageBuilderTest.java b/test/net/sourceforge/plantuml/ugraphic/ImageBuilderTest.java new file mode 100644 index 000000000..8b9377dc6 --- /dev/null +++ b/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); + } +}