Some cleanup:

  * extraction of the footnote-related loops in getNextKnuthElements() into private methods 
    to facilitate step-over when debugging (no additional breakpoints needed to skip those loops)
  * merging of the private doPhase3() methods, so theoretically, it becomes possible to do 
    column-balancing, even with a last page condition, and to eliminate a significant amount 
    of duplicate lines of code
  * other minor changes, like renaming stray Hungarians, renaming some variables to make the 
    code slightly more self-explanatory, removal of unused import, and whatnot...


git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@765272 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/fop/layoutmgr/PageBreaker.java

@@ -28,7 +28,6 @@ import org.apache.fop.area.Footnote;
 import org.apache.fop.area.PageViewport;
 import org.apache.fop.fo.Constants;
 import org.apache.fop.fo.FObj;
-import org.apache.fop.fo.pagination.PageSequence;
 import org.apache.fop.fo.pagination.Region;
 import org.apache.fop.fo.pagination.RegionBody;
 import org.apache.fop.fo.pagination.StaticContent;
@@ -136,16 +135,9 @@ public class PageBreaker extends AbstractBreaker {
         return super.getNextBlockList(childLC, nextSequenceStartsOn);
     }
 
-    /** {@inheritDoc} */
-    protected List getNextKnuthElements(LayoutContext context, int alignment) {
-        List contentList = null;
-
-        while (!childFLM.isFinished() && contentList == null) {
-            contentList = childFLM.getNextKnuthElements(context, alignment);
-        }
+    private boolean containsFootnotes(List contentList, LayoutContext context) {
 
-        // scan contentList, searching for footnotes
-        boolean bFootnotesPresent = false;
+        boolean containsFootnotes = false;
         if (contentList != null) {
             ListIterator contentListIterator = contentList.listIterator();
             while (contentListIterator.hasNext()) {
@@ -153,7 +145,7 @@ public class PageBreaker extends AbstractBreaker {
                 if (element instanceof KnuthBlockBox
                     && ((KnuthBlockBox) element).hasAnchors()) {
                     // element represents a line with footnote citations
-                    bFootnotesPresent = true;
+                    containsFootnotes = true;
                     LayoutContext footnoteContext = new LayoutContext(context);
                     footnoteContext.setStackLimitBP(context.getStackLimitBP());
                     footnoteContext.setRefIPD(pslm.getCurrentPV()
@@ -173,31 +165,46 @@ public class PageBreaker extends AbstractBreaker {
                 }
             }
         }
+        return containsFootnotes;
+    }
 
-        if (bFootnotesPresent) {
-            // handle the footnote separator
-            StaticContent footnoteSeparator;
-            footnoteSeparator = pslm.getPageSequence().getStaticContent("xsl-footnote-separator");
-            if (footnoteSeparator != null) {
-                // the footnote separator can contain page-dependent content such as
-                // page numbers or retrieve markers, so its areas cannot simply be
-                // obtained now and repeated in each page;
-                // we need to know in advance the separator bpd: the actual separator
-                // could be different from page to page, but its bpd would likely be
-                // always the same
-
-                // create a Block area that will contain the separator areas
-                separatorArea = new Block();
-                separatorArea.setIPD(pslm.getCurrentPV()
-                            .getRegionReference(Constants.FO_REGION_BODY).getIPD());
-                // create a StaticContentLM for the footnote separator
-                footnoteSeparatorLM = (StaticContentLayoutManager)
-                    pslm.getLayoutManagerMaker().makeStaticContentLayoutManager(
-                    pslm, footnoteSeparator, separatorArea);
-                footnoteSeparatorLM.doLayout();
+    private void handleFootnoteSeparator() {
+        StaticContent footnoteSeparator;
+        footnoteSeparator = pslm.getPageSequence().getStaticContent("xsl-footnote-separator");
+        if (footnoteSeparator != null) {
+            // the footnote separator can contain page-dependent content such as
+            // page numbers or retrieve markers, so its areas cannot simply be
+            // obtained now and repeated in each page;
+            // we need to know in advance the separator bpd: the actual separator
+            // could be different from page to page, but its bpd would likely be
+            // always the same
 
-                footnoteSeparatorLength = new MinOptMax(separatorArea.getBPD());
-            }
+            // create a Block area that will contain the separator areas
+            separatorArea = new Block();
+            separatorArea.setIPD(pslm.getCurrentPV()
+                        .getRegionReference(Constants.FO_REGION_BODY).getIPD());
+            // create a StaticContentLM for the footnote separator
+            footnoteSeparatorLM
+                    = pslm.getLayoutManagerMaker().makeStaticContentLayoutManager(
+                        pslm, footnoteSeparator, separatorArea);
+            footnoteSeparatorLM.doLayout();
+
+            footnoteSeparatorLength = new MinOptMax(separatorArea.getBPD());
+        }
+    }
+
+    /** {@inheritDoc} */
+    protected List getNextKnuthElements(LayoutContext context, int alignment) {
+        List contentList = null;
+
+        while (!childFLM.isFinished() && contentList == null) {
+            contentList = childFLM.getNextKnuthElements(context, alignment);
+        }
+
+        // scan contentList, searching for footnotes
+        if (containsFootnotes(contentList, context)) {
+            // handle the footnote separator
+            handleFootnoteSeparator();
         }
         return contentList;
     }
@@ -241,49 +248,61 @@ public class PageBreaker extends AbstractBreaker {
     }
 
     /**
-     * Performs phase 3 operation
-     *
-     * @param alg page breaking algorithm
-     * @param partCount part count
-     * @param originalList the block sequence original list
-     * @param effectiveList the block sequence effective list
+     * {@inheritDoc}
+     * This implementation checks whether to trigger column-balancing,
+     * or whether to take into account a 'last-page' condition.
      */
     protected void doPhase3(PageBreakingAlgorithm alg, int partCount,
             BlockSequence originalList, BlockSequence effectiveList) {
+
         if (needColumnBalancing) {
-            doPhase3WithColumnBalancing(alg, partCount, originalList, effectiveList);
-        } else {
-            if (!hasMoreContent() && pslm.getPageSequence().hasPagePositionLast()) {
-                //last part is reached and we have a "last page" condition
-                doPhase3WithLastPage(alg, partCount, originalList, effectiveList);
-            } else {
-                //Directly add areas after finding the breaks
-                addAreas(alg, partCount, originalList, effectiveList);
+            //column balancing for the last part
+            doPhase3(alg, partCount, originalList, effectiveList, false);
+            return;
+        }
+
+        boolean lastPageMasterDefined = pslm.getPageSequence().hasPagePositionLast();
+        if (!hasMoreContent()) {
+            //last part is reached
+            if (lastPageMasterDefined) {
+                //last-page condition
+                doPhase3(alg, partCount, originalList, effectiveList, true);
+                return;
             }
         }
+
+        //nothing special: just add the areas now
+        addAreas(alg, partCount, originalList, effectiveList);
     }
 
-    private void doPhase3WithLastPage(PageBreakingAlgorithm alg, int partCount,
-            BlockSequence originalList, BlockSequence effectiveList) {
-        int newStartPos;
+    /**
+     * Restart the algorithm at the break corresponding
+     * to the given partCount
+     * (currently only used to redo the part after the
+     *  last break in case of column-balancing
+     *  and/or a last page-master)
+     */
+    private void doPhase3(PageBreakingAlgorithm alg, int partCount,
+            BlockSequence originalList, BlockSequence effectiveList,
+            boolean isLastPart) {
+
+
+        int newStartPos = 0;
         int restartPoint = pageProvider.getStartingPartIndexForLastPage(partCount);
         if (restartPoint > 0) {
-            //Add definitive areas before last page
+            //Add definitive areas for the parts before the
+            //restarting point
             addAreas(alg, restartPoint, originalList, effectiveList);
             //Get page break from which we restart
             PageBreakPosition pbp = (PageBreakPosition)
                     alg.getPageBreaks().get(restartPoint - 1);
-            //Set starting position to the first element *after* the page-break
             newStartPos = pbp.getLeafPos() + 1;
             //Handle page break right here to avoid any side-effects
             if (newStartPos > 0) {
                 handleBreakTrait(Constants.EN_PAGE);
             }
-        } else {
-            newStartPos = 0;
         }
-        AbstractBreaker.log.debug("Last page handling now!!!");
-        AbstractBreaker.log.debug("===================================================");
+
         AbstractBreaker.log.debug("Restarting at " + restartPoint
                 + ", new start position: " + newStartPos);
 
@@ -292,91 +311,74 @@ public class PageBreaker extends AbstractBreaker {
         int currentPageNum = pslm.getCurrentPageNum();
         pageProvider.setStartOfNextElementList(currentPageNum,
                 pslm.getCurrentPV().getCurrentSpan().getCurrentFlowIndex());
-        pageProvider.setLastPageIndex(currentPageNum);
-
-        //Restart last page
-        PageBreakingAlgorithm algRestart = new PageBreakingAlgorithm(
-                getTopLevelLM(),
-                getPageProvider(), createLayoutListener(),
-                alg.getAlignment(), alg.getAlignmentLast(),
-                footnoteSeparatorLength,
-                isPartOverflowRecoveryActivated(), false, false);
-        //alg.setConstantLineWidth(flowBPD);
-        int iOptPageCount = algRestart.findBreakingPoints(effectiveList,
-                    newStartPos,
-                    1, true, BreakingAlgorithm.ALL_BREAKS);
-        AbstractBreaker.log.debug("restart: iOptPageCount= " + iOptPageCount
-                + " pageBreaks.size()= " + algRestart.getPageBreaks().size());
+
+        PageBreakingAlgorithm algRestart = null;
+        int optimalPageCount;
         //Make sure we only add the areas we haven't added already
         effectiveList.ignoreAtStart = newStartPos;
-        boolean replaceLastPage
-                = iOptPageCount <= pslm.getCurrentPV().getBodyRegion().getColumnCount();
-        if (replaceLastPage) {
-            //Replace last page
-            pslm.setCurrentPage(pageProvider.getPage(false, currentPageNum));
-            addAreas(algRestart, iOptPageCount, originalList, effectiveList);
-        } else {
-            addAreas(alg, restartPoint, partCount - restartPoint, originalList, effectiveList);
-            //Add blank last page
-            pageProvider.setLastPageIndex(currentPageNum + 1);
-            pslm.setCurrentPage(pslm.makeNewPage(true, true));
-        }
-        AbstractBreaker.log.debug("===================================================");
-    }
 
-    private void doPhase3WithColumnBalancing(PageBreakingAlgorithm alg, int partCount,
-            BlockSequence originalList, BlockSequence effectiveList) {
-        AbstractBreaker.log.debug("Column balancing now!!!");
-        AbstractBreaker.log.debug("===================================================");
-        int newStartPos;
-        int restartPoint = pageProvider.getStartingPartIndexForLastPage(partCount);
-        if (restartPoint > 0) {
-            //Add definitive areas
-            addAreas(alg, restartPoint, originalList, effectiveList);
-            //Get page break from which we restart
-            PageBreakPosition pbp = (PageBreakPosition)
-                    alg.getPageBreaks().get(restartPoint - 1);
-            newStartPos = pbp.getLeafPos();
-            //Handle page break right here to avoid any side-effects
-            if (newStartPos > 0) {
-                handleBreakTrait(Constants.EN_PAGE);
-            }
-        } else {
-            newStartPos = 0;
+        if (isLastPart) {
+            pageProvider.setLastPageIndex(currentPageNum);
         }
-        AbstractBreaker.log.debug("Restarting at " + restartPoint
-                + ", new start position: " + newStartPos);
 
-        pageBreakHandled = true;
-        //Update so the available BPD is reported correctly
-        pageProvider.setStartOfNextElementList(pslm.getCurrentPageNum(),
-                pslm.getCurrentPV().getCurrentSpan().getCurrentFlowIndex());
+        if (needColumnBalancing) {
+            AbstractBreaker.log.debug("Column balancing now!!!");
+            AbstractBreaker.log.debug("===================================================");
+
+            //Restart last page
+            algRestart = new BalancingColumnBreakingAlgorithm(
+                    getTopLevelLM(), getPageProvider(), createLayoutListener(),
+                    alignment, Constants.EN_START, footnoteSeparatorLength,
+                    isPartOverflowRecoveryActivated(),
+                    pslm.getCurrentPV().getBodyRegion().getColumnCount());
+            AbstractBreaker.log.debug("===================================================");
+        } else  {
+            //plain last page, no column balancing
+            AbstractBreaker.log.debug("Last page handling now!!!");
+            AbstractBreaker.log.debug("===================================================");
+            //Restart last page
+            algRestart = new PageBreakingAlgorithm(
+                    getTopLevelLM(), getPageProvider(), createLayoutListener(),
+                    alg.getAlignment(), alg.getAlignmentLast(),
+                    footnoteSeparatorLength,
+                    isPartOverflowRecoveryActivated(), false, false);
+            AbstractBreaker.log.debug("===================================================");
+        }
 
-        //Restart last page
-        PageBreakingAlgorithm algRestart = new BalancingColumnBreakingAlgorithm(
-                getTopLevelLM(),
-                getPageProvider(), createLayoutListener(),
-                alignment, Constants.EN_START, footnoteSeparatorLength,
-                isPartOverflowRecoveryActivated(),
-                pslm.getCurrentPV().getBodyRegion().getColumnCount());
-        //alg.setConstantLineWidth(flowBPD);
-        int iOptPageCount = algRestart.findBreakingPoints(effectiveList,
+        optimalPageCount = algRestart.findBreakingPoints(effectiveList,
                     newStartPos,
                     1, true, BreakingAlgorithm.ALL_BREAKS);
-        AbstractBreaker.log.debug("restart: iOptPageCount= " + iOptPageCount
+        AbstractBreaker.log.debug("restart: optimalPageCount= " + optimalPageCount
                 + " pageBreaks.size()= " + algRestart.getPageBreaks().size());
-        if (iOptPageCount > pslm.getCurrentPV().getBodyRegion().getColumnCount()) {
-            AbstractBreaker.log.warn(
-                    "Breaking algorithm produced more columns than are available.");
-            /* reenable when everything works
-            throw new IllegalStateException(
-                    "Breaking algorithm must not produce more columns than available.");
-            */
+        
+        boolean fitsOnePage
+                = optimalPageCount <= pslm.getCurrentPV().getBodyRegion().getColumnCount();
+
+        if (isLastPart) {
+            if (fitsOnePage) {
+                //Replace last page
+                pslm.setCurrentPage(pageProvider.getPage(false, currentPageNum));
+            } else {
+                //Last page-master cannot hold the content.
+                //Add areas now...
+                addAreas(alg, restartPoint, partCount - restartPoint, originalList, effectiveList);
+                //...and add a blank last page
+                pageProvider.setLastPageIndex(currentPageNum + 1);
+                pslm.setCurrentPage(pslm.makeNewPage(true, true));
+                return;
+            }
+        } else {
+            if (!fitsOnePage) {
+                AbstractBreaker.log.warn(
+                        "Breaking algorithm produced more columns than are available.");
+                /* reenable when everything works
+                throw new IllegalStateException(
+                        "Breaking algorithm must not produce more columns than available.");
+                */
+            }
         }
-        //Make sure we only add the areas we haven't added already
-        effectiveList.ignoreAtStart = newStartPos;
-        addAreas(algRestart, iOptPageCount, originalList, effectiveList);
-        AbstractBreaker.log.debug("===================================================");
+
+        addAreas(algRestart, optimalPageCount, originalList, effectiveList);
     }
 
     protected void startPart(BlockSequence list, int breakClass) {