On 12/07/12 14:47, Glenn Adams wrote:
> On Thu, Jul 12, 2012 at 7:45 AM, Vincent Hennebert 
> <vhenneb...@gmail.com>wrote:
> 
>> On 12/07/12 14:37, Glenn Adams wrote:
>>> Is this addressing a specific bug? If not, then please enter a bug and
>>> update the status to ensure this association for documentation and
>> release
>>> notes tracking purposes.
>>
>> I added an entry in the status.xml file.
>>
> 
> Yes, I noticed. But there is no bug #.  There should be a documented bug
> against every non-trivial substantive fix of this nature.

Why? As long as the status.xml file is the authoritative list of bug
fixes and implemented features, adding an entry to Bugzilla is just
redundant paperwork I’m not keen on doing. Unless I missed something, of
course...


>>> Also, I notice you left a printf in place below.  Is that 
>>> intentional? If
>>> so, then I would suggest using something other than printf.
>>
>> It’s intentional, to have the association between test index and test
>> file when investigating failing test cases.
>>
>>
>> Vincent
>>
>>
>>
>>> On Thu, Jul 12, 2012 at 7:25 AM, <vhennebert> wrote:
>>>
>>>> Author: vhennebert
>>>> Date: Thu Jul 12 13:25:34 2012
>>>> New Revision: 1360665
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1360665&view=rev
>>>> Log:
>>>> Bugfix: When restarting layout for the last page, discard glues and
>>>> penalties at the beginning of the restarted Knuth sequence.
>>>>
>>>> Modified:
>>>>
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>>
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>>
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>>
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>>     xmlgraphics/fop/trunk/status.xml
>>>>
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>>
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -492,7 +492,6 @@ public abstract class AbstractBreaker {
>>>>      protected void addAreas(PageBreakingAlgorithm alg, int startPart,
>> int
>>>> partCount,
>>>>              BlockSequence originalList, BlockSequence effectiveList) {
>>>>          LayoutContext childLC;
>>>> -        // add areas
>>>>          int startElementIndex = 0;
>>>>          int endElementIndex = 0;
>>>>          int lastBreak = -1;
>>>> @@ -550,12 +549,7 @@ public abstract class AbstractBreaker {
>>>>
>>>>              // ignore KnuthGlue and KnuthPenalty objects
>>>>              // at the beginning of the line
>>>> -            ListIterator<KnuthElement> effectiveListIterator
>>>> -                = effectiveList.listIterator(startElementIndex);
>>>> -            while (effectiveListIterator.hasNext()
>>>> -                    && !(effectiveListIterator.next()).isBox()) {
>>>> -                startElementIndex++;
>>>> -            }
>>>> +            startElementIndex =
>>>> alg.par.getFirstBoxIndex(startElementIndex);
>>>>
>>>>              if (startElementIndex <= endElementIndex) {
>>>>                  if (log.isDebugEnabled()) {
>>>> @@ -576,7 +570,9 @@ public abstract class AbstractBreaker {
>>>>                          && p < (partCount - 1)) {
>>>>                      // count the boxes whose width is not 0
>>>>                      int boxCount = 0;
>>>> -                    effectiveListIterator =
>>>> effectiveList.listIterator(startElementIndex);
>>>> +                    @SuppressWarnings("unchecked")
>>>> +                    ListIterator<KnuthElement> effectiveListIterator =
>>>> effectiveList
>>>> +                            .listIterator(startElementIndex);
>>>>                      while (effectiveListIterator.nextIndex() <=
>>>> endElementIndex) {
>>>>                          KnuthElement tempEl =
>>>> effectiveListIterator.next();
>>>>                          if (tempEl.isBox() && tempEl.getWidth() > 0) {
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -532,14 +532,15 @@ public abstract class BreakingAlgorithm
>>>>          // index of the first KnuthBox in the sequence, in case of
>>>> non-centered
>>>>          // alignment. For centered alignment, we need to take into
>>>> account preceding
>>>>          // penalties+glues used for the filler spaces
>>>> -        int firstBoxIndex = startIndex;
>>>> +        int previousPosition = startIndex;
>>>>          if (alignment != Constants.EN_CENTER) {
>>>> -            firstBoxIndex = par.getFirstBoxIndex(startIndex);
>>>> +            int firstBoxIndex = par.getFirstBoxIndex(startIndex);
>>>> +            previousPosition = (firstBoxIndex >= par.size()) ?
>> startIndex
>>>> : firstBoxIndex - 1;
>>>>          }
>>>> -        firstBoxIndex = (firstBoxIndex < 0) ? 0 : firstBoxIndex;
>>>> +        previousPosition = (previousPosition < 0) ? 0 :
>> previousPosition;
>>>>
>>>>          // create an active node representing the starting point
>>>> -        addNode(0, createNode(firstBoxIndex, 0, 1, 0, 0, 0, 0, 0, 0, 0,
>>>> 0, null));
>>>> +        addNode(0, createNode(previousPosition, 0, 1, 0, 0, 0, 0, 0, 0,
>>>> 0, 0, null));
>>>>          KnuthNode lastForced = getNode(0);
>>>>
>>>>          if (log.isTraceEnabled()) {
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/KnuthSequence.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -20,6 +20,7 @@
>>>>  package org.apache.fop.layoutmgr;
>>>>
>>>>  import java.util.ArrayList;
>>>> +import java.util.Iterator;
>>>>  import java.util.List;
>>>>  import java.util.ListIterator;
>>>>
>>>> @@ -159,46 +160,26 @@ public abstract class KnuthSequence exte
>>>>                  : (ListElement) get(index);
>>>>      }
>>>>
>>>> -    /** @return the position index of the first box in this sequence */
>>>> -    protected int getFirstBoxIndex() {
>>>> -        if (isEmpty()) {
>>>> -            return -1;
>>>> -        } else {
>>>> -            return getFirstBoxIndex(0);
>>>> -        }
>>>> -    }
>>>> -
>>>>      /**
>>>> -     * Get the position index of the first box in this sequence,
>>>> -     * starting at the given index. If there is no box after the
>>>> -     * passed {@code startIndex}, the starting index itself is
>> returned.
>>>> -     * @param startIndex    the starting index for the lookup
>>>> -     * @return  the absolute position index of the next box element
>>>> +     * Returns the position index of the first box in this sequence,
>>>> starting at the given
>>>> +     * index. If {@code startIndex} is outside the bounds of this
>>>> sequence, it is
>>>> +     * returned.
>>>> +     *
>>>> +     * @param startIndex the index from which to start the lookup
>>>> +     * @return the index of the next box element, {@link #size()} if
>>>> there is no such
>>>> +     * element, {@code startIndex} if {@code (startIndex < 0 ||
>>>> startIndex >= size())}
>>>>       */
>>>>      protected int getFirstBoxIndex(int startIndex) {
>>>> -        if (isEmpty() || startIndex < 0 || startIndex >= size()) {
>>>> -            return -1;
>>>> +        if (startIndex < 0 || startIndex >= size()) {
>>>> +            return startIndex;
>>>>          } else {
>>>> -            ListElement element = null;
>>>> -            int posIndex = startIndex;
>>>> -            int lastIndex = size();
>>>> -            while ( posIndex < lastIndex ) {
>>>> -                element = getElement(posIndex);
>>>> -                if ( !element.isBox() ) {
>>>> -                    posIndex++;
>>>> -                } else {
>>>> -                    break;
>>>> -                }
>>>> -            }
>>>> -            if ( posIndex != startIndex ) {
>>>> -                if ( ( element != null ) && element.isBox() ) {
>>>> -                    return posIndex - 1;
>>>> -                } else {
>>>> -                    return startIndex;
>>>> -                }
>>>> -            } else {
>>>> -                return startIndex;
>>>> +            int boxIndex = startIndex;
>>>> +            @SuppressWarnings("unchecked")
>>>> +            Iterator<ListElement> iter = listIterator(startIndex);
>>>> +            while (iter.hasNext() && !iter.next().isBox()) {
>>>> +                boxIndex++;
>>>>              }
>>>> +            return boxIndex;
>>>>          }
>>>>      }
>>>>
>>>>
>>>> Modified:
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>> (original)
>>>> +++
>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreaker.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -330,7 +330,7 @@ public class PageBreaker extends Abstrac
>>>>              //Get page break from which we restart
>>>>              PageBreakPosition pbp = (PageBreakPosition)
>>>>                      alg.getPageBreaks().get(restartPoint - 1);
>>>> -            newStartPos = pbp.getLeafPos() + 1;
>>>> +            newStartPos = alg.par.getFirstBoxIndex(pbp.getLeafPos() +
>> 1);
>>>>              //Handle page break right here to avoid any side-effects
>>>>              if (newStartPos > 0) {
>>>>                  handleBreakTrait(Constants.EN_PAGE);
>>>>
>>>> Modified: xmlgraphics/fop/trunk/status.xml
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> --- xmlgraphics/fop/trunk/status.xml (original)
>>>> +++ xmlgraphics/fop/trunk/status.xml Thu Jul 12 13:25:34 2012
>>>> @@ -63,6 +63,10 @@
>>>>        documents. Example: the fix of marks layering will be such a case
>>>> when it's done.
>>>>      -->
>>>>      <release version="FOP Trunk" date="TBD">
>>>> +      <action context="Layout" dev="VH" type="fix">
>>>> +        When restarting layout for the last page, discard glues and
>>>> penalties at the beginning of
>>>> +        the restarted Knuth sequence.
>>>> +      </action>
>>>>        <action context="Test" dev="GA" type="fix">
>>>>          Fix errors and warnings in example files. Add build.xml for
>>>> documentation examples.
>>>>        </action>
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/intermediate/IFParserTestCase.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -48,9 +48,6 @@ import org.apache.fop.render.intermediat
>>>>  @RunWith(Parameterized.class)
>>>>  public class IFParserTestCase extends AbstractIFTest {
>>>>
>>>> -    /** Set this to true to get the correspondence between test number
>>>> and test file. */
>>>> -    private static final boolean DEBUG = false;
>>>> -
>>>>      /**
>>>>       * Gets the parameters for this test
>>>>       *
>>>> @@ -59,19 +56,7 @@ public class IFParserTestCase extends Ab
>>>>       */
>>>>      @Parameters
>>>>      public static Collection<File[]> getParameters() throws
>> IOException {
>>>> -        Collection<File[]> testFiles =
>>>> LayoutEngineTestUtils.getLayoutTestFiles();
>>>> -        if (DEBUG) {
>>>> -            printFiles(testFiles);
>>>> -        }
>>>> -        return testFiles;
>>>> -    }
>>>> -
>>>> -    private static void printFiles(Collection<File[]> files) {
>>>> -        int index = 0;
>>>> -        for (File[] file : files) {
>>>> -            assert file.length == 1;
>>>> -            System.out.println(String.format("%3d %s", index++,
>> file[0]));
>>>> -        }
>>>> +        return LayoutEngineTestUtils.getLayoutTestFiles();
>>>>      }
>>>>
>>>>      /**
>>>>
>>>> Modified:
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java?rev=1360665&r1=1360664&r2=1360665&view=diff
>>>>
>>>>
>> ==============================================================================
>>>> ---
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>> (original)
>>>> +++
>>>>
>> xmlgraphics/fop/trunk/test/java/org/apache/fop/layoutengine/LayoutEngineTestUtils.java
>>>> Thu Jul 12 13:25:34 2012
>>>> @@ -51,6 +51,9 @@ import org.apache.commons.io.filefilter.
>>>>   */
>>>>  public final class LayoutEngineTestUtils {
>>>>
>>>> +    /** Set this to true to get the correspondence between test number
>>>> and test file. */
>>>> +    private static final boolean DEBUG = false;
>>>> +
>>>>      private LayoutEngineTestUtils() {
>>>>      }
>>>>
>>>> @@ -157,8 +160,12 @@ public final class LayoutEngineTestUtils
>>>>          }
>>>>
>>>>          Collection<File[]> parametersForJUnit4 = new
>> ArrayList<File[]>();
>>>> +        int index = 0;
>>>>          for (File f : files) {
>>>>              parametersForJUnit4.add(new File[] { f });
>>>> +            if (DEBUG) {
>>>> +                System.out.println(String.format("%3d %s", index++,
>> f));
>>>> +            }
>>>>          }
>>>>
>>>>          return parametersForJUnit4;
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: fop-commits-unsubscr...@xmlgraphics.apache.org
>>>> For additional commands, e-mail:
>> fop-commits-h...@xmlgraphics.apache.org
>>>>
>>>>
>>>
>>
>>
> 

Reply via email to