Avoid unwrapping and rewrapping of Positions; use inspection instead. Remove unused method removeWordSpace

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

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

@@ -48,14 +48,39 @@ public class Position {
     }
 
     /**
-     * Overridden by NonLeafPosition to return the Position of its
-     * child LM.
+     * @param depth the depth at which the LM in this position is found
+     * @return associated layout manager
+     */
+    public LayoutManager getLM(int depth) {
+        Position subPos = getPosition(depth);
+        if (subPos == null) {
+            return null;
+        } else {
+            return subPos.getLM();
+        }
+    }
+
+    /**
+     * Overridden by NonLeafPosition to return the Position of its child LM.
      * @return a position or null
      */
     public Position getPosition() {
         return null;
     }
 
+    /**
+     * Overridden by NonLeafPosition to return the Position of its child LM.
+     * @param depth the depth at which the position in this position is found
+     * @return a position or null
+     */
+    public Position getPosition(int depth) {
+        Position subPos = this;
+        for (int i = 0; i < depth && subPos != null; ++i, subPos = subPos.getPosition()) {
+            // no-op
+        }
+        return subPos;
+    }
+
     /** @return true if generates areas */
     public boolean generatesAreas() {
         return false;

src/java/org/apache/fop/layoutmgr/inline/ContentLayoutManager.java

@@ -292,14 +292,9 @@ public class ContentLayoutManager extends AbstractBaseLayoutManager
         return oldList;
     }
 
-    /**
-     * Remove the word space represented by the given elements
-     *
-     * @param oldList the elements representing the word space
-     */
-    public void removeWordSpace(List oldList) {
-        // do nothing
-        log.warn(this.getClass().getName() + " should not receive a call to removeWordSpace(list)");
+    /** {@inheritDoc} */
+    public List addALetterSpaceTo(List oldList, int depth) {
+        return addALetterSpaceTo(oldList);
     }
 
     /** {@inheritDoc} */
@@ -317,12 +312,20 @@ public class ContentLayoutManager extends AbstractBaseLayoutManager
     }
 
     /** {@inheritDoc} */
-    public List getChangedKnuthElements(List oldList,
-                                              /*int flaggedPenalty,*/
-                                              int alignment) {
+    public boolean applyChanges(List oldList, int depth) {
+        return applyChanges(oldList);
+    }
+
+    /** {@inheritDoc} */
+    public List getChangedKnuthElements(List oldList, int alignment) {
         return null;
     }
 
+    /** {@inheritDoc} */
+    public List getChangedKnuthElements(List oldList, int alignment, int depth) {
+        return getChangedKnuthElements(oldList, alignment);
+    }
+
     /** {@inheritDoc} */
     public PageSequenceLayoutManager getPSLM() {
         return parentLM.getPSLM();

src/java/org/apache/fop/layoutmgr/inline/FootnoteLayoutManager.java

@@ -122,9 +122,8 @@ public class FootnoteLayoutManager extends InlineStackingLayoutManager {
     /**
      * {@inheritDoc}
      */
-    public List getChangedKnuthElements(List oldList,
-                                              int alignment) {
-        List returnedList = super.getChangedKnuthElements(oldList, alignment);
+    public List getChangedKnuthElements(List oldList, int alignment, int depth) {
+        List returnedList = super.getChangedKnuthElements(oldList, alignment, depth);
         addAnchor(returnedList);
         return returnedList;
     }

src/java/org/apache/fop/layoutmgr/inline/InlineLayoutManager.java

@@ -532,10 +532,10 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
     }
 
     /** {@inheritDoc} */
-    public List getChangedKnuthElements(List oldList, int alignment) {
+    public List getChangedKnuthElements(List oldList, int alignment, int depth) {
         List returnedList = new LinkedList();
         addKnuthElementsForBorderPaddingStart(returnedList);
-        returnedList.addAll(super.getChangedKnuthElements(oldList, alignment));
+        returnedList.addAll(super.getChangedKnuthElements(oldList, alignment, depth));
         addKnuthElementsForBorderPaddingEnd(returnedList);
         return returnedList;
     }

src/java/org/apache/fop/layoutmgr/inline/InlineLevelLayoutManager.java

@@ -40,12 +40,15 @@ public interface InlineLevelLayoutManager extends LayoutManager {
     List addALetterSpaceTo(List oldList);
 
     /**
-     * Tell the LM to modify its data, removing the word space
-     * represented by the given elements
+     * Tell the LM to modify its data, adding a letter space
+     * to the word fragment represented by the given elements,
+     * and returning the corrected elements
      *
-     * @param oldList the elements representing the word space
+     * @param oldList the elements which must be given one more letter space
+     * @param depth the depth at which the Positions for this LM in oldList are found
+     * @return        the new elements replacing the old ones
      */
-    void removeWordSpace(List oldList);
+List addALetterSpaceTo(List oldList, int depth);
 
     /**
      * Get the word chars corresponding to the given position.
@@ -71,4 +74,23 @@ public interface InlineLevelLayoutManager extends LayoutManager {
      */
     boolean applyChanges(List oldList);
 
+    /**
+     * Tell the LM to apply the changes due to hyphenation
+     *
+     * @param oldList the list of the old elements the changes refer to
+     * @param depth the depth at which the Positions for this LM in oldList are found
+     * @return        true if the LM had to change its data, false otherwise
+     */
+    boolean applyChanges(List oldList, int depth);
+
+    /**
+     * Get a sequence of KnuthElements representing the content
+     * of the node assigned to the LM, after changes have been applied
+     * @param oldList        the elements to replace
+     * @param alignment      the desired text alignment
+     * @param depth the depth at which the Positions for this LM in oldList are found
+     * @return               the updated list of KnuthElements
+     **/
+    List getChangedKnuthElements(List oldList, int alignment, int depth);
+
 }

src/java/org/apache/fop/layoutmgr/inline/InlineStackingLayoutManager.java

@@ -217,54 +217,48 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
 
     /** {@inheritDoc} */
     public List addALetterSpaceTo(List oldList) {
+        return addALetterSpaceTo(oldList, 0);
+    }
+
+    /** {@inheritDoc} */
+    public List addALetterSpaceTo(List oldList, int thisDepth) {
         // old list contains only a box, or the sequence: box penalty glue box
 
-        ListIterator oldListIterator = oldList.listIterator();
-        KnuthElement element = null;
-        // "unwrap" the Position stored in each element of oldList
-        while (oldListIterator.hasNext()) {
-            element = (KnuthElement) oldListIterator.next();
-            element.setPosition(element.getPosition().getPosition());
-        }
+        ListIterator oldListIterator = oldList.listIterator(oldList.size());
+        KnuthElement element = (KnuthElement) oldListIterator.previous();
+        int depth = thisDepth + 1;
 
         // The last element may not have a layout manager (its position == null);
         // this may happen if it is a padding box; see bug 39571.
-        InlineLevelLayoutManager lm
-            = (InlineLevelLayoutManager) element.getLayoutManager();
-        if (lm != null) {
-            oldList = lm.addALetterSpaceTo(oldList);
+        Position pos = element.getPosition();
+        InlineLevelLayoutManager lm = null;
+        if (pos != null) {
+            lm = (InlineLevelLayoutManager) pos.getLM(depth);
+        }
+        if (lm == null) {
+            return oldList;
         }
-        // "wrap" again the Position stored in each element of oldList
+        oldList = lm.addALetterSpaceTo(oldList, depth);
+        // "wrap" the Position stored in new elements of oldList
         oldListIterator = oldList.listIterator();
         while (oldListIterator.hasNext()) {
             element = (KnuthElement) oldListIterator.next();
-            element.setPosition(notifyPos(new NonLeafPosition(this, element.getPosition())));
+            pos = element.getPosition();
+            lm = null;
+            if (pos != null) {
+                lm = (InlineLevelLayoutManager) pos.getLM(thisDepth);
+            }
+            // in old elements the position at thisDepth is a position for this LM
+            // only wrap new elements
+            if (lm != this) {
+                // new element, wrap position
+                element.setPosition(notifyPos(new NonLeafPosition(this, element.getPosition())));
+            }
         }
 
         return oldList;
     }
 
-    /**
-     * remove the AreaInfo object represented by the given elements,
-     * so that it won't generate any element when getChangedKnuthElements
-     * will be called
-     *
-     * @param oldList the elements representing the word space
-     */
-    public void removeWordSpace(List oldList) {
-        ListIterator oldListIterator = oldList.listIterator();
-        KnuthElement element = null;
-        // "unwrap" the Position stored in each element of oldList
-        while (oldListIterator.hasNext()) {
-            element = (KnuthElement) oldListIterator.next();
-            element.setPosition(element.getPosition().getPosition());
-        }
-
-        ((InlineLevelLayoutManager)
-                   element.getLayoutManager()).removeWordSpace(oldList);
-
-    }
-
     /** {@inheritDoc} */
     public String getWordChars(Position pos) {
         Position newPos = pos.getPosition();
@@ -280,17 +274,14 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
 
     /** {@inheritDoc} */
     public boolean applyChanges(List oldList) {
-        // "unwrap" the Positions stored in the elements
+        return applyChanges(oldList, 0);
+    }
+
+    /** {@inheritDoc} */
+    public boolean applyChanges(List oldList, int depth) {
         ListIterator oldListIterator = oldList.listIterator();
         KnuthElement oldElement;
-        while (oldListIterator.hasNext()) {
-            oldElement = (KnuthElement) oldListIterator.next();
-            if (oldElement.getPosition() != null) {
-                oldElement.setPosition(oldElement.getPosition().getPosition());
-            }
-        }
-        // reset the iterator
-        oldListIterator = oldList.listIterator();
+        depth += 1;
 
         InlineLevelLayoutManager prevLM = null;
         InlineLevelLayoutManager currLM;
@@ -299,7 +290,12 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
         boolean bSomethingChanged = false;
         while (oldListIterator.hasNext()) {
             oldElement = (KnuthElement) oldListIterator.next();
-            currLM = (InlineLevelLayoutManager) oldElement.getLayoutManager();
+            Position pos = oldElement.getPosition();
+            if (pos == null) {
+                currLM = null;
+            } else {
+                currLM = (InlineLevelLayoutManager) pos.getLM(depth);
+            }
             // initialize prevLM
             if (prevLM == null) {
                 prevLM = currLM;
@@ -310,41 +306,34 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
                     prevLM = currLM;
                 } else if (oldListIterator.hasNext()) {
                     bSomethingChanged
-                        = prevLM.applyChanges(oldList.subList(fromIndex
-                                                              , oldListIterator.previousIndex()))
+                        = prevLM.applyChanges(oldList.subList(fromIndex,
+                                                              oldListIterator.previousIndex()),
+                                                              depth)
                         || bSomethingChanged;
                     prevLM = currLM;
                     fromIndex = oldListIterator.previousIndex();
                 } else if (currLM == prevLM) {
                     bSomethingChanged
                         = (prevLM != null)
-                            && prevLM.applyChanges(oldList.subList(fromIndex, oldList.size()))
+                            && prevLM.applyChanges(oldList.subList(fromIndex,
+                                                                   oldList.size()), depth)
                             || bSomethingChanged;
                 } else {
                     bSomethingChanged
-                        = prevLM.applyChanges(oldList.subList(fromIndex
-                                                              , oldListIterator.previousIndex()))
+                        = prevLM.applyChanges(oldList.subList(fromIndex,
+                                                              oldListIterator.previousIndex()),
+                                                              depth)
                             || bSomethingChanged;
                     if (currLM != null) {
                         bSomethingChanged
-                            = currLM.applyChanges(oldList.subList(oldListIterator.previousIndex()
-                                                                  , oldList.size()))
+                            = currLM.applyChanges(oldList.subList(oldListIterator.previousIndex(),
+                                                                  oldList.size()), depth)
                             || bSomethingChanged;
                     }
                 }
             }
         }
 
-        // "wrap" again the Positions stored in the elements
-        oldListIterator = oldList.listIterator();
-        while (oldListIterator.hasNext()) {
-            oldElement = (KnuthElement) oldListIterator.next();
-            NonLeafPosition newPos = new NonLeafPosition(this, oldElement.getPosition());
-            if (newPos.generatesAreas()) {
-                notifyPos(newPos);
-            }
-            oldElement.setPosition(newPos);
-        }
         return bSomethingChanged;
     }
 
@@ -352,16 +341,15 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
      * {@inheritDoc}
      */
     public List getChangedKnuthElements(List oldList, int alignment) {
+        return getChangedKnuthElements(oldList, alignment, 0);
+    }
+
+    /** {@inheritDoc} */
+    public List getChangedKnuthElements(List oldList, int alignment, int depth) {
         // "unwrap" the Positions stored in the elements
         ListIterator oldListIterator = oldList.listIterator();
         KnuthElement oldElement;
-        while (oldListIterator.hasNext()) {
-            oldElement = (KnuthElement) oldListIterator.next();
-            oldElement.setPosition
-                (oldElement.getPosition().getPosition());
-        }
-        // reset the iterator
-        oldListIterator = oldList.listIterator();
+        depth += 1;
 
         KnuthElement returnedElement;
         LinkedList returnedList = new LinkedList();
@@ -372,7 +360,12 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
 
         while (oldListIterator.hasNext()) {
             oldElement = (KnuthElement) oldListIterator.next();
-            currLM = (InlineLevelLayoutManager) oldElement.getLayoutManager();
+            Position pos = oldElement.getPosition();
+            if (pos == null) {
+                currLM = null;
+            } else {
+                currLM = (InlineLevelLayoutManager) pos.getLM(depth);
+            }
             if (prevLM == null) {
                 prevLM = currLM;
             }
@@ -381,33 +374,31 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
                 if (oldListIterator.hasNext()) {
                     returnedList.addAll
                         (prevLM.getChangedKnuthElements
-                         (oldList.subList(fromIndex,
-                                          oldListIterator.previousIndex()),
-                          /*flaggedPenalty,*/ alignment));
+                         (oldList.subList(fromIndex, oldListIterator.previousIndex()),
+                          alignment, depth));
                     prevLM = currLM;
                     fromIndex = oldListIterator.previousIndex();
                 } else if (currLM == prevLM) {
                     returnedList.addAll
                         (prevLM.getChangedKnuthElements
                          (oldList.subList(fromIndex, oldList.size()),
-                          /*flaggedPenalty,*/ alignment));
+                          alignment, depth));
                 } else {
                     returnedList.addAll
                         (prevLM.getChangedKnuthElements
-                         (oldList.subList(fromIndex,
-                                          oldListIterator.previousIndex()),
-                          /*flaggedPenalty,*/ alignment));
+                         (oldList.subList(fromIndex, oldListIterator.previousIndex()),
+                          alignment, depth));
                     if (currLM != null) {
                         returnedList.addAll
                             (currLM.getChangedKnuthElements
-                             (oldList.subList(oldListIterator.previousIndex(),
-                                              oldList.size()),
-                              /*flaggedPenalty,*/ alignment));
+                             (oldList.subList(oldListIterator.previousIndex(), oldList.size()),
+                              alignment, depth));
                     }
                 }
             }
         }
 
+        // this is a new list
         // "wrap" the Position stored in each element of returnedList
         ListIterator listIter = returnedList.listIterator();
         while (listIter.hasNext()) {
@@ -416,6 +407,7 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
                 (notifyPos(new NonLeafPosition(this, returnedElement.getPosition())));
             returnList.add(returnedElement);
         }
+
         return returnList;
     }
 }

src/java/org/apache/fop/layoutmgr/inline/LeafNodeLayoutManager.java

@@ -297,13 +297,11 @@ public abstract class LeafNodeLayoutManager extends AbstractLayoutManager
     }
 
     /**
-     * Remove the word space represented by the given elements
-     *
-     * @param oldList the elements representing the word space
+     * {@inheritDoc}
+     * Only TextLM has a meaningful implementation of this method
      */
-    public void removeWordSpace(List oldList) {
-        // do nothing
-        log.warn(this.getClass().getName() + " should not receive a call to removeWordSpace(list)");
+    public List addALetterSpaceTo(List oldList, int depth) {
+        return addALetterSpaceTo(oldList);
     }
 
     /** {@inheritDoc} */
@@ -321,9 +319,24 @@ public abstract class LeafNodeLayoutManager extends AbstractLayoutManager
         return false;
     }
 
+    /**
+     * {@inheritDoc}
+     * Only TextLM has a meaningful implementation of this method
+     */
+    public boolean applyChanges(List oldList, int depth) {
+        return applyChanges(oldList);
+    }
+
+    /**
+     * {@inheritDoc}
+     * No subclass has a meaningful implementation of this method
+     */
+    public List getChangedKnuthElements(List oldList, int alignment, int depth) {
+        return getChangedKnuthElements(oldList, alignment);
+    }
+
     /** {@inheritDoc} */
-    public List getChangedKnuthElements(List oldList,
-                                              int alignment) {
+    public List getChangedKnuthElements(List oldList, int alignment) {
         if (isFinished()) {
             return null;
         }

src/java/org/apache/fop/layoutmgr/inline/LineLayoutManager.java

@@ -1167,6 +1167,11 @@ public class LineLayoutManager extends InlineStackingLayoutManager
 
     /** {@inheritDoc} */
     @Override
+    public List getChangedKnuthElements(List oldList, int alignment, int depth) {
+        return getChangedKnuthElements(oldList, alignment);
+    }
+
+    /** {@inheritDoc} */
     public List getChangedKnuthElements(List oldList, int alignment) {
         List returnList = new LinkedList();
         for (int p = 0; p < knuthParagraphs.size(); p++) {

src/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java

@@ -844,14 +844,20 @@ public class TextLayoutManager extends LeafNodeLayoutManager {
     }
 
     /** {@inheritDoc} */
-    public List addALetterSpaceTo(final List oldList) {
+    public List addALetterSpaceTo(List oldList) {
+        return addALetterSpaceTo(oldList, 0);
+    }
+
+    /** {@inheritDoc} */
+    public List addALetterSpaceTo(final List oldList, int depth) {
         // old list contains only a box, or the sequence: box penalty glue box;
         // look at the Position stored in the first element in oldList
         // which is always a box
         ListIterator oldListIterator = oldList.listIterator();
         KnuthElement knuthElement = (KnuthElement) oldListIterator.next();
-        LeafPosition pos = (LeafPosition) ((KnuthBox) knuthElement).getPosition();
-        int index = pos.getLeafPos();
+        Position pos = knuthElement.getPosition();
+        LeafPosition leafPos = (LeafPosition) pos.getPosition(depth);
+        int index = leafPos.getLeafPos();
         //element could refer to '-1' position, for non-collapsed spaces (?)
         if (index > -1) {
             AreaInfo areaInfo = getAreaInfo(index);
@@ -866,6 +872,7 @@ public class TextLayoutManager extends LeafNodeLayoutManager {
                 oldListIterator.add(new KnuthGlue(letterSpaceIPD, auxiliaryPosition, false));
             } else if (letterSpaceIPD.isStiff()) {
                 // constant letter space: replace the box
+                // give it the unwrapped position of the replaced element
                 oldListIterator.set(new KnuthInlineBox(areaInfo.areaIPD.getOpt(),
                         alignmentContext, pos, false));
             } else {
@@ -879,36 +886,6 @@ public class TextLayoutManager extends LeafNodeLayoutManager {
         return oldList;
     }
 
-    /**
-     * Removes the <code>AreaInfo</code> object represented by the given elements, so that it won't
-     * generate any element when <code>getChangedKnuthElements</code> is called.
-     *
-     * @param oldList the elements representing the word space
-     */
-    public void removeWordSpace(final List oldList) {
-        // find the element storing the Position whose value
-        // points to the AreaInfo object
-        final ListIterator oldListIterator = oldList.listIterator();
-        if (((KnuthElement) ((LinkedList) oldList).getFirst()).isPenalty()) {
-            // non breaking space: oldList starts with a penalty
-            oldListIterator.next();
-        }
-        if (oldList.size() > 2) {
-            // alignment is either center, start or end:
-            // the first two elements does not store the needed Position
-            oldListIterator.next();
-            oldListIterator.next();
-        }
-        KnuthElement knuthElement = (KnuthElement) oldListIterator.next();
-        int leafValue = ((LeafPosition) knuthElement.getPosition()).getLeafPos();
-        // only the last word space can be a trailing space!
-        if (leafValue == areaInfos.size() - 1) {
-            areaInfos.remove(leafValue);
-        } else {
-            TextLayoutManager.LOG.error("trying to remove a non-trailing word space");
-        }
-    }
-
     /** {@inheritDoc} */
     public void hyphenate(Position pos, HyphContext hyphContext) {
         AreaInfo areaInfo = getAreaInfo(((LeafPosition) pos).getLeafPos() + changeOffset);
@@ -976,6 +953,11 @@ public class TextLayoutManager extends LeafNodeLayoutManager {
 
     /** {@inheritDoc} */
     public boolean applyChanges(final List oldList) {
+        return applyChanges(oldList, 0);
+    }
+
+    /** {@inheritDoc} */
+    public boolean applyChanges(final List oldList, int depth) {
 
         // make sure the LM appears unfinished in between this call
         // and the next call to getChangedKnuthElements()
@@ -990,13 +972,15 @@ public class TextLayoutManager extends LeafNodeLayoutManager {
         LeafPosition startPos = null, endPos = null;
         ListIterator oldListIter;
         for (oldListIter = oldList.listIterator(); oldListIter.hasNext();) {
-            startPos = (LeafPosition) ((KnuthElement) oldListIter.next()).getPosition();
+            Position pos = ((KnuthElement) oldListIter.next()).getPosition();
+            startPos = (LeafPosition) pos.getPosition(depth);
             if (startPos != null && startPos.getLeafPos() != -1) {
                 break;
             }
         }
         for (oldListIter = oldList.listIterator(oldList.size()); oldListIter.hasPrevious();) {
-            endPos = (LeafPosition) ((KnuthElement) oldListIter.previous()).getPosition();
+            Position pos = ((KnuthElement) oldListIter.previous()).getPosition();
+            endPos = (LeafPosition) pos.getPosition(depth);
             if (endPos != null && endPos.getLeafPos() != -1) {
                 break;
             }

test/layoutengine/standard-testcases/inline_letter-spacing_3.xml

@@ -48,7 +48,7 @@
     </fo:root>
   </fo>
   <checks>
-    <eval expected="105384"
+    <eval expected="117384"
 xpath="//flow/block[1]/lineArea[1]/inlineparent[1]/@ipd" desc="IPD of
 containing inline area"/>
     <eval expected="73368"