You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by vh...@apache.org on 2009/06/23 17:50:16 UTC

svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

Author: vhennebert
Date: Tue Jun 23 15:50:15 2009
New Revision: 787733

URL: http://svn.apache.org/viewvc?rev=787733&view=rev
Log:
Code clean-up

Modified:
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/LeafPosition.java
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/PageBreaker.java
    xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java Tue Jun 23 15:50:15 2009
@@ -260,14 +260,6 @@
     /**
      * Starts the page breaking process.
      * @param flowBPD the constant available block-progression-dimension (used for every part)
-     */
-    public void doLayout(int flowBPD) {
-        doLayout(flowBPD, false);
-    }
-
-    /**
-     * Starts the page breaking process.
-     * @param flowBPD the constant available block-progression-dimension (used for every part)
      * @param autoHeight true if warnings about overflows should be disabled because the
      *                   the BPD is really undefined (for footnote-separators, for example)
      */

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java Tue Jun 23 15:50:15 2009
@@ -67,9 +67,6 @@
     private MinOptMax effSpaceBefore;
     private MinOptMax effSpaceAfter;
 
-    /** The list of child BreakPoss instances. */
-    protected List childBreaks = new java.util.ArrayList();
-
     /**
      * Creates a new BlockLayoutManager.
      * @param inBlock the block FO object to create the layout manager for.

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java Tue Jun 23 15:50:15 2009
@@ -1083,12 +1083,8 @@
      * @return the width/length in millipoints
      */
     protected int getLineWidth(int line) {
-        if (this.lineWidth < 0) {
-            throw new IllegalStateException("lineWidth must be set"
-                    + (this.lineWidth != 0 ? " and positive, but it is: " + this.lineWidth : ""));
-        } else {
-            return this.lineWidth;
-        }
+        assert lineWidth >= 0;
+        return this.lineWidth;
     }
 
     /** @return the constant line/part width or -1 if there is no such value */

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java Tue Jun 23 15:50:15 2009
@@ -25,11 +25,10 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.apache.fop.area.Area;
 import org.apache.fop.area.BlockParent;
 import org.apache.fop.fo.pagination.Flow;
-import org.apache.fop.layoutmgr.inline.InlineLevelLayoutManager;
-import org.apache.fop.layoutmgr.inline.WrapperLayoutManager;
 
 /**
  * LayoutManager for an fo:flow object.
@@ -63,23 +62,11 @@
     /** {@inheritDoc} */
     public List getNextKnuthElements(LayoutContext context, int alignment) {
 
-        // set layout dimensions
-        int flowIPD = getCurrentPV().getCurrentSpan().getColumnWidth();
-        int flowBPD = getCurrentPV().getBodyRegion().getBPD();
-
         // currently active LM
         LayoutManager curLM;
-        List returnedList;
-        List returnList = new LinkedList();
+        List elements = new LinkedList();
 
         while ((curLM = getChildLM()) != null) {
-            if (!(curLM instanceof WrapperLayoutManager)
-                && curLM instanceof InlineLevelLayoutManager) {
-                log.error("inline area not allowed under flow - ignoring");
-                curLM.setFinished(true);
-                continue;
-            }
-
             int span = EN_NONE;
             int disableColumnBalancing = EN_FALSE;
             if (curLM instanceof BlockLayoutManager) {
@@ -99,44 +86,41 @@
                 }
                 log.debug("span change from " + currentSpan + " to " + span);
                 context.signalSpanChange(span);
-                SpaceResolver.resolveElementList(returnList);
-                return returnList;
+                SpaceResolver.resolveElementList(elements);
+                return elements;
             }
 
-            // Set up a LayoutContext
-            //MinOptMax bpd = context.getStackLimit();
-
             LayoutContext childLC = new LayoutContext(0);
             childLC.setStackLimitBP(context.getStackLimitBP());
             childLC.setRefIPD(context.getRefIPD());
             childLC.setWritingMode(getCurrentPage().getSimplePageMaster().getWritingMode());
 
             // get elements from curLM
-            returnedList = curLM.getNextKnuthElements(childLC, alignment);
+            List childrenElements = curLM.getNextKnuthElements(childLC, alignment);
             //log.debug("FLM.getNextKnuthElements> returnedList.size() = " + returnedList.size());
-            if (returnList.size() == 0 && childLC.isKeepWithPreviousPending()) {
+            if (elements.size() == 0 && childLC.isKeepWithPreviousPending()) {
                 context.updateKeepWithPreviousPending(childLC.getKeepWithPreviousPending());
                 childLC.clearKeepWithPreviousPending();
             }
 
             // "wrap" the Position inside each element
-            List tempList = returnedList;
-            returnedList = new LinkedList();
-            wrapPositionElements(tempList, returnedList);
+            List tempList = childrenElements;
+            childrenElements = new LinkedList();
+            wrapPositionElements(tempList, childrenElements);
 
-            if (returnedList.size() == 1
-                && ElementListUtils.endsWithForcedBreak(returnedList)) {
+            if (childrenElements.size() == 1
+                    && ElementListUtils.endsWithForcedBreak(childrenElements)) {
                 // a descendant of this flow has break-before
-                returnList.addAll(returnedList);
-                SpaceResolver.resolveElementList(returnList);
-                return returnList;
-            } else if (returnedList.size() > 0) {
-                if (returnList.size() > 0
-                        && !ElementListUtils.startsWithForcedBreak(returnedList)) {
-                    addInBetweenBreak(returnList, context, childLC);
+                elements.addAll(childrenElements);
+                SpaceResolver.resolveElementList(elements);
+                return elements;
+            } else if (childrenElements.size() > 0) {
+                if (elements.size() > 0
+                        && !ElementListUtils.startsWithForcedBreak(childrenElements)) {
+                    addInBetweenBreak(elements, context, childLC);
                 }
-                returnList.addAll(returnedList);
-                if (ElementListUtils.endsWithForcedBreak(returnList)) {
+                elements.addAll(childrenElements);
+                if (ElementListUtils.endsWithForcedBreak(elements)) {
                     if (curLM.isFinished() && !hasNextChildLM()) {
                         //If the layout manager is finished at this point, the pending
                         //marks become irrelevant.
@@ -145,8 +129,8 @@
                         break;
                     }
                     // a descendant of this flow has break-after
-                    SpaceResolver.resolveElementList(returnList);
-                    return returnList;
+                    SpaceResolver.resolveElementList(elements);
+                    return elements;
                 }
             }
 
@@ -157,11 +141,11 @@
             context.updateKeepWithNextPending(getKeepWithNextStrength());
         }
 
-        SpaceResolver.resolveElementList(returnList);
+        SpaceResolver.resolveElementList(elements);
         setFinished(true);
 
-        if (returnList.size() > 0) {
-            return returnList;
+        if (elements.size() > 0) {
+            return elements;
         } else {
             return null;
         }

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/LeafPosition.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/LeafPosition.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/LeafPosition.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/LeafPosition.java Tue Jun 23 15:50:15 2009
@@ -21,15 +21,15 @@
 
 public class LeafPosition extends Position {
 
-    private int iLeafPos;
+    private int leafPos;
 
     public LeafPosition(LayoutManager lm, int pos) {
         super(lm);
-        iLeafPos = pos;
+        leafPos = pos;
     }
 
     public int getLeafPos() {
-        return iLeafPos;
+        return leafPos;
     }
 
     public boolean generatesAreas() {

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/PageBreaker.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/PageBreaker.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/PageBreaker.java Tue Jun 23 15:50:15 2009
@@ -77,6 +77,14 @@
         return pslm.getPageProvider();
     }
 
+    /**
+     * Starts the page breaking process.
+     * @param flowBPD the constant available block-progression-dimension (used for every part)
+     */
+    void doLayout(int flowBPD) {
+        doLayout(flowBPD, false);
+    }
+
     /** {@inheritDoc} */
     protected PageBreakingLayoutListener createLayoutListener() {
         return new PageBreakingLayoutListener() {
@@ -350,7 +358,7 @@
                     1, true, BreakingAlgorithm.ALL_BREAKS);
         AbstractBreaker.log.debug("restart: optimalPageCount= " + optimalPageCount
                 + " pageBreaks.size()= " + algRestart.getPageBreaks().size());
-        
+
         boolean fitsOnePage
                 = optimalPageCount <= pslm.getCurrentPV().getBodyRegion().getColumnCount();
 
@@ -542,4 +550,4 @@
             return true;
         }
     }
-}
\ No newline at end of file
+}

Modified: xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java (original)
+++ xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java Tue Jun 23 15:50:15 2009
@@ -600,22 +600,20 @@
     private void collectInlineKnuthElements(LayoutContext context) {
         LayoutContext inlineLC = new LayoutContext(context);
 
-        InlineLevelLayoutManager curLM;
-        List returnedList = null;
         ipd = context.getStackLimitIP().opt;
 
         // convert all the text in a sequence of paragraphs made
         // of KnuthBox, KnuthGlue and KnuthPenalty objects
-        boolean bPrevWasKnuthBox = false;
+        boolean previousIsBox = false;
 
         StringBuffer trace = new StringBuffer("LineLM:");
 
         Paragraph lastPar = null;
 
+        InlineLevelLayoutManager curLM;
         while ((curLM = (InlineLevelLayoutManager) getChildLM()) != null) {
-            returnedList = curLM.getNextKnuthElements(inlineLC, effectiveAlignment);
-            if (returnedList == null
-                    || returnedList.size() == 0) {
+            List inlineElements = curLM.getNextKnuthElements(inlineLC, effectiveAlignment);
+            if (inlineElements == null || inlineElements.size() == 0) {
                 /* curLM.getNextKnuthElements() returned null or an empty list;
                  * this can happen if there is nothing more to layout,
                  * so just iterate once more to see if there are other children */
@@ -623,7 +621,7 @@
             }
 
             if (lastPar != null) {
-                KnuthSequence firstSeq = (KnuthSequence) returnedList.get(0);
+                KnuthSequence firstSeq = (KnuthSequence) inlineElements.get(0);
 
                 // finish last paragraph before a new block sequence
                 if (!firstSeq.isInlineSequence()) {
@@ -633,7 +631,7 @@
                     if (log.isTraceEnabled()) {
                         trace.append(" ]");
                     }
-                    bPrevWasKnuthBox = false;
+                    previousIsBox = false;
                 }
 
                 // does the first element of the first paragraph add to an existing word?
@@ -641,14 +639,14 @@
                     KnuthElement thisElement;
                     thisElement = (KnuthElement) firstSeq.get(0);
                     if (thisElement.isBox() && !thisElement.isAuxiliary()
-                            && bPrevWasKnuthBox) {
+                            && previousIsBox) {
                         lastPar.addALetterSpace();
                     }
                 }
             }
 
             // loop over the KnuthSequences (and single KnuthElements) in returnedList
-            ListIterator iter = returnedList.listIterator();
+            ListIterator iter = inlineElements.listIterator();
             while (iter.hasNext()) {
                 KnuthSequence sequence = (KnuthSequence) iter.next();
                 // the sequence contains inline Knuth elements
@@ -656,9 +654,9 @@
                     // look at the last element
                     ListElement lastElement = sequence.getLast();
                     assert lastElement != null;
-                    bPrevWasKnuthBox = lastElement.isBox()
-                                        && !((KnuthElement) lastElement).isAuxiliary()
-                                        && ((KnuthElement) lastElement).getW() != 0;
+                    previousIsBox = lastElement.isBox()
+                            && !((KnuthElement) lastElement).isAuxiliary()
+                            && ((KnuthElement) lastElement).getW() != 0;
 
                     // if last paragraph is open, add the new elements to the paragraph
                     // else this is the last paragraph
@@ -683,8 +681,7 @@
 
                     // finish last paragraph if it was closed with a linefeed
                     if (lastElement.isPenalty()
-                            && ((KnuthPenalty) lastElement).getP()
-                            == -KnuthPenalty.INFINITE) {
+                            && ((KnuthPenalty) lastElement).getP() == -KnuthPenalty.INFINITE) {
                         // a penalty item whose value is -inf
                         // represents a preserved linefeed,
                         // which forces a line break
@@ -700,7 +697,7 @@
                         if (log.isTraceEnabled()) {
                             trace.append(" ]");
                         }
-                        bPrevWasKnuthBox = false;
+                        previousIsBox = false;
                     }
                 } else { // the sequence is a block sequence
                     // the positions will be wrapped with this LM in postProcessLineBreaks
@@ -939,15 +936,14 @@
                     while (elementIterator.nextIndex() <= endIndex) {
                         KnuthElement element = (KnuthElement) elementIterator.next();
                         if (element instanceof KnuthInlineBox
-                            && ((KnuthInlineBox) element).isAnchor()) {
+                                && ((KnuthInlineBox) element).isAnchor()) {
                             footnoteList.add(((KnuthInlineBox) element).getFootnoteBodyLM());
                         } else if (element instanceof KnuthBlockBox) {
                             footnoteList.addAll(((KnuthBlockBox) element).getFootnoteBodyLMs());
                         }
                     }
                     startIndex = endIndex + 1;
-                    LineBreakPosition lbp
-                      = (LineBreakPosition) llPoss.getChosenPosition(i);
+                    LineBreakPosition lbp = (LineBreakPosition) llPoss.getChosenPosition(i);
                     returnList.add(new KnuthBlockBox
                                    (lbp.lineHeight + lbp.spaceBefore + lbp.spaceAfter,
                                     footnoteList, lbp, false));



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

Posted by Andreas Delmelle <an...@telenet.be>.
On 23 Jun 2009, at 19:11, Vincent Hennebert wrote:

Hi Vincent

>> <snip />
>> Maybe nothing, since the check will always return false (should be
>> caught during FO tree validation, but then there's relaxed  
>> validation...).
>
> o.a.fop.fo.pagination.Flow explicitly checks that every child  
> element is
> a block-level element, even in relaxed validation mode. Also, after
> replacing the log with a ‘throw new IllegalStateException()’ the whole
> test suite ran without any problem. So I considered it safe to remove.

OK. The only thing I remembered about the check, was modifying it when  
implementing fo:wrappers as direct children of the fo:flow, but as I  
read it now, it seems like that could indeed only happen if the FO is  
invalid, and so the check is obsolete.

>
>> All the other changes receive my blessing! :-)
>
> Thanks for double-checking!

Also, no problem! I have a lot more of those cleanups following  
shortly (removal of superfluous code, renaming stray Hungarians...),  
in the classes affected by the column-keeps patch. AFAICT from your  
commits, the conflicts should be minimal, if any.

Just mentioning this so you're aware. If you find anything dubious,  
chances are that I've already changed it locally, and it will be  
committed to the trunk pretty soon.


Regards,

Andreas



Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

Posted by Vincent Hennebert <vh...@gmail.com>.
Hi Andreas,

Andreas Delmelle wrote:
> On 23 Jun 2009, at 17:50, vhennebert apache org wrote:
> 
>> Author: vhennebert
>> Date: Tue Jun 23 15:50:15 2009
>> New Revision: 787733
>>
>> URL: http://svn.apache.org/viewvc?rev=787733&view=rev
>> Log:
>> Code clean-up
>>
> <snip />
>> http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
> <snip />
>> -            if (!(curLM instanceof WrapperLayoutManager)
>> -                && curLM instanceof InlineLevelLayoutManager) {
>> -                log.error("inline area not allowed under flow -
>> ignoring");
>> -                curLM.setFinished(true);
>> -                continue;
>> -            }
> 
> This may be too much cleanup. I'm not entirely certain, but the
> 'continue' statement is meant to prevent this condition from crashing
> FOP on something we can perfectly recover from... (IIRC, without this
> check, we would end up with a ClassCastException when adding the areas)
> 
> Admitted, this event would better be routed through the event mechanism,
> so users can decide for themselves. As long as that has not been done...
> 
> Maybe nothing, since the check will always return false (should be
> caught during FO tree validation, but then there's relaxed validation...).

o.a.fop.fo.pagination.Flow explicitly checks that every child element is
a block-level element, even in relaxed validation mode. Also, after
replacing the log with a ‘throw new IllegalStateException()’ the whole
test suite ran without any problem. So I considered it safe to remove.


> All the other changes receive my blessing! :-)

Thanks for double-checking!

Vincent

Re: svn commit: r787733 - in /xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr: ./ inline/

Posted by Andreas Delmelle <an...@telenet.be>.
On 23 Jun 2009, at 17:50, vhennebert@apache.org wrote:

> Author: vhennebert
> Date: Tue Jun 23 15:50:15 2009
> New Revision: 787733
>
> URL: http://svn.apache.org/viewvc?rev=787733&view=rev
> Log:
> Code clean-up
>
<snip />
> http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_ChangingIPDHack/src/java/org/apache/fop/layoutmgr/FlowLayoutManager.java?rev=787733&r1=787732&r2=787733&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
<snip />
> -            if (!(curLM instanceof WrapperLayoutManager)
> -                && curLM instanceof InlineLevelLayoutManager) {
> -                log.error("inline area not allowed under flow -  
> ignoring");
> -                curLM.setFinished(true);
> -                continue;
> -            }

This may be too much cleanup. I'm not entirely certain, but the  
'continue' statement is meant to prevent this condition from crashing  
FOP on something we can perfectly recover from... (IIRC, without this  
check, we would end up with a ClassCastException when adding the areas)

Admitted, this event would better be routed through the event  
mechanism, so users can decide for themselves. As long as that has not  
been done...

Maybe nothing, since the check will always return false (should be  
caught during FO tree validation, but then there's relaxed  
validation...).

All the other changes receive my blessing! :-)


Thanks!

Regards

Andreas