FOP-3192: Redo layout for multipage columns

10 files changed, 149 insertions, 71 deletions

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 443b02028..12707b30b 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
@@ -847,4 +847,8 @@ public class FOUserAgent {
     public boolean isSkipPagePositionOnlyAllowed() {
         return factory.isSkipPagePositionOnlyAllowed();
     }
+
+    public boolean isLegacySkipPagePositionOnly() {
+        return factory.isLegacySkipPagePositionOnly();
+    }
 }
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 3bece2002..f36593ed5 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
@@ -59,6 +59,7 @@ public class FopConfParser {
     private static final String TABLE_BORDER_OVERPAINT = "table-border-overpaint";
     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 final Log log = LogFactory.getLog(FopConfParser.class);
 
@@ -299,6 +300,14 @@ public class FopConfParser {
                 LogUtil.handleException(log, e, false);
             }
         }
+        if (cfg.getChild(LEGACY_SKIP_PAGE_POSITION_ONLY, false) != null) {
+            try {
+                fopFactoryBuilder.setLegacySkipPagePositionOnly(
+                        cfg.getChild(LEGACY_SKIP_PAGE_POSITION_ONLY).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 3b90614b4..7696c3997 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
@@ -244,6 +244,10 @@ public final class FopFactory implements ImageContext {
         return config.isSkipPagePositionOnlyAllowed();
     }
 
+    boolean isLegacySkipPagePositionOnly() {
+        return config.isLegacySkipPagePositionOnly();
+    }
+
     /**
      * 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 afbcb227e..aa90bd544 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
@@ -355,6 +355,11 @@ public final class FopFactoryBuilder {
         return this;
     }
 
+    public FopFactoryBuilder setLegacySkipPagePositionOnly(boolean b) {
+        fopFactoryConfigBuilder.setLegacySkipPagePositionOnly(b);
+        return this;
+    }
+
     public static class FopFactoryConfigImpl implements FopFactoryConfig {
 
         private final EnvironmentProfile enviro;
@@ -401,6 +406,8 @@ public final class FopFactoryBuilder {
 
         private boolean skipPagePositionOnlyAllowed = true;
 
+        private boolean legacySkipPagePositionOnly;
+
         private static final class ImageContextImpl implements ImageContext {
 
             private final FopFactoryConfig config;
@@ -529,6 +536,10 @@ public final class FopFactoryBuilder {
             return skipPagePositionOnlyAllowed;
         }
 
+        public boolean isLegacySkipPagePositionOnly() {
+            return legacySkipPagePositionOnly;
+        }
+
         public Map<String, String> getHyphenationPatternNames() {
             return hyphPatNames;
         }
@@ -580,6 +591,8 @@ public final class FopFactoryBuilder {
         void setSimpleLineBreaking(boolean b);
 
         void setSkipPagePositionOnlyAllowed(boolean b);
+
+        void setLegacySkipPagePositionOnly(boolean b);
     }
 
     private static final class CompletedFopFactoryConfigBuilder implements FopFactoryConfigBuilder {
@@ -675,6 +688,10 @@ public final class FopFactoryBuilder {
         public void setSkipPagePositionOnlyAllowed(boolean b) {
             throwIllegalStateException();
         }
+
+        public void setLegacySkipPagePositionOnly(boolean b) {
+            throwIllegalStateException();
+        }
     }
 
     private static final class ActiveFopFactoryConfigBuilder implements FopFactoryConfigBuilder {
@@ -771,6 +788,10 @@ public final class FopFactoryBuilder {
         public void setSkipPagePositionOnlyAllowed(boolean b) {
             config.skipPagePositionOnlyAllowed = b;
         }
+
+        public void setLegacySkipPagePositionOnly(boolean b) {
+            config.legacySkipPagePositionOnly = 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 55cf7143e..a74b0437c 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
@@ -169,6 +169,8 @@ public interface FopFactoryConfig {
 
     boolean isSkipPagePositionOnlyAllowed();
 
+    boolean isLegacySkipPagePositionOnly();
+
     /** @return the hyphenation pattern names */
     Map<String, String> getHyphenationPatternNames();
 
diff --git a/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractBreaker.java b/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractBreaker.java
index e86d9cf50..9a519276e 100644
--- a/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractBreaker.java
+++ b/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractBreaker.java
@@ -382,81 +382,86 @@ public abstract class AbstractBreaker {
         while (hasMoreContent()) {
             blockLists.clear();
 
-            //*** Phase 1: Get Knuth elements ***
-            nextSequenceStartsOn = getNextBlockList(childLC, nextSequenceStartsOn);
-            empty = empty && blockLists.size() == 0;
-
-            //*** Phases 2 and 3 ***
-            log.debug("PLM> blockLists.size() = " + blockLists.size());
-            for (blockListIndex = 0; blockListIndex < blockLists.size(); blockListIndex++) {
-                blockList = blockLists.get(blockListIndex);
-
-                //debug code start
-                if (log.isDebugEnabled()) {
-                    log.debug("  blockListIndex = " + blockListIndex);
-                    log.debug("  sequence starts on " + getBreakClassName(blockList.startOn));
-                }
-                observeElementList(blockList);
-                //debug code end
-
-                //*** Phase 2: Alignment and breaking ***
-                log.debug("PLM> start of algorithm (" + this.getClass().getName()
-                        + "), flow BPD =" + flowBPD);
-                PageBreakingAlgorithm alg = new PageBreakingAlgorithm(getTopLevelLM(),
-                         getPageProvider(), createLayoutListener(),
-                         alignment, alignmentLast, footnoteSeparatorLength,
-                         isPartOverflowRecoveryActivated(), autoHeight, isSinglePartFavored());
-
-                alg.setConstantLineWidth(flowBPD);
-                int optimalPageCount = alg.findBreakingPoints(blockList, 1, true,
-                        BreakingAlgorithm.ALL_BREAKS);
-                boolean ipdChangesOnNextPage = (alg.getIPDdifference() != 0);
-                boolean onLastPageAndIPDChanges = false;
-                if (!ipdChangesOnNextPage) {
-                    onLastPageAndIPDChanges = (lastPageHasIPDChange(optimalPageCount) && !thereIsANonRestartableLM(alg)
-                            && (shouldRedoLayout() || (wasLayoutRedone() && optimalPageCount > 1)));
-                }
-                if (shouldRedoLayoutWithoutPagePositionOnly(ipdChangesOnNextPage, optimalPageCount)) {
-                    return false;
-                }
-                if (alg.handlingFloat()) {
-                    nextSequenceStartsOn = handleFloatLayout(alg, optimalPageCount, blockList, childLC);
-                } else if (ipdChangesOnNextPage || onLastPageAndIPDChanges) {
-                    boolean visitedBefore = false;
-                    if (onLastPageAndIPDChanges) {
-                        visitedBefore = wasLayoutRedone();
-                        prepareToRedoLayout(alg, optimalPageCount, blockList, blockList);
+            try {
+                //*** Phase 1: Get Knuth elements ***
+                nextSequenceStartsOn = getNextBlockList(childLC, nextSequenceStartsOn);
+                empty = empty && blockLists.size() == 0;
+
+                //*** Phases 2 and 3 ***
+                log.debug("PLM> blockLists.size() = " + blockLists.size());
+                for (blockListIndex = 0; blockListIndex < blockLists.size(); blockListIndex++) {
+                    blockList = blockLists.get(blockListIndex);
+
+                    //debug code start
+                    if (log.isDebugEnabled()) {
+                        log.debug("  blockListIndex = " + blockListIndex);
+                        log.debug("  sequence starts on " + getBreakClassName(blockList.startOn));
                     }
-
-                    firstElementsForRestart = null;
-                    RestartAtLM restartAtLMClass = new RestartAtLM();
-                    LayoutManager restartAtLM = restartAtLMClass.getRestartAtLM(this, alg, ipdChangesOnNextPage,
-                            onLastPageAndIPDChanges, visitedBefore, blockList, 1);
-                    if (restartAtLMClass.invalidPosition) {
+                    observeElementList(blockList);
+                    //debug code end
+
+                    //*** Phase 2: Alignment and breaking ***
+                    log.debug("PLM> start of algorithm (" + this.getClass().getName()
+                            + "), flow BPD =" + flowBPD);
+                    PageBreakingAlgorithm alg = new PageBreakingAlgorithm(getTopLevelLM(),
+                             getPageProvider(), createLayoutListener(),
+                             alignment, alignmentLast, footnoteSeparatorLength,
+                             isPartOverflowRecoveryActivated(), autoHeight, isSinglePartFavored());
+
+                    alg.setConstantLineWidth(flowBPD);
+                    int optimalPageCount = alg.findBreakingPoints(blockList, 1, true,
+                            BreakingAlgorithm.ALL_BREAKS);
+                    boolean ipdChangesOnNextPage = (alg.getIPDdifference() != 0);
+                    boolean onLastPageAndIPDChanges = false;
+                    if (!ipdChangesOnNextPage) {
+                        onLastPageAndIPDChanges = (lastPageHasIPDChange(optimalPageCount)
+                                && !thereIsANonRestartableLM(alg)
+                                && (shouldRedoLayout() || (wasLayoutRedone() && optimalPageCount > 1)));
+                    }
+                    if (shouldRedoLayoutWithoutPagePositionOnly(ipdChangesOnNextPage, optimalPageCount, alg)) {
                         return false;
                     }
-                    if (restartAtLM == null || restartAtLM.getChildLMs().isEmpty()) {
+                    if (alg.handlingFloat()) {
+                        nextSequenceStartsOn = handleFloatLayout(alg, optimalPageCount, blockList, childLC);
+                    } else if (ipdChangesOnNextPage || onLastPageAndIPDChanges) {
+                        boolean visitedBefore = false;
+                        if (onLastPageAndIPDChanges) {
+                            visitedBefore = wasLayoutRedone();
+                            prepareToRedoLayout(alg, optimalPageCount, blockList, blockList);
+                        }
+
                         firstElementsForRestart = null;
-                        LayoutManager restartAtLM2 = new RestartAtLM().getRestartAtLM(this, alg, ipdChangesOnNextPage,
-                                onLastPageAndIPDChanges, visitedBefore, blockList, 0);
-                        if (restartAtLM2 != null) {
-                            restartAtLM = restartAtLM2;
+                        RestartAtLM restartAtLMClass = new RestartAtLM();
+                        LayoutManager restartAtLM = restartAtLMClass.getRestartAtLM(this, alg, ipdChangesOnNextPage,
+                                onLastPageAndIPDChanges, visitedBefore, blockList, 1);
+                        if (restartAtLMClass.invalidPosition) {
+                            return false;
                         }
-                    }
-                    if (ipdChangesOnNextPage) {
-                        addAreas(alg, optimalPageCount, blockList, blockList);
-                    }
-                    blockLists.clear();
-                    blockListIndex = -1;
-                    nextSequenceStartsOn = getNextBlockList(childLC, Constants.EN_COLUMN, positionAtBreak,
-                            restartAtLM, firstElementsForRestart);
-                } else {
-                    log.debug("PLM> optimalPageCount= " + optimalPageCount
-                            + " pageBreaks.size()= " + alg.getPageBreaks().size());
+                        if (restartAtLM == null || restartAtLM.getChildLMs().isEmpty()) {
+                            firstElementsForRestart = null;
+                            LayoutManager restartAtLM2 = new RestartAtLM().getRestartAtLM(this, alg,
+                                    ipdChangesOnNextPage, onLastPageAndIPDChanges, visitedBefore, blockList, 0);
+                            if (restartAtLM2 != null) {
+                                restartAtLM = restartAtLM2;
+                            }
+                        }
+                        if (ipdChangesOnNextPage) {
+                            addAreas(alg, optimalPageCount, blockList, blockList);
+                        }
+                        blockLists.clear();
+                        blockListIndex = -1;
+                        nextSequenceStartsOn = getNextBlockList(childLC, Constants.EN_COLUMN, positionAtBreak,
+                                restartAtLM, firstElementsForRestart);
+                    } else {
+                        log.debug("PLM> optimalPageCount= " + optimalPageCount
+                                + " pageBreaks.size()= " + alg.getPageBreaks().size());
 
-                    //*** Phase 3: Add areas ***
-                    doPhase3(alg, optimalPageCount, blockList, blockList);
+                        //*** Phase 3: Add areas ***
+                        doPhase3(alg, optimalPageCount, blockList, blockList);
+                    }
                 }
+            } catch (PagePositionOnlyException e) {
+                return false;
             }
         }
 
@@ -465,11 +470,26 @@ public abstract class AbstractBreaker {
         return true;
     }
 
-    private boolean shouldRedoLayoutWithoutPagePositionOnly(boolean ipdChangesOnNextPage, int optimalPageCount) {
+    static class PagePositionOnlyException extends RuntimeException {
+    }
+
+    private boolean shouldRedoLayoutWithoutPagePositionOnly(boolean ipdChangesOnNextPage, int optimalPageCount,
+                                                            PageBreakingAlgorithm alg) {
         if ((ipdChangesOnNextPage || hasMoreContent() || optimalPageCount > 1)
                 && pslm != null && pslm.getCurrentPage().isPagePositionOnly) {
+            if (getPageProvider().foUserAgent.isLegacySkipPagePositionOnly()) {
+                return true;
+            }
             RegionBody rb = (RegionBody)pslm.getCurrentPage().getSimplePageMaster().getRegion(Constants.FO_REGION_BODY);
-            return rb.getColumnCount() == 1;
+            if (rb.getColumnCount() == 1) {
+                return true;
+            }
+            int restartPoint = getPageProvider().getStartingPartIndexForLastPage(optimalPageCount);
+            if (restartPoint > 0) {
+                PageBreakPosition pbp = (PageBreakPosition) alg.getPageBreaks().get(restartPoint - 1);
+                int newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() + 1);
+                return newStartPos > 0;
+            }
         }
         return false;
     }
diff --git a/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractLayoutManager.java b/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractLayoutManager.java
index ff693ed67..616de6850 100644
--- a/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractLayoutManager.java
+++ b/fop-core/src/main/java/org/apache/fop/layoutmgr/AbstractLayoutManager.java
@@ -67,7 +67,7 @@ public abstract class AbstractLayoutManager extends AbstractBaseLayoutManager im
     private int lastGeneratedPosition = -1;
     private int smallestPosNumberChecked = Integer.MAX_VALUE;
 
-    private boolean preserveChildrenAtEndOfLayout;
+    private boolean preserveChildrenAtEndOfLayout = true;
 
     /**
      * Abstract layout manager.
diff --git a/fop-core/src/main/java/org/apache/fop/layoutmgr/PageBreaker.java b/fop-core/src/main/java/org/apache/fop/layoutmgr/PageBreaker.java
index 1e9a1e87b..d95bd3c48 100644
--- a/fop-core/src/main/java/org/apache/fop/layoutmgr/PageBreaker.java
+++ b/fop-core/src/main/java/org/apache/fop/layoutmgr/PageBreaker.java
@@ -632,6 +632,10 @@ public class PageBreaker extends AbstractBreaker {
                         log.trace("Moving to next flow");
                         pv.getCurrentSpan().moveToNextFlow();
                     } else {
+                        if (pslm.getCurrentPage().isPagePositionOnly
+                                && !pslm.fobj.getUserAgent().isLegacySkipPagePositionOnly()) {
+                            throw new PagePositionOnlyException();
+                        }
                         log.trace("Making new page");
                         pslm.makeNewPage(false, emptyContent);
                     }
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 27ff94f7c..17348a2e6 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
@@ -145,6 +145,10 @@ public final class MutableConfig implements FopFactoryConfig {
         return delegate.isSkipPagePositionOnlyAllowed();
     }
 
+    public boolean isLegacySkipPagePositionOnly() {
+        return delegate.isLegacySkipPagePositionOnly();
+    }
+
     public Map<String, String> 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 daf4b50dc..8b5aad6c2 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
@@ -126,6 +126,7 @@ public class TestAssistant {
         builder.setTableBorderOverpaint(isTableBorderOverpaint(testDoc));
         builder.setSimpleLineBreaking(isSimpleLineBreaking(testDoc));
         builder.setSkipPagePositionOnlyAllowed(isSkipPagePositionOnlyAllowed(testDoc));
+        builder.setLegacySkipPagePositionOnly(isLegacySkipPagePositionOnly(testDoc));
         return builder.build();
     }
 
@@ -179,6 +180,15 @@ public class TestAssistant {
         }
     }
 
+    private boolean isLegacySkipPagePositionOnly(Document testDoc) {
+        try {
+            String s = eval(testDoc, "/testcase/cfg/legacy-skip-page-position-only");
+            return "true".equalsIgnoreCase(s);
+        } catch (XPathExpressionException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     /**
      * Loads a test case into a DOM document.
      * @param testFile the test file