Fix for bug #37270:

No more exceptions.

Code touch-up and logging optimization.

git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@331844 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 6c8b9e05bc088a71e24bc2eaf4ce3ae7a2b07a4f..1cc7df68ae8daaf39590158c7f17afc8a29fd519 100644 (file)
--- a/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
@@ -438,8 +438,10 @@ public class TableContentLayoutManager implements PercentBaseContext {
                             //Check for bpd on row, see CSS21, 17.5.3 Table height algorithms
                             LengthRangeProperty bpd = tableRow.getBlockProgressionDimension();
                             if (!bpd.getMinimum(getTableLM()).isAuto()) {
-                                minContentHeight = Math.max(minContentHeight, 
-                                        bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()));
+                                minContentHeight = Math.max(
+                                        minContentHeight, 
+                                        bpd.getMinimum(
+                                                getTableLM()).getLength().getValue(getTableLM()));
                             }
                             MinOptMaxUtil.restrict(explicitRowHeights[rgi], bpd, getTableLM());
                             
@@ -502,12 +504,14 @@ public class TableContentLayoutManager implements PercentBaseContext {
                         int effCellContentHeight = minContentHeight;
                         LengthRangeProperty bpd = primary.getCell().getBlockProgressionDimension();
                         if (!bpd.getMinimum(getTableLM()).isAuto()) {
-                            effCellContentHeight = Math.max(effCellContentHeight,
-                                    bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()));
+                            effCellContentHeight = Math.max(
+                                effCellContentHeight,
+                                bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()));
                         }
                         if (!bpd.getOptimum(getTableLM()).isAuto()) {
-                            effCellContentHeight = Math.max(effCellContentHeight,
-                                    bpd.getOptimum(getTableLM()).getLength().getValue(getTableLM()));
+                            effCellContentHeight = Math.max(
+                                effCellContentHeight,
+                                bpd.getOptimum(getTableLM()).getLength().getValue(getTableLM()));
                         }
                         if (gu.getRowSpanIndex() == 0) {
                             //TODO ATM only non-row-spanned cells are taken for this
@@ -770,21 +774,29 @@ public class TableContentLayoutManager implements PercentBaseContext {
         }
         
         public void handleTableContentPosition(TableContentPosition tcpos) {
-            log.debug("===handleTableContentPosition(" + tcpos);
             if (lastRow != tcpos.row && lastRow != null) {
                 addAreasAndFlushRow(false);
                 yoffset += lastRowHeight;
                 this.accumulatedBPD += lastRowHeight;
             }
-            rowFO = null;
+            if (log.isDebugEnabled()) {
+                log.debug("===handleTableContentPosition(" + tcpos);
+            }
+            rowFO = tcpos.row.getTableRow();
             lastRow = tcpos.row;
             Iterator partIter = tcpos.gridUnitParts.iterator();
             //Iterate over all grid units in the current step
             while (partIter.hasNext()) {
                 GridUnitPart gup = (GridUnitPart)partIter.next();
-                log.debug(">" + gup);
+                if (log.isDebugEnabled()) {
+                    log.debug(">" + gup);
+                }
                 int colIndex = gup.pgu.getStartCol();
                 if (gridUnits[colIndex] != gup.pgu) {
+                    if (gridUnits[colIndex] != null) {
+                        log.warn("Replacing GU in slot " + colIndex 
+                                + ". Some content may not be painted.");
+                    }
                     gridUnits[colIndex] = gup.pgu;
                     start[colIndex] = gup.start;
                     end[colIndex] = gup.end;
@@ -794,10 +806,6 @@ public class TableContentLayoutManager implements PercentBaseContext {
                     }
                     end[colIndex] = gup.end;
                 }
-                if (rowFO == null) {
-                    //Find the row if any
-                    rowFO = gridUnits[colIndex].getRow();
-                }
             }
         }
         
@@ -806,37 +814,51 @@ public class TableContentLayoutManager implements PercentBaseContext {
             int readyCount = 0;
             
             int bt = lastRow.getBodyType();
+            if (log.isDebugEnabled()) {
+                log.debug("Remembering yoffset for row " + lastRow.getIndex() + ": " + yoffset);
+            }
             rowOffsets[bt].put(new Integer(lastRow.getIndex()), new Integer(yoffset));
 
             for (int i = 0; i < gridUnits.length; i++) {
                 if ((gridUnits[i] != null) 
                         && (forcedFlush || (end[i] == gridUnits[i].getElements().size() - 1))) {
-                    log.debug("getting len for " + i + " " 
-                            + start[i] + "-" + end[i]);
+                    if (log.isTraceEnabled()) {
+                        log.trace("getting len for " + i + " " 
+                                + start[i] + "-" + end[i]);
+                    }
                     readyCount++;
                     int len = ElementListUtils.calcContentLength(
                             gridUnits[i].getElements(), start[i], end[i]);
                     partLength[i] = len;
-                    log.debug("len of part: " + len);
+                    if (log.isTraceEnabled()) {
+                        log.trace("len of part: " + len);
+                    }
 
                     if (start[i] == 0) {
                         LengthRangeProperty bpd = gridUnits[i].getCell()
                                 .getBlockProgressionDimension();
                         if (!bpd.getMinimum(getTableLM()).isAuto()) {
-                            if (bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()) > 0) {
-                                len = Math.max(len, bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()));
+                            int min = bpd.getMinimum(getTableLM())
+                                        .getLength().getValue(getTableLM()); 
+                            if (min > 0) {
+                                len = Math.max(len, bpd.getMinimum(getTableLM())
+                                        .getLength().getValue(getTableLM()));
                             }
                         }
                         if (!bpd.getOptimum(getTableLM()).isAuto()) {
-                            if (bpd.getOptimum(getTableLM()).getLength().getValue(getTableLM()) > 0) {
-                                len = Math.max(len, bpd.getOptimum(getTableLM()).getLength().getValue(getTableLM()));
+                            int opt = bpd.getOptimum(getTableLM())
+                                        .getLength().getValue(getTableLM());
+                            if (opt > 0) {
+                                len = Math.max(len, opt);
                             }
                         }
                         if (gridUnits[i].getRow() != null) {
                             bpd = gridUnits[i].getRow().getBlockProgressionDimension();
                             if (!bpd.getMinimum(getTableLM()).isAuto()) {
-                                if (bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()) > 0) {
-                                    len = Math.max(len, bpd.getMinimum(getTableLM()).getLength().getValue(getTableLM()));
+                                int min = bpd.getMinimum(getTableLM()).getLength()
+                                            .getValue(getTableLM()); 
+                                if (min > 0) {
+                                    len = Math.max(len, min);
                                 }
                             }
                         }
@@ -897,22 +919,30 @@ public class TableContentLayoutManager implements PercentBaseContext {
             return actualRowHeight;
         }
 
-        private void addAreasForCell(PrimaryGridUnit pgu, int start, int end, 
+        private void addAreasForCell(PrimaryGridUnit pgu, int startPos, int endPos, 
                 EffRow row, int contentHeight, int rowHeight) {
             int bt = row.getBodyType();
             if (firstRow[bt] < 0) {
                 firstRow[bt] = row.getIndex();
             }
             //Determine the first row in this sequence
-            //TODO Maybe optimize since addAreasAndFlushRow uses almost the same code
             int startRow = Math.max(pgu.getStartRow(), firstRow[bt]);
-            int effYOffset = ((Integer)rowOffsets[bt].get(new Integer(startRow))).intValue();
+            //Determine y offset for the cell
+            Integer offset = (Integer)rowOffsets[bt].get(new Integer(startRow));
+            while (offset == null) {
+                startRow--;
+                offset = (Integer)rowOffsets[bt].get(new Integer(startRow));
+            }
+            int effYOffset = offset.intValue();
             int effCellHeight = rowHeight;
             effCellHeight += yoffset - effYOffset;
-            log.debug("current row: " + row.getIndex());
-            log.debug("start row: " + pgu.getStartRow() + " " + yoffset + " " + effYOffset);
-            log.debug("contentHeight: " + contentHeight + " rowHeight=" + rowHeight 
-                    + " effCellHeight=" + effCellHeight);
+            if (log.isDebugEnabled()) {
+                log.debug("Creating area for cell:");
+                log.debug("  current row: " + row.getIndex());
+                log.debug("  start row: " + pgu.getStartRow() + " " + yoffset + " " + effYOffset);
+                log.debug("  contentHeight: " + contentHeight + " rowHeight=" + rowHeight 
+                        + " effCellHeight=" + effCellHeight);
+            }
             TableCellLayoutManager cellLM = pgu.getCellLM();
             cellLM.setXOffset(getXOffsetOfGridUnit(pgu));
             cellLM.setYOffset(effYOffset);
@@ -920,7 +950,7 @@ public class TableContentLayoutManager implements PercentBaseContext {
             cellLM.setRowHeight(effCellHeight);
             //cellLM.setRowHeight(row.getHeight().opt);
             cellLM.addAreas(new KnuthPossPosIter(pgu.getElements(), 
-                    start, end + 1), layoutContext);
+                    startPos, endPos + 1), layoutContext);
         }
         
     }
@@ -957,7 +987,8 @@ public class TableContentLayoutManager implements PercentBaseContext {
             rowBackground.setXOffset(this.startXOffset);
             rowBackground.setYOffset(yoffset);
             getTableLM().addChildArea(rowBackground);
-            TraitSetter.addBackground(rowBackground, row.getCommonBorderPaddingBackground(), getTableLM());
+            TraitSetter.addBackground(rowBackground, 
+                    row.getCommonBorderPaddingBackground(), getTableLM());
         }
     }
     
@@ -1079,8 +1110,10 @@ public class TableContentLayoutManager implements PercentBaseContext {
         public String toString() {
             StringBuffer sb = new StringBuffer("TableContentPosition:");
             sb.append(getIndex());
-            sb.append("[").append(getFlag(FIRST_IN_ROWGROUP) ? "F" : "-");
-            sb.append((getFlag(LAST_IN_ROWGROUP) ? "L" : "-")).append("]");
+            sb.append("[");
+            sb.append(row.getIndex()).append("/");
+            sb.append(getFlag(FIRST_IN_ROWGROUP) ? "F" : "-");
+            sb.append(getFlag(LAST_IN_ROWGROUP) ? "L" : "-").append("]");
             sb.append("(");
             sb.append(gridUnitParts);
             sb.append(")");

diff --git a/src/java/org/apache/fop/layoutmgr/table/TableStepper.java b/src/java/org/apache/fop/layoutmgr/table/TableStepper.java
index 68b8cb4c1111e05a2c953af56ed6674433583e27..3aa91e80be4e1a2ed3aee7fd264d537bce8d7ab6 100644 (file)
--- a/src/java/org/apache/fop/layoutmgr/table/TableStepper.java
+++ b/src/java/org/apache/fop/layoutmgr/table/TableStepper.java
@@ -107,11 +107,16 @@ public class TableStepper {
     }
     
     private GridUnit getActiveGridUnit(int column) {
-        return getActiveRow().getGridUnit(column);
+        return getActiveRow().safelyGetGridUnit(column);
     }
     
     private PrimaryGridUnit getActivePrimaryGridUnit(int column) {
-        return getActiveGridUnit(column).getPrimary();
+        GridUnit gu = getActiveGridUnit(column);
+        if (gu == null) {
+            return null;
+        } else {
+            return gu.getPrimary();
+        }
     }
     
     private void calcTotalHeight() {
@@ -163,7 +168,7 @@ public class TableStepper {
     private void setupElementList(int column) {
         GridUnit gu = getActiveGridUnit(column);
         EffRow row = getActiveRow();
-        if (gu.isEmpty()) {
+        if (gu == null || gu.isEmpty()) {
             elementLists[column] = null;
             start[column] = 0;
             end[column] = -1;
@@ -243,6 +248,7 @@ public class TableStepper {
         TableContentPosition lastTCPos = null;
         LinkedList returnList = new LinkedList();
         while ((step = getNextStep(laststep)) >= 0) {
+            int normalRow = activeRow;
             if (rowBacktrackForLastStep) {
                 //Even though we've already switched to the next row, we have to 
                 //calculate as if we were still on the previous row
@@ -257,7 +263,7 @@ public class TableStepper {
             List gridUnitParts = new java.util.ArrayList(maxColumnCount);
             for (int i = 0; i < start.length; i++) {
                 if (end[i] >= start[i]) {
-                    PrimaryGridUnit pgu = getActivePrimaryGridUnit(i);
+                    PrimaryGridUnit pgu = rowGroup[startRow[i]].getGridUnit(i).getPrimary();
                     if (start[i] == 0 && end[i] == 0 
                             && elementLists[i].size() == 1
                             && elementLists[i].get(0) instanceof KnuthBoxCellWithBPD) {
@@ -298,12 +304,15 @@ public class TableStepper {
             //Create elements for step
             int effPenaltyLen = penaltyLen;
             TableContentPosition tcpos = new TableContentPosition(getTableLM(), 
-                    gridUnitParts, getActiveRow());
+                    gridUnitParts, rowGroup[normalRow]);
             if (returnList.size() == 0) {
                 tcpos.setFlag(TableContentPosition.FIRST_IN_ROWGROUP, true);
             }
             lastTCPos = tcpos;
-            log.debug(" - " + rowBacktrackForLastStep + " - " + activeRow + " - " + tcpos);
+            if (log.isDebugEnabled()) {
+                log.debug(" - backtrack=" + rowBacktrackForLastStep 
+                        + " - row=" + activeRow + " - " + tcpos);
+            }
             returnList.add(new KnuthBox(boxLen, tcpos, false));
             TableHFPenaltyPosition penaltyPos = new TableHFPenaltyPosition(getTableLM());
             if (bodyType == TableRowIterator.BODY) {
@@ -409,7 +418,9 @@ public class TableStepper {
                             + "in progress (See XSL 1.0, 7.19.1)");
                 }
                 activeRow++;
-                log.debug("===> new row: " + activeRow);
+                if (log.isDebugEnabled()) {
+                    log.debug("===> new row: " + activeRow);
+                }
                 initializeElementLists();
                 for (int i = 0; i < backupWidths.length; i++) {
                     if (end[i] < 0) {
@@ -479,8 +490,10 @@ public class TableStepper {
                     borderAfter[i] = getActivePrimaryGridUnit(i).getHalfMaxAfterBorderWidth();
                 }
             }
-            log.debug("borders before=" + borderBefore[i] + " after=" + borderAfter[i]);
-            log.debug("padding before=" + paddingBefore[i] + " after=" + paddingAfter[i]);
+            if (log.isTraceEnabled()) {
+                log.trace("borders before=" + borderBefore[i] + " after=" + borderAfter[i]);
+                log.trace("padding before=" + paddingBefore[i] + " after=" + paddingAfter[i]);
+            }
         }
         if (seqCount == 0) {
             return -1;

diff --git a/test/layoutengine/disabled-testcases.txt b/test/layoutengine/disabled-testcases.txt
index 156f1da7de4186804077322397c6198c5523ed1d..86ba9003144d7e741c752e07b1b63ce051949f64 100644 (file)
--- a/test/layoutengine/disabled-testcases.txt
+++ b/test/layoutengine/disabled-testcases.txt
@@ -34,7 +34,6 @@ table-body_background-image.xml
 table_border-collapse_collapse_1.xml
 table_border-collapse_collapse_2.xml
 table_border_padding.xml
-table_bug37270.xml
 table-cell_block_keep-with-previous.xml
 table-cell_border_padding_conditionality.xml
 table-column_first-row-width.xml

diff --git a/test/layoutengine/testcases/table_bug37270.xml b/test/layoutengine/testcases/table_bug37270.xml
index 56d87e7aac943750c749f3f01ec0e3d6250c9c56..c3d8cf6ae316e6f394a9bbd9845534a56d9ebae5 100644 (file)
--- a/test/layoutengine/testcases/table_bug37270.xml
+++ b/test/layoutengine/testcases/table_bug37270.xml
@@ -38,19 +38,30 @@
             <fo:table-body>
               <fo:table-row>
                 <!-- Note: the padding-top caused a NullPointerException -->
-                <fo:table-cell padding-top="1.2cm" number-rows-spanned="2">
+                <fo:table-cell padding-top="40pt" number-rows-spanned="3" background-color="yellow" id="left">
                   <fo:block>row 1 col 1</fo:block>
                 </fo:table-cell>
                 <!-- absent table-cells in first row -->
+                <!--fo:table-cell column-number="4">
+                  <fo:block>xxxxx</fo:block>
+                </fo:table-cell-->
               </fo:table-row>
-              <fo:table-row>
-                <fo:table-cell number-columns-spanned="2">
+              <fo:table-row background-color="orange">
+                <fo:table-cell number-columns-spanned="2" id="r2c2">
                   <fo:block>row 2 col 2</fo:block>
                 </fo:table-cell>
-                <fo:table-cell>
+                <fo:table-cell id="r2c4">
                   <fo:block>Row 2 col 4</fo:block>
                 </fo:table-cell>
               </fo:table-row>
+              <fo:table-row>
+                <fo:table-cell number-columns-spanned="2" id="r3c2">
+                  <fo:block>row 3 col 2</fo:block>
+                </fo:table-cell>
+                <fo:table-cell id="r3c4">
+                  <fo:block>Row 3 col 4</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
             </fo:table-body>
           </fo:table>
         </fo:flow>
@@ -59,6 +70,20 @@
   </fo>
   <checks>
     <!-- Simply check that FOP doesn't fail with an IndexOutOfBoundsException or an NPE -->
-    <eval expected="1" xpath="count(//pageViewport)"/>
+    <element-list category="breaker">
+      <box w="0"/>
+      <penalty w="14400"/> <!-- p > 0 && p <= INF -->
+      <box w="14400"/>
+      <penalty w="14400"/> <!-- p > 0 && p <= INF -->
+      <box w="40000"/>
+      <skip>3</skip>
+    </element-list>
+    
+    <!-- Checking vertical position here. I had trouble with this while debugging. -->
+    <true xpath="not(boolean(//block[@prod-id='left']/@top-offset))"/>
+    <true xpath="not(boolean(//block[@prod-id='r2c2']/@top-offset))"/>
+    <true xpath="not(boolean(//block[@prod-id='r2c4']/@top-offset))"/>
+    <eval expected="14400" xpath="//block[@prod-id='r3c2']/@top-offset"/>
+    <eval expected="14400" xpath="//block[@prod-id='r3c4']/@top-offset"/>
   </checks>
 </testcase>