From aa09fc131c1cc8278486293cef5b3fc72f6ebac2 Mon Sep 17 00:00:00 2001 From: Vincent Hennebert Date: Mon, 17 Mar 2014 08:55:22 +0000 Subject: [PATCH] Handle the case where dynamic content is in the middle of a page, selecting the first variant Patch by Seifeddine Dridi, applied with some modifications git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/branches/Temp_WhitespaceManagement@1578270 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/fop/fo/flow/MultiCase.java | 28 +--- .../apache/fop/fo/flow/MultiCaseHandler.java | 38 ------ .../org/apache/fop/fo/flow/MultiSwitch.java | 16 --- .../org/apache/fop/fo/flow/MultiToggle.java | 37 +---- .../fop/layoutmgr/BestFitLayoutUtils.java | 36 +++-- .../apache/fop/layoutmgr/BestFitPenalty.java | 15 +- .../layoutmgr/MultiSwitchLayoutManager.java | 34 +---- .../fop/layoutmgr/PageBreakingAlgorithm.java | 10 +- .../multi-switch_best-fit.xml | 51 +++++++ .../multi-switch_best-fit_middle-page.xml | 63 +++++++++ ...ulti-switch_best-fit_multiple-variants.xml | 129 ++++++++++++++++++ 11 files changed, 283 insertions(+), 174 deletions(-) delete mode 100644 src/java/org/apache/fop/fo/flow/MultiCaseHandler.java create mode 100644 test/layoutengine/standard-testcases/multi-switch_best-fit.xml create mode 100644 test/layoutengine/standard-testcases/multi-switch_best-fit_middle-page.xml create mode 100644 test/layoutengine/standard-testcases/multi-switch_best-fit_multiple-variants.xml diff --git a/src/java/org/apache/fop/fo/flow/MultiCase.java b/src/java/org/apache/fop/fo/flow/MultiCase.java index 09a854c3e..ef6e444e1 100644 --- a/src/java/org/apache/fop/fo/flow/MultiCase.java +++ b/src/java/org/apache/fop/fo/flow/MultiCase.java @@ -33,13 +33,11 @@ import org.apache.fop.fo.ValidationException; * TODO implement validateChildNode() */ public class MultiCase extends FObj { - // The value of properties relevant for fo:multi-case. + + // FO multi-case properties private int startingState; private String caseName; private String caseTitle; - private MultiCaseHandler multiCaseHandler; - // private ToBeImplementedProperty caseName; - // private ToBeImplementedProperty caseTitle; // Unused but valid items, commented out for performance: // private CommonAccessibility commonAccessibility; // End of property values @@ -59,12 +57,6 @@ public class MultiCase extends FObj { startingState = pList.get(PR_STARTING_STATE).getEnum(); caseName = pList.get(PR_CASE_NAME).getString(); caseTitle = pList.get(PR_CASE_TITLE).getString(); - if (startingState == EN_SHOW) { - MultiSwitch multiSwitch = (MultiSwitch) parent; - if (multiSwitch.getCurrentlyVisibleNode() == null) { - multiSwitch.setCurrentlyVisibleNode(this); - } - } } /** @@ -91,14 +83,6 @@ public class MultiCase extends FObj { } } - @Override - protected void addChildNode(FONode child) throws FOPException { - if (child instanceof MultiCaseHandler) { - multiCaseHandler = (MultiCaseHandler) child; - } - super.addChildNode(child); - } - /** @return the "starting-state" property */ public int getStartingState() { return startingState; @@ -126,12 +110,4 @@ public class MultiCase extends FObj { return caseTitle; } - public boolean hasToggle() { - return multiCaseHandler != null; - } - - MultiCaseHandler getHandler() { - return multiCaseHandler; - } - } diff --git a/src/java/org/apache/fop/fo/flow/MultiCaseHandler.java b/src/java/org/apache/fop/fo/flow/MultiCaseHandler.java deleted file mode 100644 index 25a11bd68..000000000 --- a/src/java/org/apache/fop/fo/flow/MultiCaseHandler.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.fop.fo.flow; - -import org.apache.fop.apps.FOPException; - -/** - * This interface is used in conjunction with {@link MultiSwitch} - * to implement a callback system for handling fo:multi-case elements. - */ - -public interface MultiCaseHandler { - - /** - * A filtering function used to select one or more fo:multi-case - * elements, children of {@link MultiSwitch}. - * - * @param multiSwitch parent of the fo:multi-case elements to filter - * @throws FOPException - */ - void filter(MultiSwitch multiSwitch) throws FOPException; - -} diff --git a/src/java/org/apache/fop/fo/flow/MultiSwitch.java b/src/java/org/apache/fop/fo/flow/MultiSwitch.java index 4197b8675..6c4333cff 100644 --- a/src/java/org/apache/fop/fo/flow/MultiSwitch.java +++ b/src/java/org/apache/fop/fo/flow/MultiSwitch.java @@ -68,22 +68,6 @@ public class MultiSwitch extends FObj { super.endOfNode(); } - @Override - public void finalizeNode() throws FOPException { - if (autoToggle.equals("best-fit")) { - // Nothing to do in this case - setCurrentlyVisibleNode(null); - } else { - FONodeIterator nodeIter = getChildNodes(); - while (nodeIter.hasNext()) { - MultiCase multiCase = (MultiCase) nodeIter.next(); - if (multiCase.hasToggle()) { - multiCase.getHandler().filter(this); - } - } - } - } - /** * {@inheritDoc} *
XSL Content Model: (multi-case+) diff --git a/src/java/org/apache/fop/fo/flow/MultiToggle.java b/src/java/org/apache/fop/fo/flow/MultiToggle.java index 04da3083a..05a634ada 100644 --- a/src/java/org/apache/fop/fo/flow/MultiToggle.java +++ b/src/java/org/apache/fop/fo/flow/MultiToggle.java @@ -33,7 +33,7 @@ import org.apache.fop.fo.properties.StringProperty; * Class modelling the * fo:multi-toggle property. */ -public class MultiToggle extends FObj implements MultiCaseHandler { +public class MultiToggle extends FObj { // The value of properties relevant for fo:multi-toggle (commented out for performance). // private CommonAccessibility commonAccessibility; public StringProperty prSwitchTo; @@ -92,39 +92,4 @@ public class MultiToggle extends FObj implements MultiCaseHandler { return FO_MULTI_TOGGLE; } - public void filter(MultiSwitch multiSwitch) throws FOPException { - if (multiSwitch.getCurrentlyVisibleNode() == null) { - multiSwitch.setCurrentlyVisibleNode(parent); - } - - FONode currentlyVisibleMultiCase = multiSwitch.getCurrentlyVisibleNode(); - - if (prSwitchTo.getString().equals("xsl-any")) { -// NoOp - } else if (prSwitchTo.getString().equals("xsl-preceding")) { - FONodeIterator nodeIter = multiSwitch.getChildNodes(currentlyVisibleMultiCase); - if (nodeIter != null) { - if (!nodeIter.hasPrevious()) { - currentlyVisibleMultiCase = nodeIter.lastNode(); - } else { - currentlyVisibleMultiCase = nodeIter.previousNode(); - } - } - } else if (prSwitchTo.getString().equals("xsl-following")) { - FONodeIterator nodeIter = multiSwitch.getChildNodes(currentlyVisibleMultiCase); - if (nodeIter != null) { - //Ignore the first node - nodeIter.next(); - if (!nodeIter.hasNext()) { - currentlyVisibleMultiCase = nodeIter.firstNode(); - } else { - currentlyVisibleMultiCase = nodeIter.nextNode(); - } - } - } else { - // Pick an fo:multi-case that matches a case-name... - } - - multiSwitch.setCurrentlyVisibleNode(currentlyVisibleMultiCase); - } } diff --git a/src/java/org/apache/fop/layoutmgr/BestFitLayoutUtils.java b/src/java/org/apache/fop/layoutmgr/BestFitLayoutUtils.java index 17dddc239..b5b712a33 100644 --- a/src/java/org/apache/fop/layoutmgr/BestFitLayoutUtils.java +++ b/src/java/org/apache/fop/layoutmgr/BestFitLayoutUtils.java @@ -20,7 +20,10 @@ package org.apache.fop.layoutmgr; import java.util.LinkedList; import java.util.List; -/* +import org.apache.fop.fo.flow.MultiSwitch; +import org.apache.fop.layoutmgr.BestFitPenalty.Variant; + +/** * Utility class used in {@link MultiSwitchLayoutManager} * to handle the best-fit property value if specified in {@link MultiSwitch} */ @@ -30,32 +33,27 @@ public final class BestFitLayoutUtils { static class BestFitPosition extends Position { - public List knuthList; + private List knuthList; public BestFitPosition(LayoutManager lm) { super(lm); } - public BestFitPosition(LayoutManager lm, List knuthList) { - super(lm); - this.knuthList = knuthList; - } - public List getPositionList() { List positions = new LinkedList(); if (knuthList != null) { SpaceResolver.performConditionalsNotification(knuthList, 0, knuthList.size() - 1, -1); for (ListElement elem : knuthList) { - if (elem.getPosition() != null && elem.getLayoutManager() != null) { - positions.add(elem.getPosition()); - } + positions.add(elem.getPosition()); } } return positions; } + public void setKnuthList(List knuthList) { this.knuthList = knuthList; } + } public static List getKnuthList(LayoutManager lm, @@ -65,14 +63,23 @@ public final class BestFitLayoutUtils { BestFitPenalty bestFitPenalty = new BestFitPenalty(new BestFitPosition(lm)); for (List childList : childrenLists) { + // TODO Doing space resolution here is not correct. + // Space elements will simply be nulled. SpaceResolver.resolveElementList(childList); int contentLength = ElementListUtils.calcContentLength(childList); - bestFitPenalty.addVariant(childList, contentLength); + bestFitPenalty.addVariant(new Variant(childList, contentLength)); } - // TODO Adding the two enclosing boxes is definitely a dirty hack. - // Let's leave it like that for now, until I find a proper fix. + // TODO Adding two enclosing boxes is definitely a dirty hack. + // The first box forces the breaking algorithm to consider the penalty + // in case there are no elements preceding it + // and the last box prevents the glue and penalty from getting deleted + // when they are at the end of the Knuth list. knuthList.add(new KnuthBox(0, new Position(lm), false)); knuthList.add(bestFitPenalty); + Variant firstVariant = bestFitPenalty.getVariants().get(0); + BestFitPosition pos = new BestFitPosition(lm); + pos.setKnuthList(firstVariant.knuthList); + knuthList.add(new KnuthGlue(firstVariant.width, 0, 0, pos, false)); knuthList.add(new KnuthBox(0, new Position(lm), false)); return knuthList; } @@ -82,9 +89,8 @@ public final class BestFitLayoutUtils { // "unwrap" the NonLeafPositions stored in parentIter // and put them in a new list; LinkedList positionList = new LinkedList(); - Position pos; while (posIter.hasNext()) { - pos = posIter.next(); + Position pos = posIter.next(); if (pos instanceof BestFitPosition) { positionList.addAll(((BestFitPosition) pos).getPositionList()); } else { diff --git a/src/java/org/apache/fop/layoutmgr/BestFitPenalty.java b/src/java/org/apache/fop/layoutmgr/BestFitPenalty.java index 0f77edc8d..334a36900 100644 --- a/src/java/org/apache/fop/layoutmgr/BestFitPenalty.java +++ b/src/java/org/apache/fop/layoutmgr/BestFitPenalty.java @@ -27,11 +27,11 @@ import org.apache.fop.layoutmgr.BestFitLayoutUtils.BestFitPosition; /** * A type of penalty used to specify a set of alternatives for the layout engine * to choose from. The chosen alternative must have an occupied size - * that is less than the available BPD of the current page. + * less than the remaining BPD on the active page. */ public class BestFitPenalty extends KnuthPenalty { - public class Variant { + public static class Variant { public final List knuthList; public final int width; @@ -40,13 +40,14 @@ public class BestFitPenalty extends KnuthPenalty { this.knuthList = knuthList; this.width = width; } + public KnuthElement toPenalty() { return new KnuthPenalty(width, 0, false, null, false); } + } private final BestFitPosition bestFitPosition; - private final List variantList; public BestFitPenalty(BestFitPosition pos) { @@ -55,15 +56,15 @@ public class BestFitPenalty extends KnuthPenalty { variantList = new ArrayList(); } - public void addVariant(List knuthList, int width) { - variantList.add(new Variant(knuthList, width)); + public void addVariant(Variant variant) { + variantList.add(variant); } - public void activatePenalty(Variant bestVariant) { + public void setActiveVariant(Variant bestVariant) { bestFitPosition.setKnuthList(bestVariant.knuthList); } - public List getVariantList() { + public List getVariants() { return variantList; } diff --git a/src/java/org/apache/fop/layoutmgr/MultiSwitchLayoutManager.java b/src/java/org/apache/fop/layoutmgr/MultiSwitchLayoutManager.java index 386aba39b..8ef901a56 100644 --- a/src/java/org/apache/fop/layoutmgr/MultiSwitchLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/MultiSwitchLayoutManager.java @@ -24,10 +24,7 @@ import java.util.List; import org.apache.fop.area.Area; import org.apache.fop.area.Block; import org.apache.fop.area.LineArea; -import org.apache.fop.fo.FONode; -import org.apache.fop.fo.FONode.FONodeIterator; import org.apache.fop.fo.FObj; -import org.apache.fop.fo.flow.MultiSwitch; public class MultiSwitchLayoutManager extends BlockStackingLayoutManager { @@ -41,7 +38,7 @@ public class MultiSwitchLayoutManager extends BlockStackingLayoutManager { public List getNextKnuthElements(LayoutContext context, int alignment) { referenceIPD = context.getRefIPD(); - List> childrenLists = new LinkedList>(); + List> childrenLists = new ArrayList>(); LayoutManager childLM; while ((childLM = getChildLM()) != null) { if (!childLM.isFinished()) { @@ -58,33 +55,6 @@ public class MultiSwitchLayoutManager extends BlockStackingLayoutManager { return BestFitLayoutUtils.getKnuthList(this, childrenLists); } - @Override - protected List createChildLMs(int size) { - MultiSwitch multiSwitch = (MultiSwitch) getFObj(); - if (multiSwitch.getCurrentlyVisibleNode() != null) { - List newLMs = new ArrayList(size); - if (childLMs.size() == 0) { - createMultiCaseLM(multiSwitch.getCurrentlyVisibleNode()); - return new ArrayList(size); - } - return newLMs; - } else { - return super.createChildLMs(size); - } - } - - private void createMultiCaseLM(FONode multiCase) { - FONodeIterator childIter = multiCase.getChildNodes(); - while (childIter.hasNext()) { - List newLMs = new ArrayList(1); - getPSLM().getLayoutManagerMaker() - .makeLayoutManagers((childIter.nextNode()), newLMs); - if (!newLMs.isEmpty()) { - this.getParent().addChildLM(newLMs.get(0)); - } - } - } - @Override public Keep getKeepTogether() { return Keep.KEEP_AUTO; @@ -151,6 +121,8 @@ public class MultiSwitchLayoutManager extends BlockStackingLayoutManager { AreaAdditionUtil.addAreas(this, newPosIter, context); flush(); + // TODO removing the following line forces the generated area + // to be rendered twice in some cases... curBlockArea = null; } diff --git a/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java b/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java index 6abc7e350..a5ed077d8 100644 --- a/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java +++ b/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java @@ -99,7 +99,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm { private int currentKeepContext = Constants.EN_AUTO; private KnuthNode lastBeforeKeepContextSwitch; - /** Holds the variant that should be assigned to the next node to be created */ + /** Holds the variant of a dynamic content that must be attached to the next page node */ private Variant variant; /** @@ -458,7 +458,6 @@ class PageBreakingAlgorithm extends BreakingAlgorithm { /** {@inheritDoc} */ @Override protected void considerLegalBreak(KnuthElement element, int elementIdx) { - variant = null; if (element.isPenalty()) { int breakClass = ((KnuthPenalty) element).getBreakClass(); switch (breakClass) { @@ -526,6 +525,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm { int actualWidth = totalWidth - pageNode.totalWidth; int footnoteSplit; boolean canDeferOldFN; + variant = null; if (element.isPenalty()) { if (element instanceof BestFitPenalty) { actualWidth += handleBestFitPenalty(activeNode, (BestFitPenalty) element, elementIndex); @@ -592,7 +592,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm { } private int handleBestFitPenalty(KnuthNode activeNode, BestFitPenalty penalty, int elementIndex) { - for (Variant var : penalty.getVariantList()) { + for (Variant var : penalty.getVariants()) { int difference = computeDifference(activeNode, var.toPenalty(), elementIndex); double r = computeAdjustmentRatio(activeNode, difference); if (r >= -1.0) { @@ -1015,11 +1015,11 @@ class PageBreakingAlgorithm extends BreakingAlgorithm { int total) { //int difference = (bestActiveNode.line < total) // ? bestActiveNode.difference : bestActiveNode.difference + fillerMinWidth; - // Check if the given node has an attached dynamic content + // Check if the given node has an attached variant of a dynamic content KnuthPageNode pageNode = (KnuthPageNode) bestActiveNode; if (pageNode.variant != null) { BestFitPenalty penalty = (BestFitPenalty) par.get(pageNode.position); - penalty.activatePenalty(pageNode.variant); + penalty.setActiveVariant(pageNode.variant); } int difference = bestActiveNode.difference; if (difference + bestActiveNode.availableShrink < 0) { diff --git a/test/layoutengine/standard-testcases/multi-switch_best-fit.xml b/test/layoutengine/standard-testcases/multi-switch_best-fit.xml new file mode 100644 index 000000000..d41fa1c13 --- /dev/null +++ b/test/layoutengine/standard-testcases/multi-switch_best-fit.xml @@ -0,0 +1,51 @@ + + + + +

+ Basic test for fo:multi-switch using the whitespace management extension. +

+ + + + + + + + + + + + + First variant + + + Second variant + + + + + + + + + + + + diff --git a/test/layoutengine/standard-testcases/multi-switch_best-fit_middle-page.xml b/test/layoutengine/standard-testcases/multi-switch_best-fit_middle-page.xml new file mode 100644 index 000000000..6b9342b85 --- /dev/null +++ b/test/layoutengine/standard-testcases/multi-switch_best-fit_middle-page.xml @@ -0,0 +1,63 @@ + + + + +

+ Dynamic content in the middle of a page: the first variant is selected. +

+ + + + + + + + + + + Before the multi-switch + + + First variant + + + Second variant + Second variant + + + After the multi-switch + Filler 1 + Filler 2 + This should be on page 2. + + + + + + + + + + + + + + + + diff --git a/test/layoutengine/standard-testcases/multi-switch_best-fit_multiple-variants.xml b/test/layoutengine/standard-testcases/multi-switch_best-fit_multiple-variants.xml new file mode 100644 index 000000000..bb0715fa0 --- /dev/null +++ b/test/layoutengine/standard-testcases/multi-switch_best-fit_multiple-variants.xml @@ -0,0 +1,129 @@ + + + + +

+ Check that the right variant is being selected (if any), depending on the space left on the + page. +

+ + + + + + + + + + + Page 1 line 1 + Page 1 line 2 + Page 1 line 3 + Filler + Before the multi-switch + + + Variant 1 line 1 + Variant 1 line 2 + + + Variant 2 line 1 + + + This text should be on page 3. + + + + + + Page 1 line 1 + Page 1 line 2 + Page 2 line 1 + Filler + Before the multi-switch + + + Variant 1 line 1 + Variant 1 line 2 + + + Variant 2 line 1 + + + This text should be on page 3. + + + + + + Page 1 line 1 + Page 1 line 2 + Page 2 line 1 + Page 2 line 2 + Filler + Before the multi-switch + + + Variant 1 line 1 + Variant 1 line 2 + + + Variant 2 line 1 + + + This text should be on page 3. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + -- 2.39.5