From: Vincent Hennebert Date: Mon, 10 Mar 2008 20:52:43 +0000 (+0000) Subject: Bugfix: forced break ignored when the minimum height of a table-row isn't reached X-Git-Tag: fop-0_95beta~14 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b7538be4dcd299f90ad3525fd24758614d10352e;p=xmlgraphics-fop.git Bugfix: forced break ignored when the minimum height of a table-row isn't reached git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@635686 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java b/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java index 297a3176c..8af896fa2 100644 --- a/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java +++ b/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java @@ -238,18 +238,6 @@ public class PrimaryGridUnit extends GridUnit { return contentLength; } - /** @return true if cell/row has an explicit BPD/height */ - public boolean hasBPD() { - if (!getCell().getBlockProgressionDimension().getOptimum(null).isAuto()) { - return true; - } - if (getRow() != null - && !getRow().getBlockProgressionDimension().getOptimum(null).isAuto()) { - return true; - } - return false; - } - /** * Returns the grid units belonging to the same span as this one. * diff --git a/src/java/org/apache/fop/layoutmgr/MinOptMaxUtil.java b/src/java/org/apache/fop/layoutmgr/MinOptMaxUtil.java index ff9bdb1d9..b58af1cfe 100644 --- a/src/java/org/apache/fop/layoutmgr/MinOptMaxUtil.java +++ b/src/java/org/apache/fop/layoutmgr/MinOptMaxUtil.java @@ -68,19 +68,16 @@ public class MinOptMaxUtil { } /** - * Extend the minimum length to the given length. + * Extends the minimum length to the given length if necessary, and adjusts opt and + * max accordingly. + * * @param mom the min/opt/max trait * @param len the new minimum length - * @param optToLen if set adjusts the optimum length to be the smaller of the - * minimum length and the given length */ - public static void extendMinimum(MinOptMax mom, int len, boolean optToLen) { + public static void extendMinimum(MinOptMax mom, int len) { if (mom.min < len) { mom.min = len; mom.opt = Math.max(mom.min, mom.opt); - if (optToLen) { - mom.opt = Math.min(mom.min, len); - } mom.max = Math.max(mom.opt, mom.max); } } @@ -111,7 +108,7 @@ public class MinOptMaxUtil { ? 0 : prop.getMinimum(context).getLength().getValue(context)), (prop.getOptimum(context).isAuto() ? 0 : prop.getOptimum(context).getLength().getValue(context)), - (prop.getMinimum(context).isAuto() + (prop.getMaximum(context).isAuto() ? Integer.MAX_VALUE : prop.getMaximum(context).getLength().getValue(context))); return mom; diff --git a/src/java/org/apache/fop/layoutmgr/table/ActiveCell.java b/src/java/org/apache/fop/layoutmgr/table/ActiveCell.java index 46544700b..4e41e909f 100644 --- a/src/java/org/apache/fop/layoutmgr/table/ActiveCell.java +++ b/src/java/org/apache/fop/layoutmgr/table/ActiveCell.java @@ -33,6 +33,8 @@ import org.apache.fop.layoutmgr.ElementListUtils; import org.apache.fop.layoutmgr.KnuthBox; import org.apache.fop.layoutmgr.KnuthElement; import org.apache.fop.layoutmgr.KnuthPenalty; +import org.apache.fop.layoutmgr.MinOptMaxUtil; +import org.apache.fop.traits.MinOptMax; /** * A cell playing in the construction of steps for a row-group. @@ -133,6 +135,49 @@ class ActiveCell { } } + // TODO to be removed along with the RowPainter#computeContentLength method + /** See {@link ActiveCell#handleExplicitHeight(MinOptMax, MinOptMax)}. */ + private static class FillerPenalty extends KnuthPenalty { + + private int contentLength; + + FillerPenalty(KnuthPenalty p, int length) { + super(length, p.getP(), p.isFlagged(), p.getBreakClass(), + p.getPosition(), p.isAuxiliary()); + contentLength = p.getW(); + } + + FillerPenalty(int length) { + super(length, 0, false, null, true); + contentLength = 0; + } + } + + /** See {@link ActiveCell#handleExplicitHeight(MinOptMax, MinOptMax)}. */ + private static class FillerBox extends KnuthBox { + FillerBox(int length) { + super(length, null, true); + } + } + + /** + * Returns the actual length of the content represented by the given element. In the + * case where this element is used as a filler to match a row's fixed height, the + * value returned by the getW() method will be higher than the actual content. + * + * @param el an element + * @return the actual content length corresponding to the element + */ + static int getElementContentLength(KnuthElement el) { + if (el instanceof FillerPenalty) { + return ((FillerPenalty) el).contentLength; + } else if (el instanceof FillerBox) { + return 0; + } else { + return el.getW(); + } + } + ActiveCell(PrimaryGridUnit pgu, EffRow row, int rowIndex, int previousRowsLength, TableLayoutManager tableLM) { this.pgu = pgu; @@ -149,22 +194,10 @@ class ActiveCell { + pgu.getBeforeBorderWidth(0, ConditionalBorder.REST); bpAfterNormal = paddingAfterNormal + pgu.getAfterBorderWidth(ConditionalBorder.NORMAL); bpAfterTrailing = paddingAfterTrailing + pgu.getAfterBorderWidth(0, ConditionalBorder.REST); - boolean makeBoxForWholeRow = false; - if (row.getExplicitHeight().min > 0) { - boolean contentsSmaller = ElementListUtils.removeLegalBreaks( - pgu.getElements(), row.getExplicitHeight()); - if (contentsSmaller) { - makeBoxForWholeRow = true; - } - } - if (makeBoxForWholeRow) { - elementList = new java.util.ArrayList(1); - int height = row.getHeight().opt; - height -= bpBeforeNormal + bpAfterNormal; - elementList.add(new KnuthBoxCellWithBPD(height)); - } else { - elementList = pgu.getElements(); - } + elementList = pgu.getElements(); + handleExplicitHeight( + MinOptMaxUtil.toMinOptMax(pgu.getCell().getBlockProgressionDimension(), tableLM), + row.getExplicitHeight()); knuthIter = elementList.listIterator(); includedLength = -1; // Avoid troubles with cells having content of zero length totalLength = previousRowsLength + ElementListUtils.calcContentLength(elementList); @@ -181,6 +214,46 @@ class ActiveCell { } } + /** + * Modifies the cell's element list by putting filler elements, so that the cell's or + * row's explicit height is always reached. + * + * TODO this will work properly only for the first break. Then the limitation + * explained on http://wiki.apache.org/xmlgraphics-fop/TableLayout/KnownProblems + * occurs. The list of elements needs to be re-adjusted after each break. + */ + private void handleExplicitHeight(MinOptMax cellBPD, MinOptMax rowBPD) { + int minBPD = Math.max(cellBPD.min, rowBPD.min); + if (minBPD > 0) { + ListIterator iter = elementList.listIterator(); + int cumulateLength = 0; + boolean prevIsBox = false; + while (iter.hasNext() && cumulateLength < minBPD) { + KnuthElement el = (KnuthElement) iter.next(); + if (el.isBox()) { + prevIsBox = true; + cumulateLength += el.getW(); + } else if (el.isGlue()) { + if (prevIsBox) { + elementList.add(iter.nextIndex() - 1, + new FillerPenalty(minBPD - cumulateLength)); + } + prevIsBox = false; + cumulateLength += el.getW(); + } else { + prevIsBox = false; + if (cumulateLength + el.getW() < minBPD) { + iter.set(new FillerPenalty((KnuthPenalty) el, minBPD - cumulateLength)); + } + } + } + } + int optBPD = Math.max(minBPD, Math.max(cellBPD.opt, rowBPD.opt)); + if (pgu.getContentLength() < optBPD) { + elementList.add(new FillerBox(optBPD - pgu.getContentLength())); + } + } + PrimaryGridUnit getPrimaryGridUnit() { return pgu; } @@ -444,13 +517,6 @@ class ActiveCell { return new CellPart(pgu, nextStep.start, previousStep.end, lastCellPart, 0, 0, previousStep.penaltyLength, bpBeforeNormal, bpBeforeFirst, bpAfterNormal, bpAfterTrailing); - } else if (nextStep.start == 0 && nextStep.end == 0 - && elementList.size() == 1 - && elementList.get(0) instanceof KnuthBoxCellWithBPD) { - //Special case: Cell with fixed BPD - return new CellPart(pgu, 0, pgu.getElements().size() - 1, lastCellPart, - nextStep.condBeforeContentLength, length, nextStep.penaltyLength, - bpBeforeNormal, bpBeforeFirst, bpAfterNormal, bpAfterTrailing); } else { return new CellPart(pgu, nextStep.start, nextStep.end, lastCellPart, nextStep.condBeforeContentLength, length, nextStep.penaltyLength, @@ -467,15 +533,4 @@ class ActiveCell { public String toString() { return "Cell " + (pgu.getRowIndex() + 1) + "." + (pgu.getColIndex() + 1); } - - - /** - * Marker class denoting table cells fitting in just one box (no legal break inside). - */ - private static class KnuthBoxCellWithBPD extends KnuthBox { - - public KnuthBoxCellWithBPD(int w) { - super(w, null, true); - } - } } diff --git a/src/java/org/apache/fop/layoutmgr/table/RowGroupLayoutManager.java b/src/java/org/apache/fop/layoutmgr/table/RowGroupLayoutManager.java index 917e8296b..9c97ca827 100644 --- a/src/java/org/apache/fop/layoutmgr/table/RowGroupLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/table/RowGroupLayoutManager.java @@ -19,17 +19,16 @@ package org.apache.fop.layoutmgr.table; +import java.util.Iterator; import java.util.LinkedList; -import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.apache.fop.fo.Constants; -import org.apache.fop.fo.FONode; import org.apache.fop.fo.flow.table.EffRow; import org.apache.fop.fo.flow.table.GridUnit; import org.apache.fop.fo.flow.table.PrimaryGridUnit; +import org.apache.fop.fo.flow.table.TableColumn; import org.apache.fop.fo.flow.table.TableRow; import org.apache.fop.fo.properties.CommonBorderPaddingBackground; import org.apache.fop.fo.properties.LengthRangeProperty; @@ -95,137 +94,119 @@ class RowGroupLayoutManager { private void createElementsForRowGroup(LayoutContext context, int alignment, int bodyType, LinkedList returnList) { log.debug("Handling row group with " + rowGroup.length + " rows..."); + EffRow row; + for (int rgi = 0; rgi < rowGroup.length; rgi++) { + row = rowGroup[rgi]; + for (Iterator iter = row.getGridUnits().iterator(); iter.hasNext();) { + GridUnit gu = (GridUnit) iter.next(); + if (gu.isPrimary()) { + PrimaryGridUnit primary = gu.getPrimary(); + // TODO a new LM must be created for every new static-content + primary.createCellLM(); + primary.getCellLM().setParent(tableLM); + //Calculate width of cell + int spanWidth = 0; + Iterator colIter = tableLM.getTable().getColumns().listIterator( + primary.getColIndex()); + for (int i = 0, c = primary.getCell().getNumberColumnsSpanned(); i < c; i++) { + spanWidth += ((TableColumn) colIter.next()).getColumnWidth().getValue( + tableLM); + } + LayoutContext childLC = new LayoutContext(0); + childLC.setStackLimitBP(context.getStackLimitBP()); //necessary? + childLC.setRefIPD(spanWidth); + + //Get the element list for the cell contents + LinkedList elems = primary.getCellLM().getNextKnuthElements( + childLC, alignment); + ElementListObserver.observe(elems, "table-cell", primary.getCell().getId()); + primary.setElements(elems); + } + } + } + computeRowHeights(); + LinkedList elements = tableStepper.getCombinedKnuthElementsForRowGroup(context, + rowGroup, bodyType); + returnList.addAll(elements); + } + + /** + * Calculate the heights of the rows in the row group, see CSS21, 17.5.3 Table height + * algorithms. + * + * TODO this method will need to be adapted once clarification has been made by the + * W3C regarding whether borders or border-separation must be included or not + */ + private void computeRowHeights() { + log.debug("rowGroup:"); MinOptMax[] rowHeights = new MinOptMax[rowGroup.length]; - MinOptMax[] explicitRowHeights = new MinOptMax[rowGroup.length]; EffRow row; - List pgus = new java.util.ArrayList(); //holds a list of a row's primary grid units for (int rgi = 0; rgi < rowGroup.length; rgi++) { row = rowGroup[rgi]; - rowHeights[rgi] = new MinOptMax(0, 0, Integer.MAX_VALUE); - explicitRowHeights[rgi] = new MinOptMax(0, 0, Integer.MAX_VALUE); - - pgus.clear(); - TableRow tableRow = null; - // The row's minimum content height; 0 if the row's height is auto, otherwise - // the .minimum component of the explicitly specified value - int minRowBPD = 0; // The BPD of the biggest cell in the row - int maxCellBPD = 0; - for (int j = 0; j < row.getGridUnits().size(); j++) { - GridUnit gu = row.getGridUnit(j); - if ((gu.isPrimary() || (gu.getColSpanIndex() == 0 && gu.isLastGridUnitRowSpan())) - && !gu.isEmpty()) { +// int maxCellBPD = 0; + MinOptMax explicitRowHeight; + TableRow tableRowFO = rowGroup[rgi].getTableRow(); + if (tableRowFO == null) { + rowHeights[rgi] = new MinOptMax(0, 0, Integer.MAX_VALUE); + explicitRowHeight = new MinOptMax(0, 0, Integer.MAX_VALUE); + } else { + LengthRangeProperty rowBPD = tableRowFO.getBlockProgressionDimension(); + rowHeights[rgi] = MinOptMaxUtil.toMinOptMax(rowBPD, tableLM); + explicitRowHeight = MinOptMaxUtil.toMinOptMax(rowBPD, tableLM); + } + for (Iterator iter = row.getGridUnits().iterator(); iter.hasNext();) { + GridUnit gu = (GridUnit) iter.next(); + if (!gu.isEmpty() && gu.getColSpanIndex() == 0 && gu.isLastGridUnitRowSpan()) { PrimaryGridUnit primary = gu.getPrimary(); - - if (gu.isPrimary()) { - // TODO a new LM must be created for every new static-content - primary.createCellLM(); - primary.getCellLM().setParent(tableLM); - - //Determine the table-row if any - if (tableRow == null && primary.getRow() != null) { - tableRow = primary.getRow(); - - //Check for bpd on row, see CSS21, 17.5.3 Table height algorithms - LengthRangeProperty rowBPD = tableRow.getBlockProgressionDimension(); - if (!rowBPD.getMinimum(tableLM).isAuto()) { - minRowBPD = Math.max(minRowBPD, - rowBPD.getMinimum(tableLM).getLength().getValue(tableLM)); - } - MinOptMaxUtil.restrict(explicitRowHeights[rgi], rowBPD, tableLM); - - } - - //Calculate width of cell - int spanWidth = 0; - for (int i = primary.getColIndex(); - i < primary.getColIndex() - + primary.getCell().getNumberColumnsSpanned(); - i++) { - if (tableLM.getColumns().getColumn(i + 1) != null) { - spanWidth += tableLM.getColumns().getColumn(i + 1) - .getColumnWidth().getValue(tableLM); - } - } - LayoutContext childLC = new LayoutContext(0); - childLC.setStackLimitBP(context.getStackLimitBP()); //necessary? - childLC.setRefIPD(spanWidth); - - //Get the element list for the cell contents - LinkedList elems = primary.getCellLM().getNextKnuthElements( - childLC, alignment); - ElementListObserver.observe(elems, "table-cell", primary.getCell().getId()); - primary.setElements(elems); + int effectiveCellBPD = 0; + LengthRangeProperty cellBPD = primary.getCell().getBlockProgressionDimension(); + if (!cellBPD.getMinimum(tableLM).isAuto()) { + effectiveCellBPD = cellBPD.getMinimum(tableLM).getLength() + .getValue(tableLM); } - - //Calculate height of row, see CSS21, 17.5.3 Table height algorithms - if (gu.isLastGridUnitRowSpan()) { - // The effective cell's bpd, after taking into account bpd - // (possibly explicitly) set on the row or on the cell, and the - // cell's content length - int effectiveCellBPD = minRowBPD; - LengthRangeProperty cellBPD = primary.getCell() - .getBlockProgressionDimension(); - if (!cellBPD.getMinimum(tableLM).isAuto()) { - effectiveCellBPD = Math.max(effectiveCellBPD, - cellBPD.getMinimum(tableLM).getLength().getValue(tableLM)); - } - if (!cellBPD.getOptimum(tableLM).isAuto()) { - effectiveCellBPD = Math.max(effectiveCellBPD, - cellBPD.getOptimum(tableLM).getLength().getValue(tableLM)); - } - if (gu.getRowSpanIndex() == 0) { - //TODO ATM only non-row-spanned cells are taken for this - MinOptMaxUtil.restrict(explicitRowHeights[rgi], cellBPD, tableLM); - } - effectiveCellBPD = Math.max(effectiveCellBPD, - primary.getContentLength()); - - int borderWidths = primary.getBeforeAfterBorderWidth(); - int padding = 0; - maxCellBPD = Math.max(maxCellBPD, effectiveCellBPD); - CommonBorderPaddingBackground cbpb - = primary.getCell().getCommonBorderPaddingBackground(); - padding += cbpb.getPaddingBefore(false, primary.getCellLM()); - padding += cbpb.getPaddingAfter(false, primary.getCellLM()); - int effRowHeight = effectiveCellBPD - + padding + borderWidths; - for (int previous = 0; previous < gu.getRowSpanIndex(); previous++) { - effRowHeight -= rowHeights[rgi - previous - 1].opt; - } - if (effRowHeight > rowHeights[rgi].min) { - //This is the new height of the (grid) row - MinOptMaxUtil.extendMinimum(rowHeights[rgi], effRowHeight, false); - } + if (!cellBPD.getOptimum(tableLM).isAuto()) { + effectiveCellBPD = cellBPD.getOptimum(tableLM).getLength() + .getValue(tableLM); } - - if (gu.isPrimary()) { - pgus.add(primary); + if (gu.getRowSpanIndex() == 0) { + effectiveCellBPD = Math.max(effectiveCellBPD, explicitRowHeight.opt); + } + effectiveCellBPD = Math.max(effectiveCellBPD, primary.getContentLength()); + int borderWidths = primary.getBeforeAfterBorderWidth(); + int padding = 0; + CommonBorderPaddingBackground cbpb = primary.getCell() + .getCommonBorderPaddingBackground(); + padding += cbpb.getPaddingBefore(false, primary.getCellLM()); + padding += cbpb.getPaddingAfter(false, primary.getCellLM()); + int effRowHeight = effectiveCellBPD + padding + borderWidths; + for (int prev = rgi - 1; prev >= rgi - gu.getRowSpanIndex(); prev--) { + effRowHeight -= rowHeights[prev].opt; + } + if (effRowHeight > rowHeights[rgi].min) { + // This is the new height of the (grid) row + MinOptMaxUtil.extendMinimum(rowHeights[rgi], effRowHeight); } } } row.setHeight(rowHeights[rgi]); - row.setExplicitHeight(explicitRowHeights[rgi]); - if (maxCellBPD > row.getExplicitHeight().max) { - log.warn(FONode.decorateWithContextInfo( - "The contents of row " + (row.getIndex() + 1) - + " are taller than they should be (there is a" - + " block-progression-dimension or height constraint on the indicated row)." - + " Due to its contents the row grows" - + " to " + maxCellBPD + " millipoints, but the row shouldn't get" - + " any taller than " + row.getExplicitHeight() + " millipoints.", - row.getTableRow())); - } - } - if (log.isDebugEnabled()) { - log.debug("rowGroup:"); - for (int i = 0; i < rowHeights.length; i++) { - log.debug(" height=" + rowHeights[i] + " explicit=" + explicitRowHeights[i]); + row.setExplicitHeight(explicitRowHeight); + // TODO re-enable and improve after clarification +// if (maxCellBPD > row.getExplicitHeight().max) { +// log.warn(FONode.decorateWithContextInfo( +// "The contents of row " + (row.getIndex() + 1) +// + " are taller than they should be (there is a" +// + " block-progression-dimension or height constraint +// + " on the indicated row)." +// + " Due to its contents the row grows" +// + " to " + maxCellBPD + " millipoints, but the row shouldn't get" +// + " any taller than " + row.getExplicitHeight() + " millipoints.", +// row.getTableRow())); +// } + if (log.isDebugEnabled()) { + log.debug(" height=" + rowHeights[rgi] + " explicit=" + explicitRowHeight); } } - LinkedList elements = tableStepper.getCombinedKnuthElementsForRowGroup(context, - rowGroup, bodyType); - returnList.addAll(elements); } } diff --git a/src/java/org/apache/fop/layoutmgr/table/RowPainter.java b/src/java/org/apache/fop/layoutmgr/table/RowPainter.java index 61f6b3aec..bed9c53ae 100644 --- a/src/java/org/apache/fop/layoutmgr/table/RowPainter.java +++ b/src/java/org/apache/fop/layoutmgr/table/RowPainter.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -298,18 +299,22 @@ class RowPainter { // cell, in most cases) return 0; } else { - int actualStart = startIndex; + ListIterator iter = pgu.getElements().listIterator(startIndex); // Skip from the content length calculation glues and penalties occurring at the // beginning of the page - while (actualStart <= endIndex - && !((KnuthElement) pgu.getElements().get(actualStart)).isBox()) { - actualStart++; + boolean nextIsBox = false; + while (iter.nextIndex() <= endIndex && !nextIsBox) { + nextIsBox = ((KnuthElement) iter.next()).isBox(); } - int len = ElementListUtils.calcContentLength( - pgu.getElements(), actualStart, endIndex); - KnuthElement el = (KnuthElement)pgu.getElements().get(endIndex); - if (el.isPenalty()) { - len += el.getW(); + int len = 0; + if (((KnuthElement) iter.previous()).isBox()) { + while (iter.nextIndex() < endIndex) { + KnuthElement el = (KnuthElement) iter.next(); + if (el.isBox() || el.isGlue()) { + len += el.getW(); + } + } + len += ActiveCell.getElementContentLength((KnuthElement) iter.next()); } return len; } diff --git a/status.xml b/status.xml index 34307f721..1a01f73c2 100644 --- a/status.xml +++ b/status.xml @@ -94,6 +94,10 @@

+ + Bugfix: a forced break inside a cell was ignored when occurring before the minimum height + set on the enclosing row was set. + Fixed exceptions when lists, tables or block-container are children of an inline-level FO. diff --git a/test/layoutengine/standard-testcases/table_border-collapse_separate_1.xml b/test/layoutengine/standard-testcases/table_border-collapse_separate_1.xml index d0b11c1b0..8a9370a5c 100644 --- a/test/layoutengine/standard-testcases/table_border-collapse_separate_1.xml +++ b/test/layoutengine/standard-testcases/table_border-collapse_separate_1.xml @@ -31,7 +31,7 @@ - + diff --git a/test/layoutengine/standard-testcases/table_row-delay_fixed-row-height.xml b/test/layoutengine/standard-testcases/table_row-delay_fixed-row-height.xml index 70990de62..7b50996cc 100644 --- a/test/layoutengine/standard-testcases/table_row-delay_fixed-row-height.xml +++ b/test/layoutengine/standard-testcases/table_row-delay_fixed-row-height.xml @@ -144,9 +144,9 @@ 3 - - - + + +