Bugzilla 41621:

- length of the penalty now correctly computed;
- AssertionError fixed.

git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/branches/fop-0_95@650550 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java b/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
index 7cdeb79d5569e197a556af606f894ad9167e2a3f..9d5b7c685dc791d15642f1fd9395263397acbde5 100644 (file)
--- a/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
@@ -23,6 +23,7 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 
 import org.apache.commons.logging.Log;
@@ -38,6 +39,7 @@ import org.apache.fop.layoutmgr.BreakElement;
 import org.apache.fop.layoutmgr.ElementListUtils;
 import org.apache.fop.layoutmgr.KnuthBox;
 import org.apache.fop.layoutmgr.KnuthElement;
+import org.apache.fop.layoutmgr.KnuthGlue;
 import org.apache.fop.layoutmgr.KnuthPossPosIter;
 import org.apache.fop.layoutmgr.LayoutContext;
 import org.apache.fop.layoutmgr.ListElement;
@@ -224,6 +226,17 @@ public class TableContentLayoutManager implements PercentBaseContext {
             while ((rowGroup = iter.getNextRowGroup()) != null) {
                 rowGroupLM = new RowGroupLayoutManager(getTableLM(), rowGroup, stepper);
                 nextRowGroupElems = rowGroupLM.getNextKnuthElements(context, alignment, bodyType);
+                /*
+                 * The last break element produced by TableStepper (for the previous row
+                 * group) may be used to represent the break between the two row groups.
+                 * Its penalty value and break class must just be overridden by the
+                 * characteristics of the keep or break between the two.
+                 * 
+                 * However, we mustn't forget that if the after border of the last row of
+                 * the row group is thicker in the normal case than in the trailing case,
+                 * an additional glue will be appended to the element list. So we may have
+                 * to go two steps backwards in the list.
+                 */
                 int penaltyValue = 0;
                 keepBetween |= context.isKeepWithPreviousPending();
                 if (keepBetween || tableLM.getTable().mustKeepTogether()) {
@@ -234,25 +247,36 @@ public class TableContentLayoutManager implements PercentBaseContext {
                 if (breakBetween != Constants.EN_AUTO) {
                     penaltyValue = -KnuthElement.INFINITE;
                 }
-                TableHFPenaltyPosition penaltyPos = new TableHFPenaltyPosition(getTableLM());
-                int penaltyLen = 0;
-                if (bodyType == TableRowIterator.BODY) {
-                    if (!getTableLM().getTable().omitHeaderAtBreak()) {
-                        penaltyLen += getHeaderNetHeight();
-                        penaltyPos.headerElements = getHeaderElements();
-                    }
-                    if (!getTableLM().getTable().omitFooterAtBreak()) {
-                        penaltyLen += getFooterNetHeight();
-                        penaltyPos.footerElements = getFooterElements();
-                    }
+                BreakElement breakElement;
+                ListIterator elemIter = returnList.listIterator(returnList.size());
+                ListElement elem = (ListElement) elemIter.previous();
+                if (elem instanceof KnuthGlue) {
+                    breakElement = (BreakElement) elemIter.previous();
+                } else {
+                    breakElement = (BreakElement) elem;
                 }
-                returnList.add(new BreakElement(penaltyPos, 
-                        penaltyLen, penaltyValue, breakBetween, context));
+                breakElement.setPenaltyValue(penaltyValue);
+                breakElement.setBreakClass(breakBetween);
                 returnList.addAll(nextRowGroupElems);
                 breakBetween = context.getBreakAfter();
                 keepBetween = context.isKeepWithNextPending();
             }
         }
+        /*
+         * The last break produced for the last row-group of this table part must be
+         * removed, because the breaking after the table will be handled by TableLM.
+         * Unless the element list ends with a glue, which must be kept to accurately
+         * represent the content. In such a case the break is simply disabled by setting
+         * its penalty to infinite.
+         */
+        ListIterator elemIter = returnList.listIterator(returnList.size());
+        ListElement elem = (ListElement) elemIter.previous();
+        if (elem instanceof KnuthGlue) {
+            BreakElement breakElement = (BreakElement) elemIter.previous();
+            breakElement.setPenaltyValue(KnuthElement.INFINITE);
+        } else {
+            elemIter.remove();
+        }
         context.setFlags(LayoutContext.KEEP_WITH_PREVIOUS_PENDING, keepWithPrevious);
         context.setBreakBefore(breakBefore);
 

diff --git a/src/java/org/apache/fop/layoutmgr/table/TableStepper.java b/src/java/org/apache/fop/layoutmgr/table/TableStepper.java
index ba67e38e41dda26b0ba4802fb4850a010c52f892..c45e621baca4d0942947f1b97bc574e9e52a68be 100644 (file)
--- a/src/java/org/apache/fop/layoutmgr/table/TableStepper.java
+++ b/src/java/org/apache/fop/layoutmgr/table/TableStepper.java
@@ -267,13 +267,8 @@ public class TableStepper {
             laststep = step;
             step = getNextStep();
         } while (step >= 0);
-        if (!returnList.isEmpty()) {
-            lastTCPos.setFlag(TableContentPosition.LAST_IN_ROWGROUP, true);
-            // It's not up to TableStepper to decide whether there can/must be a break
-            // after the row group or not, but to ancestor stacking elements
-            assert returnList.getLast() instanceof BreakElement;
-            returnList.removeLast();
-        }
+        assert !returnList.isEmpty();
+        lastTCPos.setFlag(TableContentPosition.LAST_IN_ROWGROUP, true);
         return returnList;
     }
 

diff --git a/test/layoutengine/standard-testcases/table_bug44621.xml b/test/layoutengine/standard-testcases/table_bug44621.xml
index b4b9edbd037d14c335c618fe95c97985ddc4b29b..ae04d1903718095a36d80473e24c45521e94dcbe 100644 (file)
--- a/test/layoutengine/standard-testcases/table_bug44621.xml
+++ b/test/layoutengine/standard-testcases/table_bug44621.xml
@@ -66,12 +66,32 @@
               </fo:table-row>
             </fo:table-body>
           </fo:table>
+
+          <fo:block space-before="20pt" space-after="10pt">The after border of cell 1, in the normal
+            case, is thicker than in the trailing case.</fo:block>
+          <fo:table width="100%" table-layout="fixed"
+            font-size="8pt" line-height="10pt">
+            <fo:table-body>
+              <fo:table-row>
+                <fo:table-cell border="1pt solid black">
+                  <fo:block>Cell 1</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+              <fo:table-row>
+                <fo:table-cell border="2pt solid black">
+                  <fo:block>Cell 2</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+            </fo:table-body>
+          </fo:table>
+
         </fo:flow>
       </fo:page-sequence>
     </fo:root>
   </fo>
   <checks>
 
+    <!-- table 1 -->
     <eval expected="39000" xpath="//pageViewport//flow/block[2]/@bpd"/>
     <eval expected="39000" xpath="//pageViewport//flow/block[2]/@bpda"/>
     <!-- cell 1.1 -->
@@ -93,12 +113,27 @@
     <eval expected="10000" xpath="//pageViewport//flow/block[2]/block[6]/@bpd"/>
     <eval expected="18000" xpath="//pageViewport//flow/block[2]/block[6]/@bpda"/>
 
+    <!-- table 2 -->
+    <eval expected="23500" xpath="//pageViewport//flow/block[4]/@bpd"/>
+    <eval expected="23500" xpath="//pageViewport//flow/block[4]/@bpda"/>
+    <!-- cell 1 -->
+    <eval expected="10000" xpath="//pageViewport//flow/block[4]/block[1]/@bpd"/>
+    <eval expected="13000" xpath="//pageViewport//flow/block[4]/block[1]/@bpda"/>
+    <!-- cell 2 -->
+    <eval expected="10000" xpath="//pageViewport//flow/block[4]/block[2]/@bpd"/>
+    <eval expected="14000" xpath="//pageViewport//flow/block[4]/block[2]/@bpda"/>
+
     <element-list category="breaker">
       <skip>4</skip>
       <box w="12000"/>
-      <penalty w="14000" p="0"/><!-- should be 15000 -->
+      <penalty w="15000" p="0"/>
       <box w="13000"/>
       <box w="14000"/>
+      <skip>6</skip>
+      <box w="11000"/>
+      <penalty w="0" p="0"/>
+      <glue w="500"/>
+      <box w="12000"/>
       <skip>3</skip>
     </element-list>
   </checks>