From f8fbc6480c67b737200c687b86ebc531ecad9801 Mon Sep 17 00:00:00 2001 From: Simon Steiner Date: Tue, 24 Sep 2024 09:15:08 +0100 Subject: [PATCH] FOP-3208: Only restart line manager when there is a linebreak for blocklayout --- .../java/org/apache/fop/apps/FOUserAgent.java | 4 ++ .../org/apache/fop/apps/FopConfParser.java | 9 +++ .../java/org/apache/fop/apps/FopFactory.java | 4 ++ .../apache/fop/apps/FopFactoryBuilder.java | 21 ++++++ .../org/apache/fop/apps/FopFactoryConfig.java | 2 + .../org/apache/fop/layoutmgr/RestartAtLM.java | 65 +++++++++++++------ .../layoutmgr/inline/LineLayoutManager.java | 2 +- .../org/apache/fop/apps/MutableConfig.java | 4 ++ .../fop/intermediate/TestAssistant.java | 10 +++ .../flow_changing-ipd_last-page_19_legacy.xml | 63 ++++++++++++++++++ .../flow_changing-ipd_last-page_20.xml | 59 +++++++++++++++++ 11 files changed, 223 insertions(+), 20 deletions(-) create mode 100755 fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_19_legacy.xml create mode 100755 fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_20.xml diff --git a/fop-core/src/main/java/org/apache/fop/apps/FOUserAgent.java b/fop-core/src/main/java/org/apache/fop/apps/FOUserAgent.java index 12707b30b..149af5c90 100644 --- a/fop-core/src/main/java/org/apache/fop/apps/FOUserAgent.java +++ b/fop-core/src/main/java/org/apache/fop/apps/FOUserAgent.java @@ -851,4 +851,8 @@ public class FOUserAgent { public boolean isLegacySkipPagePositionOnly() { return factory.isLegacySkipPagePositionOnly(); } + + public boolean isLegacyLastPageChangeIPD() { + return factory.isLegacyLastPageChangeIPD(); + } } diff --git a/fop-core/src/main/java/org/apache/fop/apps/FopConfParser.java b/fop-core/src/main/java/org/apache/fop/apps/FopConfParser.java index f36593ed5..d3344ff77 100644 --- a/fop-core/src/main/java/org/apache/fop/apps/FopConfParser.java +++ b/fop-core/src/main/java/org/apache/fop/apps/FopConfParser.java @@ -60,6 +60,7 @@ public class FopConfParser { private static final String SIMPLE_LINE_BREAKING = "simple-line-breaking"; private static final String SKIP_PAGE_POSITION_ONLY_ALLOWED = "skip-page-position-only-allowed"; private static final String LEGACY_SKIP_PAGE_POSITION_ONLY = "legacy-skip-page-position-only"; + private static final String LEGACY_LAST_PAGE_CHANGE_IPD = "legacy-last-page-change-ipd"; private final Log log = LogFactory.getLog(FopConfParser.class); @@ -308,6 +309,14 @@ public class FopConfParser { LogUtil.handleException(log, e, false); } } + if (cfg.getChild(LEGACY_LAST_PAGE_CHANGE_IPD, false) != null) { + try { + fopFactoryBuilder.setLegacyLastPageChangeIPD( + cfg.getChild(LEGACY_LAST_PAGE_CHANGE_IPD).getValueAsBoolean()); + } catch (ConfigurationException e) { + LogUtil.handleException(log, e, false); + } + } // configure font manager new FontManagerConfigurator(cfg, baseURI, fopFactoryBuilder.getBaseURI(), resourceResolver) diff --git a/fop-core/src/main/java/org/apache/fop/apps/FopFactory.java b/fop-core/src/main/java/org/apache/fop/apps/FopFactory.java index 7696c3997..bdf789bf1 100644 --- a/fop-core/src/main/java/org/apache/fop/apps/FopFactory.java +++ b/fop-core/src/main/java/org/apache/fop/apps/FopFactory.java @@ -248,6 +248,10 @@ public final class FopFactory implements ImageContext { return config.isLegacySkipPagePositionOnly(); } + boolean isLegacyLastPageChangeIPD() { + return config.isLegacyLastPageChangeIPD(); + } + /** * Returns a new {@link Fop} instance. FOP will be configured with a default user agent * instance. Use this factory method if your output type requires an output stream. diff --git a/fop-core/src/main/java/org/apache/fop/apps/FopFactoryBuilder.java b/fop-core/src/main/java/org/apache/fop/apps/FopFactoryBuilder.java index aa90bd544..ebb176c03 100644 --- a/fop-core/src/main/java/org/apache/fop/apps/FopFactoryBuilder.java +++ b/fop-core/src/main/java/org/apache/fop/apps/FopFactoryBuilder.java @@ -360,6 +360,11 @@ public final class FopFactoryBuilder { return this; } + public FopFactoryBuilder setLegacyLastPageChangeIPD(boolean b) { + fopFactoryConfigBuilder.setLegacyLastPageChangeIPD(b); + return this; + } + public static class FopFactoryConfigImpl implements FopFactoryConfig { private final EnvironmentProfile enviro; @@ -408,6 +413,8 @@ public final class FopFactoryBuilder { private boolean legacySkipPagePositionOnly; + private boolean legacyLastPageChangeIPD; + private static final class ImageContextImpl implements ImageContext { private final FopFactoryConfig config; @@ -540,6 +547,10 @@ public final class FopFactoryBuilder { return legacySkipPagePositionOnly; } + public boolean isLegacyLastPageChangeIPD() { + return legacyLastPageChangeIPD; + } + public Map getHyphenationPatternNames() { return hyphPatNames; } @@ -593,6 +604,8 @@ public final class FopFactoryBuilder { void setSkipPagePositionOnlyAllowed(boolean b); void setLegacySkipPagePositionOnly(boolean b); + + void setLegacyLastPageChangeIPD(boolean b); } private static final class CompletedFopFactoryConfigBuilder implements FopFactoryConfigBuilder { @@ -692,6 +705,10 @@ public final class FopFactoryBuilder { public void setLegacySkipPagePositionOnly(boolean b) { throwIllegalStateException(); } + + public void setLegacyLastPageChangeIPD(boolean b) { + throwIllegalStateException(); + } } private static final class ActiveFopFactoryConfigBuilder implements FopFactoryConfigBuilder { @@ -792,6 +809,10 @@ public final class FopFactoryBuilder { public void setLegacySkipPagePositionOnly(boolean b) { config.legacySkipPagePositionOnly = b; } + + public void setLegacyLastPageChangeIPD(boolean b) { + config.legacyLastPageChangeIPD = b; + } } } diff --git a/fop-core/src/main/java/org/apache/fop/apps/FopFactoryConfig.java b/fop-core/src/main/java/org/apache/fop/apps/FopFactoryConfig.java index a74b0437c..55dbb8671 100644 --- a/fop-core/src/main/java/org/apache/fop/apps/FopFactoryConfig.java +++ b/fop-core/src/main/java/org/apache/fop/apps/FopFactoryConfig.java @@ -171,6 +171,8 @@ public interface FopFactoryConfig { boolean isLegacySkipPagePositionOnly(); + boolean isLegacyLastPageChangeIPD(); + /** @return the hyphenation pattern names */ Map getHyphenationPatternNames(); diff --git a/fop-core/src/main/java/org/apache/fop/layoutmgr/RestartAtLM.java b/fop-core/src/main/java/org/apache/fop/layoutmgr/RestartAtLM.java index 80b1487a4..4a0fcd885 100644 --- a/fop-core/src/main/java/org/apache/fop/layoutmgr/RestartAtLM.java +++ b/fop-core/src/main/java/org/apache/fop/layoutmgr/RestartAtLM.java @@ -23,25 +23,29 @@ import java.util.Iterator; import java.util.LinkedList; import org.apache.fop.events.EventBroadcaster; -import org.apache.fop.layoutmgr.table.TableContentPosition; +import org.apache.fop.layoutmgr.inline.LineLayoutManager; /** * Class to find the restart layoutmanager for changing IPD */ class RestartAtLM { protected boolean invalidPosition; + private Position lineBreakPosition; + private int positionIndex; protected LayoutManager getRestartAtLM(AbstractBreaker breaker, PageBreakingAlgorithm alg, boolean ipdChangesOnNextPage, boolean onLastPageAndIPDChanges, boolean visitedBefore, AbstractBreaker.BlockSequence blockList, int start) { + lineBreakPosition = null; BreakingAlgorithm.KnuthNode optimalBreak = ipdChangesOnNextPage ? alg.getBestNodeBeforeIPDChange() : alg .getBestNodeForLastPage(); if (onLastPageAndIPDChanges && visitedBefore && breaker.originalRestartAtLM == null) { optimalBreak = null; } - int positionIndex = findPositionIndex(breaker, optimalBreak, alg, start); - if (breaker.positionAtBreak.getLM() instanceof BlockLayoutManager) { - positionIndex = findPositionIndex(breaker, optimalBreak, alg, 0); + findPositionIndex(breaker, optimalBreak, alg, start); + if (!breaker.getPageProvider().foUserAgent.isLegacyLastPageChangeIPD() + && breaker.positionAtBreak.getLM() instanceof BlockLayoutManager) { + findPositionIndexForBlockLayout(breaker, optimalBreak, alg); } if (ipdChangesOnNextPage || (breaker.positionAtBreak != null && breaker.positionAtBreak.getIndex() > -1)) { breaker.firstElementsForRestart = Collections.EMPTY_LIST; @@ -116,23 +120,15 @@ class RestartAtLM { if (onLastPageAndIPDChanges && !visitedBefore && breaker.positionAtBreak.getPosition() != null) { restartAtLM = breaker.positionAtBreak.getPosition().getLM(); } - findLeafPosition(breaker); - return restartAtLM; - } - - private void findLeafPosition(AbstractBreaker breaker) { - while (breaker.positionAtBreak instanceof NonLeafPosition) { - Position pos = breaker.positionAtBreak.getPosition(); - if (pos instanceof TableContentPosition) { - break; - } - breaker.positionAtBreak = pos; + if (lineBreakPosition != null && restartAtLM instanceof BlockLayoutManager) { + breaker.positionAtBreak = lineBreakPosition; } + return restartAtLM; } - private int findPositionIndex(AbstractBreaker breaker, BreakingAlgorithm.KnuthNode optimalBreak, + private void findPositionIndex(AbstractBreaker breaker, BreakingAlgorithm.KnuthNode optimalBreak, PageBreakingAlgorithm alg, int start) { - int positionIndex = (optimalBreak != null) ? optimalBreak.position : start; + positionIndex = (optimalBreak != null) ? optimalBreak.position : start; for (int i = positionIndex; i < alg.par.size(); i++) { KnuthElement elementAtBreak = alg.getElement(i); if (elementAtBreak.getPosition() == null) { @@ -142,9 +138,40 @@ class RestartAtLM { /* Retrieve the original position wrapped into this space position */ breaker.positionAtBreak = breaker.positionAtBreak.getPosition(); if (breaker.positionAtBreak != null) { - return i; + this.positionIndex = i; + return; + } + } + } + + private void findPositionIndexForBlockLayout(AbstractBreaker breaker, BreakingAlgorithm.KnuthNode optimalBreak, + PageBreakingAlgorithm alg) { + int positionIndex = (optimalBreak != null) ? optimalBreak.position : 0; + for (int i = positionIndex; i < alg.par.size(); i++) { + KnuthElement elementAtBreak = alg.getElement(i); + if (elementAtBreak.getPosition() == null) { + elementAtBreak = alg.getElement(0); + } + Position positionAtBreak = elementAtBreak.getPosition(); + /* Retrieve the original position wrapped into this space position */ + positionAtBreak = positionAtBreak.getPosition(); + if (positionAtBreak != null) { + findLineBreakPosition(positionAtBreak); + if (lineBreakPosition != null) { + breaker.positionAtBreak = positionAtBreak; + this.positionIndex = i; + } + break; + } + } + } + + private void findLineBreakPosition(Position positionAtBreak) { + while (positionAtBreak instanceof NonLeafPosition) { + positionAtBreak = positionAtBreak.getPosition(); + if (positionAtBreak instanceof LineLayoutManager.LineBreakPosition) { + lineBreakPosition = positionAtBreak; } } - return positionIndex; } } diff --git a/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java b/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java index 2fff6c494..25b68842c 100644 --- a/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java +++ b/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java @@ -108,7 +108,7 @@ public class LineLayoutManager extends InlineStackingLayoutManager * Each value holds the start and end indexes into a List of * inline break positions. */ - static class LineBreakPosition extends LeafPosition { + public static class LineBreakPosition extends LeafPosition { private final int parIndex; // index of the Paragraph this Position refers to private final int startIndex; //index of the first element this Position refers to private final int availableShrink; diff --git a/fop-core/src/test/java/org/apache/fop/apps/MutableConfig.java b/fop-core/src/test/java/org/apache/fop/apps/MutableConfig.java index 17348a2e6..3fcaa4fbc 100644 --- a/fop-core/src/test/java/org/apache/fop/apps/MutableConfig.java +++ b/fop-core/src/test/java/org/apache/fop/apps/MutableConfig.java @@ -149,6 +149,10 @@ public final class MutableConfig implements FopFactoryConfig { return delegate.isLegacySkipPagePositionOnly(); } + public boolean isLegacyLastPageChangeIPD() { + return delegate.isLegacyLastPageChangeIPD(); + } + public Map getHyphenationPatternNames() { return delegate.getHyphenationPatternNames(); } diff --git a/fop-core/src/test/java/org/apache/fop/intermediate/TestAssistant.java b/fop-core/src/test/java/org/apache/fop/intermediate/TestAssistant.java index 8b5aad6c2..f87905772 100644 --- a/fop-core/src/test/java/org/apache/fop/intermediate/TestAssistant.java +++ b/fop-core/src/test/java/org/apache/fop/intermediate/TestAssistant.java @@ -127,6 +127,7 @@ public class TestAssistant { builder.setSimpleLineBreaking(isSimpleLineBreaking(testDoc)); builder.setSkipPagePositionOnlyAllowed(isSkipPagePositionOnlyAllowed(testDoc)); builder.setLegacySkipPagePositionOnly(isLegacySkipPagePositionOnly(testDoc)); + builder.setLegacyLastPageChangeIPD(isLegacyLastPageChangeIPD(testDoc)); return builder.build(); } @@ -189,6 +190,15 @@ public class TestAssistant { } } + private boolean isLegacyLastPageChangeIPD(Document testDoc) { + try { + String s = eval(testDoc, "/testcase/cfg/legacy-last-page-change-ipd"); + return "true".equalsIgnoreCase(s); + } catch (XPathExpressionException e) { + throw new RuntimeException(e); + } + } + /** * Loads a test case into a DOM document. * @param testFile the test file diff --git a/fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_19_legacy.xml b/fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_19_legacy.xml new file mode 100755 index 000000000..35d8a7eda --- /dev/null +++ b/fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_19_legacy.xml @@ -0,0 +1,63 @@ + + + + + +

+ This test checks that the definition of a special page-master for the last page with a + different width that the previous "rest" page causes FOP to redo the line breaking layout. + Legacy option is enabled so wrong line break is used +

+ + + true + + + + + + + + + + + + + + + + + + + + + + test + test2 + test3 + + + + + + + + + + + diff --git a/fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_20.xml b/fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_20.xml new file mode 100755 index 000000000..d28c56da6 --- /dev/null +++ b/fop/test/layoutengine/standard-testcases/flow_changing-ipd_last-page_20.xml @@ -0,0 +1,59 @@ + + + + + +

+ This test checks that the definition of a special page-master for the last page with a + different width that the previous "rest" page causes FOP to redo the line breaking layout. +

+ + + + + + + + + + + + + + + + + + + + + + + + + + NOTE + + + + + + + + + -- 2.39.5