You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by je...@apache.org on 2008/08/29 18:36:19 UTC
svn commit: r690319 - in /xmlgraphics/fop/trunk: ./
src/java/org/apache/fop/layoutmgr/table/
test/layoutengine/standard-testcases/
Author: jeremias
Date: Fri Aug 29 09:36:17 2008
New Revision: 690319
URL: http://svn.apache.org/viewvc?rev=690319&view=rev
Log:
Added missing generation of areas for empty grid units in tables with collapsing border model.
Modified:
xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/RowPainter.java
xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
xmlgraphics/fop/trunk/status.xml
xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2.xml
xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2_no-col.xml
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/RowPainter.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/RowPainter.java?rev=690319&r1=690318&r2=690319&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/RowPainter.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/RowPainter.java Fri Aug 29 09:36:17 2008
@@ -27,13 +27,19 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+
import org.apache.fop.area.Block;
+import org.apache.fop.area.Trait;
import org.apache.fop.fo.flow.table.ConditionalBorder;
import org.apache.fop.fo.flow.table.EffRow;
+import org.apache.fop.fo.flow.table.EmptyGridUnit;
import org.apache.fop.fo.flow.table.GridUnit;
import org.apache.fop.fo.flow.table.PrimaryGridUnit;
+import org.apache.fop.fo.flow.table.Table;
+import org.apache.fop.fo.flow.table.TableColumn;
import org.apache.fop.fo.flow.table.TablePart;
import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
+import org.apache.fop.fo.properties.CommonBorderPaddingBackground.BorderInfo;
import org.apache.fop.layoutmgr.ElementListUtils;
import org.apache.fop.layoutmgr.KnuthElement;
import org.apache.fop.layoutmgr.KnuthPossPosIter;
@@ -202,10 +208,16 @@
recordRowOffset(currentRow.getIndex(), currentRowOffset);
// Need to compute the actual row height first
+ // and determine border behaviour for empty cells
+ boolean firstCellPart = true;
+ boolean lastCellPart = true;
int actualRowHeight = 0;
for (int i = 0; i < colCount; i++) {
GridUnit currentGU = currentRow.getGridUnit(i);
- if (!currentGU.isEmpty() && currentGU.getColSpanIndex() == 0
+ if (currentGU.isEmpty()) {
+ continue;
+ }
+ if (currentGU.getColSpanIndex() == 0
&& (lastInPart || currentGU.isLastGridUnitRowSpan())
&& firstCellParts[i] != null) {
// TODO
@@ -225,21 +237,53 @@
actualRowHeight = Math.max(actualRowHeight, cellOffset + cellHeight
- currentRowOffset);
}
+
+ if (firstCellParts[i] != null && !firstCellParts[i].isFirstPart()) {
+ firstCellPart = false;
+ }
+ if (lastCellParts[i] != null && !lastCellParts[i].isLastPart()) {
+ lastCellPart = false;
+ }
}
// Then add areas for cells finishing on the current row
for (int i = 0; i < colCount; i++) {
GridUnit currentGU = currentRow.getGridUnit(i);
- if (currentGU.isEmpty()) {
- // TODO remove once missing cells are properly implemented (i.e., replaced
- // by an fo:table-cell element containing an empty fo:block)
+ if (currentGU.isEmpty() && !tclm.isSeparateBorderModel()) {
+ int borderBeforeWhich;
+ if (firstCellPart) {
+ if (firstCellOnPage[i]) {
+ borderBeforeWhich = ConditionalBorder.LEADING_TRAILING;
+ } else {
+ borderBeforeWhich = ConditionalBorder.NORMAL;
+ }
+ } else {
+ borderBeforeWhich = ConditionalBorder.REST;
+ }
+ int borderAfterWhich;
+ if (lastCellPart) {
+ if (lastInPart) {
+ borderAfterWhich = ConditionalBorder.LEADING_TRAILING;
+ } else {
+ borderAfterWhich = ConditionalBorder.NORMAL;
+ }
+ } else {
+ borderAfterWhich = ConditionalBorder.REST;
+ }
+ addAreaForEmptyGridUnit((EmptyGridUnit)currentGU,
+ currentRow.getIndex(), i,
+ actualRowHeight,
+ borderBeforeWhich, borderAfterWhich,
+ lastOnPage);
+
firstCellOnPage[i] = false;
} else if (currentGU.getColSpanIndex() == 0
&& (lastInPart || currentGU.isLastGridUnitRowSpan())
&& firstCellParts[i] != null) {
assert firstCellParts[i].pgu == currentGU.getPrimary();
+
int borderBeforeWhich;
- if (firstCellParts[i].start == 0) {
+ if (firstCellParts[i].isFirstPart()) {
if (firstCellOnPage[i]) {
borderBeforeWhich = ConditionalBorder.LEADING_TRAILING;
} else {
@@ -259,6 +303,7 @@
} else {
borderAfterWhich = ConditionalBorder.REST;
}
+
addAreasForCell(firstCellParts[i].pgu,
firstCellParts[i].start, lastCellParts[i].end,
actualRowHeight, borderBeforeWhich, borderAfterWhich,
@@ -387,6 +432,56 @@
startRowIndex == firstRowOnPageIndex, lastOnPage, this, firstRowHeight);
}
+ private void addAreaForEmptyGridUnit(EmptyGridUnit gu, int rowIndex, int colIndex,
+ int actualRowHeight,
+ int borderBeforeWhich, int borderAfterWhich, boolean lastOnPage) {
+
+ //get effective borders
+ BorderInfo borderBefore = gu.getBorderBefore(borderBeforeWhich);
+ BorderInfo borderAfter = gu.getBorderAfter(borderAfterWhich);
+ BorderInfo borderStart = gu.getBorderStart();
+ BorderInfo borderEnd = gu.getBorderEnd();
+ if (borderBefore.getRetainedWidth() == 0
+ && borderAfter.getRetainedWidth() == 0
+ && borderStart.getRetainedWidth() == 0
+ && borderEnd.getRetainedWidth() == 0) {
+ return; //no borders, no area necessary
+ }
+
+ TableLayoutManager tableLM = tclm.getTableLM();
+ Table table = tableLM.getTable();
+ TableColumn col = tclm.getColumns().getColumn(colIndex + 1);
+
+ //position information
+ boolean firstOnPage = (rowIndex == firstRowOnPageIndex);
+ boolean inFirstColumn = (colIndex == 0);
+ boolean inLastColumn = (colIndex == table.getNumberOfColumns() - 1);
+
+ //determine the block area's size
+ int ipd = col.getColumnWidth().getValue(tableLM);
+ ipd -= (borderStart.getRetainedWidth() + borderEnd.getRetainedWidth()) / 2;
+ int bpd = actualRowHeight;
+ bpd -= (borderBefore.getRetainedWidth() + borderAfter.getRetainedWidth()) / 2;
+
+ //generate the block area
+ Block block = new Block();
+ block.setPositioning(Block.ABSOLUTE);
+ block.addTrait(Trait.IS_REFERENCE_AREA, Boolean.TRUE);
+ block.setIPD(ipd);
+ block.setBPD(bpd);
+ block.setXOffset(tclm.getXOffsetOfGridUnit(colIndex)
+ + (borderStart.getRetainedWidth() / 2));
+ block.setYOffset(getRowOffset(rowIndex)
+ - (borderBefore.getRetainedWidth() / 2));
+ boolean[] outer = new boolean[] {firstOnPage, lastOnPage, inFirstColumn,
+ inLastColumn};
+ TraitSetter.addCollapsingBorders(block,
+ borderBefore,
+ borderAfter,
+ borderStart,
+ borderEnd, outer);
+ tableLM.addChildArea(block);
+ }
/**
* Registers the given area, that will be used to render the part of
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java?rev=690319&r1=690318&r2=690319&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java Fri Aug 29 09:36:17 2008
@@ -305,13 +305,21 @@
}
/**
- * Retuns the X offset of the given grid unit.
+ * Returns the X offset of the given grid unit.
* @param gu the grid unit
* @return the requested X offset
*/
protected int getXOffsetOfGridUnit(PrimaryGridUnit gu) {
- int col = gu.getColIndex();
- return startXOffset + getTableLM().getColumns().getXOffset(col + 1, getTableLM());
+ return getXOffsetOfGridUnit(gu.getColIndex());
+ }
+
+ /**
+ * Returns the X offset of the grid unit in the given column.
+ * @param colIndex the column index (zero-based)
+ * @return the requested X offset
+ */
+ protected int getXOffsetOfGridUnit(int colIndex) {
+ return startXOffset + getTableLM().getColumns().getXOffset(colIndex + 1, getTableLM());
}
/**
Modified: xmlgraphics/fop/trunk/status.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=690319&r1=690318&r2=690319&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/status.xml (original)
+++ xmlgraphics/fop/trunk/status.xml Fri Aug 29 09:36:17 2008
@@ -53,6 +53,10 @@
<changes>
<release version="FOP Trunk" date="TBD">
+ <action context="Layout" dev="JM" type="add">
+ Added missing generation of areas for empty grid units in tables with collapsing border
+ model.
+ </action>
<action context="Code" dev="JM" type="fix" importance="high">
Fixed memory leak in property cache (not cleaning stale PropertyCache$CacheEntry instances).
</action>
Modified: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2.xml?rev=690319&r1=690318&r2=690319&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2.xml (original)
+++ xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2.xml Fri Aug 29 09:36:17 2008
@@ -434,69 +434,68 @@
<eval expected="(double,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[19]/@border-after"/>
<eval expected="(solid,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[19]/@border-start"/>
<eval expected="(solid,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[19]/@border-end"/>
- <!--
<eval expected="94000" xpath="//flow/block[2]/block[20]/@ipd"/>
<eval expected="106000" xpath="//flow/block[2]/block[20]/@ipda"/>
<eval expected="52200" xpath="//flow/block[2]/block[20]/@bpd"/>
<eval expected="52200" xpath="//flow/block[2]/block[20]/@bpda"/>
<eval expected="103000" xpath="//flow/block[2]/block[20]/@left-offset"/>
<eval expected="241100" xpath="//flow/block[2]/block[20]/@top-offset"/>
- <eval expected="(double,#800000,0,collapse-outer)" xpath="//flow/block[2]/block[20]/@border-after"/>
+ <eval expected="" xpath="//flow/block[2]/block[20]/@border-before"/>
+ <eval expected="" xpath="//flow/block[2]/block[20]/@border-after"/>
<eval expected="(solid,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[20]/@border-start"/>
<eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[20]/@border-end"/>
- -->
- <eval expected="97000" xpath="//flow/block[2]/block[20]/@ipd"/>
- <eval expected="103000" xpath="//flow/block[2]/block[20]/@ipda"/>
- <eval expected="41300" xpath="//flow/block[2]/block[20]/@bpd"/>
- <eval expected="81300" xpath="//flow/block[2]/block[20]/@bpda"/>
- <eval expected="203000" xpath="//flow/block[2]/block[20]/@left-offset"/>
- <eval expected="115400" xpath="//flow/block[2]/block[20]/@top-offset"/>
- <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[20]/@border-before"/>
- <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[20]/@border-start"/>
- <eval expected="97500" xpath="//flow/block[2]/block[21]/@ipd"/>
- <eval expected="102500" xpath="//flow/block[2]/block[21]/@ipda"/>
+ <eval expected="97000" xpath="//flow/block[2]/block[21]/@ipd"/>
+ <eval expected="103000" xpath="//flow/block[2]/block[21]/@ipda"/>
<eval expected="41300" xpath="//flow/block[2]/block[21]/@bpd"/>
<eval expected="81300" xpath="//flow/block[2]/block[21]/@bpda"/>
- <eval expected="300000" xpath="//flow/block[2]/block[21]/@left-offset"/>
+ <eval expected="203000" xpath="//flow/block[2]/block[21]/@left-offset"/>
<eval expected="115400" xpath="//flow/block[2]/block[21]/@top-offset"/>
<eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[21]/@border-before"/>
- <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[21]/@border-end"/>
- <eval expected="70000" xpath="//flow/block[2]/block[22]/@ipd"/>
- <eval expected="130000" xpath="//flow/block[2]/block[22]/@ipda"/>
- <eval expected="44400" xpath="//flow/block[2]/block[22]/@bpd"/>
- <eval expected="44400" xpath="//flow/block[2]/block[22]/@bpda"/>
- <eval expected="230000" xpath="//flow/block[2]/block[22]/@left-offset"/>
- <eval expected="196700" xpath="//flow/block[2]/block[22]/@top-offset"/>
- <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[22]/@border-start"/>
- <eval expected="97500" xpath="//flow/block[2]/block[23]/@ipd"/>
- <eval expected="102500" xpath="//flow/block[2]/block[23]/@ipda"/>
+ <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[21]/@border-start"/>
+ <eval expected="97500" xpath="//flow/block[2]/block[22]/@ipd"/>
+ <eval expected="102500" xpath="//flow/block[2]/block[22]/@ipda"/>
+ <eval expected="41300" xpath="//flow/block[2]/block[22]/@bpd"/>
+ <eval expected="81300" xpath="//flow/block[2]/block[22]/@bpda"/>
+ <eval expected="300000" xpath="//flow/block[2]/block[22]/@left-offset"/>
+ <eval expected="115400" xpath="//flow/block[2]/block[22]/@top-offset"/>
+ <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[22]/@border-before"/>
+ <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[22]/@border-end"/>
+ <eval expected="70000" xpath="//flow/block[2]/block[23]/@ipd"/>
+ <eval expected="130000" xpath="//flow/block[2]/block[23]/@ipda"/>
<eval expected="44400" xpath="//flow/block[2]/block[23]/@bpd"/>
<eval expected="44400" xpath="//flow/block[2]/block[23]/@bpda"/>
- <eval expected="300000" xpath="//flow/block[2]/block[23]/@left-offset"/>
+ <eval expected="230000" xpath="//flow/block[2]/block[23]/@left-offset"/>
<eval expected="196700" xpath="//flow/block[2]/block[23]/@top-offset"/>
- <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[23]/@border-end"/>
- <eval expected="97000" xpath="//flow/block[2]/block[24]/@ipd"/>
- <eval expected="103000" xpath="//flow/block[2]/block[24]/@ipda"/>
- <eval expected="49700" xpath="//flow/block[2]/block[24]/@bpd"/>
- <eval expected="54700" xpath="//flow/block[2]/block[24]/@bpda"/>
- <eval expected="203000" xpath="//flow/block[2]/block[24]/@left-offset"/>
- <eval expected="241100" xpath="//flow/block[2]/block[24]/@top-offset"/>
- <eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[24]/@border-after"/>
- <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[24]/@border-start"/>
- <eval expected="97500" xpath="//flow/block[2]/block[25]/@ipd"/>
- <eval expected="102500" xpath="//flow/block[2]/block[25]/@ipda"/>
+ <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[23]/@border-start"/>
+ <eval expected="97500" xpath="//flow/block[2]/block[24]/@ipd"/>
+ <eval expected="102500" xpath="//flow/block[2]/block[24]/@ipda"/>
+ <eval expected="44400" xpath="//flow/block[2]/block[24]/@bpd"/>
+ <eval expected="44400" xpath="//flow/block[2]/block[24]/@bpda"/>
+ <eval expected="300000" xpath="//flow/block[2]/block[24]/@left-offset"/>
+ <eval expected="196700" xpath="//flow/block[2]/block[24]/@top-offset"/>
+ <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[24]/@border-end"/>
+ <eval expected="97000" xpath="//flow/block[2]/block[25]/@ipd"/>
+ <eval expected="103000" xpath="//flow/block[2]/block[25]/@ipda"/>
<eval expected="49700" xpath="//flow/block[2]/block[25]/@bpd"/>
<eval expected="54700" xpath="//flow/block[2]/block[25]/@bpda"/>
- <eval expected="300000" xpath="//flow/block[2]/block[25]/@left-offset"/>
+ <eval expected="203000" xpath="//flow/block[2]/block[25]/@left-offset"/>
<eval expected="241100" xpath="//flow/block[2]/block[25]/@top-offset"/>
<eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[25]/@border-after"/>
- <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[25]/@border-end"/>
- <eval expected="167500" xpath="//flow/block[2]/block[26]/@ipd"/>
- <eval expected="167500" xpath="//flow/block[2]/block[26]/@ipda"/>
- <eval expected="135400" xpath="//flow/block[2]/block[26]/@bpd"/>
- <eval expected="135400" xpath="//flow/block[2]/block[26]/@bpda"/>
- <eval expected="230000" xpath="//flow/block[2]/block[26]/@left-offset"/>
- <eval expected="155400" xpath="//flow/block[2]/block[26]/@top-offset"/>
+ <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[25]/@border-start"/>
+ <eval expected="97500" xpath="//flow/block[2]/block[26]/@ipd"/>
+ <eval expected="102500" xpath="//flow/block[2]/block[26]/@ipda"/>
+ <eval expected="49700" xpath="//flow/block[2]/block[26]/@bpd"/>
+ <eval expected="54700" xpath="//flow/block[2]/block[26]/@bpda"/>
+ <eval expected="300000" xpath="//flow/block[2]/block[26]/@left-offset"/>
+ <eval expected="241100" xpath="//flow/block[2]/block[26]/@top-offset"/>
+ <eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[26]/@border-after"/>
+ <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[26]/@border-end"/>
+ <eval expected="167500" xpath="//flow/block[2]/block[27]/@ipd"/>
+ <eval expected="167500" xpath="//flow/block[2]/block[27]/@ipda"/>
+ <eval expected="135400" xpath="//flow/block[2]/block[27]/@bpd"/>
+ <eval expected="135400" xpath="//flow/block[2]/block[27]/@bpda"/>
+ <eval expected="230000" xpath="//flow/block[2]/block[27]/@left-offset"/>
+ <eval expected="155400" xpath="//flow/block[2]/block[27]/@top-offset"/>
<!-- table 2 -->
<eval expected="199300" xpath="//flow/block[4]/@bpd"/>
Modified: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2_no-col.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2_no-col.xml?rev=690319&r1=690318&r2=690319&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2_no-col.xml (original)
+++ xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/table_border-collapse_collapse_spans_2_no-col.xml Fri Aug 29 09:36:17 2008
@@ -279,184 +279,186 @@
<eval expected="(solid,#c0c0c0,6000,collapse-outer)" xpath="//flow/block[2]/block[2]/@border-before"/>
<eval expected="(solid,#c0c0c0,6000,collapse-inner)" xpath="//flow/block[2]/block[2]/@border-start"/>
<eval expected="(solid,#c0c0c0,6000,collapse-inner)" xpath="//flow/block[2]/block[2]/@border-end"/>
- <eval expected="92000" xpath="//flow/block[2]/block[3]/@ipd"/>
- <eval expected="108000" xpath="//flow/block[2]/block[3]/@ipda"/>
- <eval expected="43200" xpath="//flow/block[2]/block[3]/@bpd"/>
- <eval expected="61200" xpath="//flow/block[2]/block[3]/@bpda"/>
- <eval expected="3000" xpath="//flow/block[2]/block[3]/@left-offset"/>
- <eval expected="16400" xpath="//flow/block[2]/block[3]/@top-offset"/>
- <eval expected="(solid,#800000,12000,collapse-inner)" xpath="//flow/block[2]/block[3]/@border-before"/>
- <eval expected="(double,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[3]/@border-after"/>
- <eval expected="(solid,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[3]/@border-start"/>
- <eval expected="(dashed,#ff0000,10000,collapse-inner)" xpath="//flow/block[2]/block[3]/@border-end"/>
- <eval expected="80000" xpath="//flow/block[2]/block[4]/@ipd"/>
- <eval expected="120000" xpath="//flow/block[2]/block[4]/@ipda"/>
- <eval expected="30900" xpath="//flow/block[2]/block[4]/@bpd"/>
- <eval expected="42900" xpath="//flow/block[2]/block[4]/@bpda"/>
- <eval expected="15000" xpath="//flow/block[2]/block[4]/@left-offset"/>
- <eval expected="71600" xpath="//flow/block[2]/block[4]/@top-offset"/>
- <eval expected="(double,#ff00ff,6000,collapse-inner)" xpath="//flow/block[2]/block[4]/@border-before"/>
- <eval expected="(inset,#ff00ff,6000,collapse-inner)" xpath="//flow/block[2]/block[4]/@border-after"/>
- <eval expected="(solid,#ff00ff,30000,collapse-outer)" xpath="//flow/block[2]/block[4]/@border-start"/>
+ <!-- block[3] is an empty cell -->
+ <eval expected="92000" xpath="//flow/block[2]/block[4]/@ipd"/>
+ <eval expected="108000" xpath="//flow/block[2]/block[4]/@ipda"/>
+ <eval expected="43200" xpath="//flow/block[2]/block[4]/@bpd"/>
+ <eval expected="61200" xpath="//flow/block[2]/block[4]/@bpda"/>
+ <eval expected="3000" xpath="//flow/block[2]/block[4]/@left-offset"/>
+ <eval expected="16400" xpath="//flow/block[2]/block[4]/@top-offset"/>
+ <eval expected="(solid,#800000,12000,collapse-inner)" xpath="//flow/block[2]/block[4]/@border-before"/>
+ <eval expected="(double,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[4]/@border-after"/>
+ <eval expected="(solid,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[4]/@border-start"/>
<eval expected="(dashed,#ff0000,10000,collapse-inner)" xpath="//flow/block[2]/block[4]/@border-end"/>
- <eval expected="95000" xpath="//flow/block[2]/block[5]/@ipd"/>
- <eval expected="105000" xpath="//flow/block[2]/block[5]/@ipda"/>
- <eval expected="52200" xpath="//flow/block[2]/block[5]/@bpd"/>
- <eval expected="52200" xpath="//flow/block[2]/block[5]/@bpda"/>
- <eval expected="105000" xpath="//flow/block[2]/block[5]/@left-offset"/>
- <eval expected="22400" xpath="//flow/block[2]/block[5]/@top-offset"/>
- <eval expected="(dashed,#ff0000,10000,collapse-inner)" xpath="//flow/block[2]/block[5]/@border-start"/>
- <eval expected="98000" xpath="//flow/block[2]/block[6]/@ipd"/>
- <eval expected="102000" xpath="//flow/block[2]/block[6]/@ipda"/>
- <eval expected="50200" xpath="//flow/block[2]/block[6]/@bpd"/>
- <eval expected="54200" xpath="//flow/block[2]/block[6]/@bpda"/>
- <eval expected="200000" xpath="//flow/block[2]/block[6]/@left-offset"/>
- <eval expected="20400" xpath="//flow/block[2]/block[6]/@top-offset"/>
- <eval expected="(solid,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[6]/@border-before"/>
- <eval expected="(double,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[6]/@border-end"/>
+ <!-- block[5] is an empty cell -->
+ <!-- 3.1: -->
+ <eval expected="80000" xpath="//flow/block[2]/block[6]/@ipd"/>
+ <eval expected="120000" xpath="//flow/block[2]/block[6]/@ipda"/>
+ <eval expected="30900" xpath="//flow/block[2]/block[6]/@bpd"/>
+ <eval expected="42900" xpath="//flow/block[2]/block[6]/@bpda"/>
+ <eval expected="15000" xpath="//flow/block[2]/block[6]/@left-offset"/>
+ <eval expected="71600" xpath="//flow/block[2]/block[6]/@top-offset"/>
+ <eval expected="(double,#ff00ff,6000,collapse-inner)" xpath="//flow/block[2]/block[6]/@border-before"/>
+ <eval expected="(inset,#ff00ff,6000,collapse-inner)" xpath="//flow/block[2]/block[6]/@border-after"/>
+ <eval expected="(solid,#ff00ff,30000,collapse-outer)" xpath="//flow/block[2]/block[6]/@border-start"/>
+ <eval expected="(dashed,#ff0000,10000,collapse-inner)" xpath="//flow/block[2]/block[6]/@border-end"/>
+ <!-- 4x border-only cells for 2.2: -->
<eval expected="95000" xpath="//flow/block[2]/block[7]/@ipd"/>
<eval expected="105000" xpath="//flow/block[2]/block[7]/@ipda"/>
- <eval expected="34400" xpath="//flow/block[2]/block[7]/@bpd"/>
- <eval expected="39400" xpath="//flow/block[2]/block[7]/@bpda"/>
+ <eval expected="52200" xpath="//flow/block[2]/block[7]/@bpd"/>
+ <eval expected="52200" xpath="//flow/block[2]/block[7]/@bpda"/>
<eval expected="105000" xpath="//flow/block[2]/block[7]/@left-offset"/>
- <eval expected="74600" xpath="//flow/block[2]/block[7]/@top-offset"/>
- <eval expected="(outset,#c0c0c0,5000,collapse-inner)" xpath="//flow/block[2]/block[7]/@border-after"/>
+ <eval expected="22400" xpath="//flow/block[2]/block[7]/@top-offset"/>
<eval expected="(dashed,#ff0000,10000,collapse-inner)" xpath="//flow/block[2]/block[7]/@border-start"/>
<eval expected="98000" xpath="//flow/block[2]/block[8]/@ipd"/>
<eval expected="102000" xpath="//flow/block[2]/block[8]/@ipda"/>
- <eval expected="16900" xpath="//flow/block[2]/block[8]/@bpd"/>
- <eval expected="56900" xpath="//flow/block[2]/block[8]/@bpda"/>
+ <eval expected="50200" xpath="//flow/block[2]/block[8]/@bpd"/>
+ <eval expected="54200" xpath="//flow/block[2]/block[8]/@bpda"/>
<eval expected="200000" xpath="//flow/block[2]/block[8]/@left-offset"/>
- <eval expected="74600" xpath="//flow/block[2]/block[8]/@top-offset"/>
- <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[8]/@border-after"/>
+ <eval expected="20400" xpath="//flow/block[2]/block[8]/@top-offset"/>
+ <eval expected="(solid,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[8]/@border-before"/>
<eval expected="(double,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[8]/@border-end"/>
- <eval expected="193000" xpath="//flow/block[2]/block[9]/@ipd"/>
- <eval expected="193000" xpath="//flow/block[2]/block[9]/@ipda"/>
- <eval expected="67100" xpath="//flow/block[2]/block[9]/@bpd"/>
- <eval expected="67100" xpath="//flow/block[2]/block[9]/@bpda"/>
+ <eval expected="95000" xpath="//flow/block[2]/block[9]/@ipd"/>
+ <eval expected="105000" xpath="//flow/block[2]/block[9]/@ipda"/>
+ <eval expected="34400" xpath="//flow/block[2]/block[9]/@bpd"/>
+ <eval expected="39400" xpath="//flow/block[2]/block[9]/@bpda"/>
<eval expected="105000" xpath="//flow/block[2]/block[9]/@left-offset"/>
- <eval expected="24400" xpath="//flow/block[2]/block[9]/@top-offset"/>
+ <eval expected="74600" xpath="//flow/block[2]/block[9]/@top-offset"/>
+ <eval expected="(outset,#c0c0c0,5000,collapse-inner)" xpath="//flow/block[2]/block[9]/@border-after"/>
+ <eval expected="(dashed,#ff0000,10000,collapse-inner)" xpath="//flow/block[2]/block[9]/@border-start"/>
<eval expected="98000" xpath="//flow/block[2]/block[10]/@ipd"/>
<eval expected="102000" xpath="//flow/block[2]/block[10]/@ipda"/>
- <eval expected="14400" xpath="//flow/block[2]/block[10]/@bpd"/>
- <eval expected="59400" xpath="//flow/block[2]/block[10]/@bpda"/>
- <eval expected="302000" xpath="//flow/block[2]/block[10]/@left-offset"/>
- <eval expected="72100" xpath="//flow/block[2]/block[10]/@top-offset"/>
- <eval expected="(ridge,#800000,5000,collapse-inner)" xpath="//flow/block[2]/block[10]/@border-before"/>
+ <eval expected="16900" xpath="//flow/block[2]/block[10]/@bpd"/>
+ <eval expected="56900" xpath="//flow/block[2]/block[10]/@bpda"/>
+ <eval expected="200000" xpath="//flow/block[2]/block[10]/@left-offset"/>
+ <eval expected="74600" xpath="//flow/block[2]/block[10]/@top-offset"/>
<eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[10]/@border-after"/>
- <eval expected="(double,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[10]/@border-start"/>
- <eval expected="78000" xpath="//flow/block[2]/block[11]/@ipd"/>
- <eval expected="122000" xpath="//flow/block[2]/block[11]/@ipda"/>
- <eval expected="56300" xpath="//flow/block[2]/block[11]/@bpd"/>
- <eval expected="66300" xpath="//flow/block[2]/block[11]/@bpda"/>
- <eval expected="2000" xpath="//flow/block[2]/block[11]/@left-offset"/>
- <eval expected="108500" xpath="//flow/block[2]/block[11]/@top-offset"/>
- <eval expected="(inset,#000000,6000,collapse-inner)" xpath="//flow/block[2]/block[11]/@border-before"/>
- <eval expected="(solid,#000000,4000,collapse-inner)" xpath="//flow/block[2]/block[11]/@border-after"/>
- <eval expected="(solid,#000000,4000,collapse-outer)" xpath="//flow/block[2]/block[11]/@border-start"/>
- <eval expected="(solid,#000000,40000,collapse-inner)" xpath="//flow/block[2]/block[11]/@border-end"/>
- <eval expected="77000" xpath="//flow/block[2]/block[12]/@ipd"/>
- <eval expected="123000" xpath="//flow/block[2]/block[12]/@ipda"/>
- <eval expected="28800" xpath="//flow/block[2]/block[12]/@bpd"/>
- <eval expected="93800" xpath="//flow/block[2]/block[12]/@bpda"/>
- <eval expected="120000" xpath="//flow/block[2]/block[12]/@left-offset"/>
- <eval expected="109000" xpath="//flow/block[2]/block[12]/@top-offset"/>
- <eval expected="(outset,#c0c0c0,5000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-before"/>
- <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-after"/>
- <eval expected="(solid,#000000,40000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-start"/>
- <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-end"/>
- <eval expected="95000" xpath="//flow/block[2]/block[13]/@ipd"/>
- <eval expected="105000" xpath="//flow/block[2]/block[13]/@ipda"/>
- <eval expected="36400" xpath="//flow/block[2]/block[13]/@bpd"/>
- <eval expected="52400" xpath="//flow/block[2]/block[13]/@bpda"/>
+ <eval expected="(double,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[10]/@border-end"/>
+ <eval expected="193000" xpath="//flow/block[2]/block[11]/@ipd"/>
+ <eval expected="193000" xpath="//flow/block[2]/block[11]/@ipda"/>
+ <eval expected="67100" xpath="//flow/block[2]/block[11]/@bpd"/>
+ <eval expected="67100" xpath="//flow/block[2]/block[11]/@bpda"/>
+ <eval expected="105000" xpath="//flow/block[2]/block[11]/@left-offset"/>
+ <eval expected="24400" xpath="//flow/block[2]/block[11]/@top-offset"/>
+ <eval expected="98000" xpath="//flow/block[2]/block[12]/@ipd"/>
+ <eval expected="102000" xpath="//flow/block[2]/block[12]/@ipda"/>
+ <eval expected="14400" xpath="//flow/block[2]/block[12]/@bpd"/>
+ <eval expected="59400" xpath="//flow/block[2]/block[12]/@bpda"/>
+ <eval expected="302000" xpath="//flow/block[2]/block[12]/@left-offset"/>
+ <eval expected="72100" xpath="//flow/block[2]/block[12]/@top-offset"/>
+ <eval expected="(ridge,#800000,5000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-before"/>
+ <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-after"/>
+ <eval expected="(double,#ff0000,4000,collapse-inner)" xpath="//flow/block[2]/block[12]/@border-start"/>
+ <eval expected="78000" xpath="//flow/block[2]/block[13]/@ipd"/>
+ <eval expected="122000" xpath="//flow/block[2]/block[13]/@ipda"/>
+ <eval expected="56300" xpath="//flow/block[2]/block[13]/@bpd"/>
+ <eval expected="66300" xpath="//flow/block[2]/block[13]/@bpda"/>
<eval expected="2000" xpath="//flow/block[2]/block[13]/@left-offset"/>
- <eval expected="170800" xpath="//flow/block[2]/block[13]/@top-offset"/>
- <eval expected="(solid,#000000,4000,collapse-inner)" xpath="//flow/block[2]/block[13]/@border-before"/>
- <eval expected="(solid,#800000,12000,collapse-inner)" xpath="//flow/block[2]/block[13]/@border-after"/>
+ <eval expected="108500" xpath="//flow/block[2]/block[13]/@top-offset"/>
+ <eval expected="(inset,#000000,6000,collapse-inner)" xpath="//flow/block[2]/block[13]/@border-before"/>
+ <eval expected="(solid,#000000,4000,collapse-inner)" xpath="//flow/block[2]/block[13]/@border-after"/>
<eval expected="(solid,#000000,4000,collapse-outer)" xpath="//flow/block[2]/block[13]/@border-start"/>
- <eval expected="(solid,#c0c0c0,6000,collapse-inner)" xpath="//flow/block[2]/block[13]/@border-end"/>
- <eval expected="67000" xpath="//flow/block[2]/block[14]/@ipd"/>
- <eval expected="133000" xpath="//flow/block[2]/block[14]/@ipda"/>
- <eval expected="14400" xpath="//flow/block[2]/block[14]/@bpd"/>
- <eval expected="74400" xpath="//flow/block[2]/block[14]/@bpda"/>
- <eval expected="103000" xpath="//flow/block[2]/block[14]/@left-offset"/>
- <eval expected="142800" xpath="//flow/block[2]/block[14]/@top-offset"/>
- <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-before"/>
- <eval expected="(solid,#c0c0c0,6000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-start"/>
- <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-end"/>
- <eval expected="94000" xpath="//flow/block[2]/block[15]/@ipd"/>
- <eval expected="106000" xpath="//flow/block[2]/block[15]/@ipda"/>
- <eval expected="43200" xpath="//flow/block[2]/block[15]/@bpd"/>
- <eval expected="61200" xpath="//flow/block[2]/block[15]/@bpda"/>
- <eval expected="3000" xpath="//flow/block[2]/block[15]/@left-offset"/>
- <eval expected="211200" xpath="//flow/block[2]/block[15]/@top-offset"/>
- <eval expected="(solid,#800000,12000,collapse-inner)" xpath="//flow/block[2]/block[15]/@border-before"/>
- <eval expected="(double,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[15]/@border-after"/>
- <eval expected="(solid,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[15]/@border-start"/>
- <eval expected="(solid,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[15]/@border-end"/>
- <!--
- <eval expected="94000" xpath="//flow/block[2]/block[16]/@ipd"/>
- <eval expected="106000" xpath="//flow/block[2]/block[16]/@ipda"/>
- <eval expected="52200" xpath="//flow/block[2]/block[16]/@bpd"/>
- <eval expected="52200" xpath="//flow/block[2]/block[16]/@bpda"/>
+ <eval expected="(solid,#000000,40000,collapse-inner)" xpath="//flow/block[2]/block[13]/@border-end"/>
+ <eval expected="77000" xpath="//flow/block[2]/block[14]/@ipd"/>
+ <eval expected="123000" xpath="//flow/block[2]/block[14]/@ipda"/>
+ <eval expected="28800" xpath="//flow/block[2]/block[14]/@bpd"/>
+ <eval expected="93800" xpath="//flow/block[2]/block[14]/@bpda"/>
+ <eval expected="120000" xpath="//flow/block[2]/block[14]/@left-offset"/>
+ <eval expected="109000" xpath="//flow/block[2]/block[14]/@top-offset"/>
+ <eval expected="(outset,#c0c0c0,5000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-before"/>
+ <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-after"/>
+ <eval expected="(solid,#000000,40000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-start"/>
+ <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[14]/@border-end"/>
+ <eval expected="95000" xpath="//flow/block[2]/block[15]/@ipd"/>
+ <eval expected="105000" xpath="//flow/block[2]/block[15]/@ipda"/>
+ <eval expected="36400" xpath="//flow/block[2]/block[15]/@bpd"/>
+ <eval expected="52400" xpath="//flow/block[2]/block[15]/@bpda"/>
+ <eval expected="2000" xpath="//flow/block[2]/block[15]/@left-offset"/>
+ <eval expected="170800" xpath="//flow/block[2]/block[15]/@top-offset"/>
+ <eval expected="(solid,#000000,4000,collapse-inner)" xpath="//flow/block[2]/block[15]/@border-before"/>
+ <eval expected="(solid,#800000,12000,collapse-inner)" xpath="//flow/block[2]/block[15]/@border-after"/>
+ <eval expected="(solid,#000000,4000,collapse-outer)" xpath="//flow/block[2]/block[15]/@border-start"/>
+ <eval expected="(solid,#c0c0c0,6000,collapse-inner)" xpath="//flow/block[2]/block[15]/@border-end"/>
+ <eval expected="67000" xpath="//flow/block[2]/block[16]/@ipd"/>
+ <eval expected="133000" xpath="//flow/block[2]/block[16]/@ipda"/>
+ <eval expected="14400" xpath="//flow/block[2]/block[16]/@bpd"/>
+ <eval expected="74400" xpath="//flow/block[2]/block[16]/@bpda"/>
<eval expected="103000" xpath="//flow/block[2]/block[16]/@left-offset"/>
- <eval expected="217200" xpath="//flow/block[2]/block[16]/@top-offset"/>
- <eval expected="(double,#800000,0,collapse-outer)" xpath="//flow/block[2]/block[16]/@border-after"/>
- <eval expected="(solid,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-start"/>
- <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-end"/>
- -->
- <eval expected="97000" xpath="//flow/block[2]/block[16]/@ipd"/>
- <eval expected="103000" xpath="//flow/block[2]/block[16]/@ipda"/>
- <eval expected="41300" xpath="//flow/block[2]/block[16]/@bpd"/>
- <eval expected="81300" xpath="//flow/block[2]/block[16]/@bpda"/>
- <eval expected="203000" xpath="//flow/block[2]/block[16]/@left-offset"/>
- <eval expected="91500" xpath="//flow/block[2]/block[16]/@top-offset"/>
- <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-before"/>
- <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-start"/>
- <eval expected="97500" xpath="//flow/block[2]/block[17]/@ipd"/>
- <eval expected="102500" xpath="//flow/block[2]/block[17]/@ipda"/>
- <eval expected="41300" xpath="//flow/block[2]/block[17]/@bpd"/>
- <eval expected="81300" xpath="//flow/block[2]/block[17]/@bpda"/>
- <eval expected="300000" xpath="//flow/block[2]/block[17]/@left-offset"/>
- <eval expected="91500" xpath="//flow/block[2]/block[17]/@top-offset"/>
- <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[17]/@border-before"/>
- <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[17]/@border-end"/>
- <eval expected="70000" xpath="//flow/block[2]/block[18]/@ipd"/>
- <eval expected="130000" xpath="//flow/block[2]/block[18]/@ipda"/>
- <eval expected="44400" xpath="//flow/block[2]/block[18]/@bpd"/>
- <eval expected="44400" xpath="//flow/block[2]/block[18]/@bpda"/>
- <eval expected="230000" xpath="//flow/block[2]/block[18]/@left-offset"/>
- <eval expected="172800" xpath="//flow/block[2]/block[18]/@top-offset"/>
- <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[18]/@border-start"/>
- <eval expected="97500" xpath="//flow/block[2]/block[19]/@ipd"/>
- <eval expected="102500" xpath="//flow/block[2]/block[19]/@ipda"/>
- <eval expected="44400" xpath="//flow/block[2]/block[19]/@bpd"/>
- <eval expected="44400" xpath="//flow/block[2]/block[19]/@bpda"/>
- <eval expected="300000" xpath="//flow/block[2]/block[19]/@left-offset"/>
- <eval expected="172800" xpath="//flow/block[2]/block[19]/@top-offset"/>
- <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[19]/@border-end"/>
- <eval expected="97000" xpath="//flow/block[2]/block[20]/@ipd"/>
- <eval expected="103000" xpath="//flow/block[2]/block[20]/@ipda"/>
- <eval expected="49700" xpath="//flow/block[2]/block[20]/@bpd"/>
- <eval expected="54700" xpath="//flow/block[2]/block[20]/@bpda"/>
- <eval expected="203000" xpath="//flow/block[2]/block[20]/@left-offset"/>
- <eval expected="217200" xpath="//flow/block[2]/block[20]/@top-offset"/>
- <eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[20]/@border-after"/>
- <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[20]/@border-start"/>
- <eval expected="97500" xpath="//flow/block[2]/block[21]/@ipd"/>
- <eval expected="102500" xpath="//flow/block[2]/block[21]/@ipda"/>
- <eval expected="49700" xpath="//flow/block[2]/block[21]/@bpd"/>
- <eval expected="54700" xpath="//flow/block[2]/block[21]/@bpda"/>
- <eval expected="300000" xpath="//flow/block[2]/block[21]/@left-offset"/>
- <eval expected="217200" xpath="//flow/block[2]/block[21]/@top-offset"/>
- <eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[21]/@border-after"/>
- <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[21]/@border-end"/>
- <eval expected="167500" xpath="//flow/block[2]/block[22]/@ipd"/>
- <eval expected="167500" xpath="//flow/block[2]/block[22]/@ipda"/>
- <eval expected="135400" xpath="//flow/block[2]/block[22]/@bpd"/>
- <eval expected="135400" xpath="//flow/block[2]/block[22]/@bpda"/>
- <eval expected="230000" xpath="//flow/block[2]/block[22]/@left-offset"/>
- <eval expected="131500" xpath="//flow/block[2]/block[22]/@top-offset"/>
+ <eval expected="142800" xpath="//flow/block[2]/block[16]/@top-offset"/>
+ <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-before"/>
+ <eval expected="(solid,#c0c0c0,6000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-start"/>
+ <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[16]/@border-end"/>
+ <eval expected="94000" xpath="//flow/block[2]/block[17]/@ipd"/>
+ <eval expected="106000" xpath="//flow/block[2]/block[17]/@ipda"/>
+ <eval expected="43200" xpath="//flow/block[2]/block[17]/@bpd"/>
+ <eval expected="61200" xpath="//flow/block[2]/block[17]/@bpda"/>
+ <eval expected="3000" xpath="//flow/block[2]/block[17]/@left-offset"/>
+ <eval expected="211200" xpath="//flow/block[2]/block[17]/@top-offset"/>
+ <eval expected="(solid,#800000,12000,collapse-inner)" xpath="//flow/block[2]/block[17]/@border-before"/>
+ <eval expected="(double,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[17]/@border-after"/>
+ <eval expected="(solid,#800000,6000,collapse-outer)" xpath="//flow/block[2]/block[17]/@border-start"/>
+ <eval expected="(solid,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[17]/@border-end"/>
+ <eval expected="94000" xpath="//flow/block[2]/block[18]/@ipd"/>
+ <eval expected="106000" xpath="//flow/block[2]/block[18]/@ipda"/>
+ <eval expected="52200" xpath="//flow/block[2]/block[18]/@bpd"/>
+ <eval expected="52200" xpath="//flow/block[2]/block[18]/@bpda"/>
+ <eval expected="103000" xpath="//flow/block[2]/block[18]/@left-offset"/>
+ <eval expected="217200" xpath="//flow/block[2]/block[18]/@top-offset"/>
+ <eval expected="" xpath="//flow/block[2]/block[18]/@border-after"/>
+ <eval expected="(solid,#800000,6000,collapse-inner)" xpath="//flow/block[2]/block[18]/@border-start"/>
+ <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[18]/@border-end"/>
+ <eval expected="97000" xpath="//flow/block[2]/block[19]/@ipd"/>
+ <eval expected="103000" xpath="//flow/block[2]/block[19]/@ipda"/>
+ <eval expected="41300" xpath="//flow/block[2]/block[19]/@bpd"/>
+ <eval expected="81300" xpath="//flow/block[2]/block[19]/@bpda"/>
+ <eval expected="203000" xpath="//flow/block[2]/block[19]/@left-offset"/>
+ <eval expected="91500" xpath="//flow/block[2]/block[19]/@top-offset"/>
+ <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[19]/@border-before"/>
+ <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[19]/@border-start"/>
+ <eval expected="97500" xpath="//flow/block[2]/block[20]/@ipd"/>
+ <eval expected="102500" xpath="//flow/block[2]/block[20]/@ipda"/>
+ <eval expected="41300" xpath="//flow/block[2]/block[20]/@bpd"/>
+ <eval expected="81300" xpath="//flow/block[2]/block[20]/@bpda"/>
+ <eval expected="300000" xpath="//flow/block[2]/block[20]/@left-offset"/>
+ <eval expected="91500" xpath="//flow/block[2]/block[20]/@top-offset"/>
+ <eval expected="(double,#008000,40000,collapse-inner)" xpath="//flow/block[2]/block[20]/@border-before"/>
+ <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[20]/@border-end"/>
+ <eval expected="70000" xpath="//flow/block[2]/block[21]/@ipd"/>
+ <eval expected="130000" xpath="//flow/block[2]/block[21]/@ipda"/>
+ <eval expected="44400" xpath="//flow/block[2]/block[21]/@bpd"/>
+ <eval expected="44400" xpath="//flow/block[2]/block[21]/@bpda"/>
+ <eval expected="230000" xpath="//flow/block[2]/block[21]/@left-offset"/>
+ <eval expected="172800" xpath="//flow/block[2]/block[21]/@top-offset"/>
+ <eval expected="(solid,#c0c0c0,60000,collapse-inner)" xpath="//flow/block[2]/block[21]/@border-start"/>
+ <eval expected="97500" xpath="//flow/block[2]/block[22]/@ipd"/>
+ <eval expected="102500" xpath="//flow/block[2]/block[22]/@ipda"/>
+ <eval expected="44400" xpath="//flow/block[2]/block[22]/@bpd"/>
+ <eval expected="44400" xpath="//flow/block[2]/block[22]/@bpda"/>
+ <eval expected="300000" xpath="//flow/block[2]/block[22]/@left-offset"/>
+ <eval expected="172800" xpath="//flow/block[2]/block[22]/@top-offset"/>
+ <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[22]/@border-end"/>
+ <eval expected="97000" xpath="//flow/block[2]/block[23]/@ipd"/>
+ <eval expected="103000" xpath="//flow/block[2]/block[23]/@ipda"/>
+ <eval expected="49700" xpath="//flow/block[2]/block[23]/@bpd"/>
+ <eval expected="54700" xpath="//flow/block[2]/block[23]/@bpda"/>
+ <eval expected="203000" xpath="//flow/block[2]/block[23]/@left-offset"/>
+ <eval expected="217200" xpath="//flow/block[2]/block[23]/@top-offset"/>
+ <eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[23]/@border-after"/>
+ <eval expected="(solid,#008000,6000,collapse-inner)" xpath="//flow/block[2]/block[23]/@border-start"/>
+ <eval expected="97500" xpath="//flow/block[2]/block[24]/@ipd"/>
+ <eval expected="102500" xpath="//flow/block[2]/block[24]/@ipda"/>
+ <eval expected="49700" xpath="//flow/block[2]/block[24]/@bpd"/>
+ <eval expected="54700" xpath="//flow/block[2]/block[24]/@bpda"/>
+ <eval expected="300000" xpath="//flow/block[2]/block[24]/@left-offset"/>
+ <eval expected="217200" xpath="//flow/block[2]/block[24]/@top-offset"/>
+ <eval expected="(solid,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[24]/@border-after"/>
+ <eval expected="(dashed,#008000,5000,collapse-outer)" xpath="//flow/block[2]/block[24]/@border-end"/>
+ <eval expected="167500" xpath="//flow/block[2]/block[25]/@ipd"/>
+ <eval expected="167500" xpath="//flow/block[2]/block[25]/@ipda"/>
+ <eval expected="135400" xpath="//flow/block[2]/block[25]/@bpd"/>
+ <eval expected="135400" xpath="//flow/block[2]/block[25]/@bpda"/>
+ <eval expected="230000" xpath="//flow/block[2]/block[25]/@left-offset"/>
+ <eval expected="131500" xpath="//flow/block[2]/block[25]/@top-offset"/>
<!-- table 2 -->
<eval expected="199300" xpath="//flow/block[4]/@bpd"/>
---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/
Posted by Andreas Delmelle <an...@telenet.be>.
On Sep 12, 2008, at 08:27, Jeremias Maerki wrote:
> On 11.09.2008 22:59:31 Vincent Hennebert wrote:
>> Jeremias Maerki wrote:
>> <snip />
>>> At any rate, I don't think that generating FOs for missing table
>>> cells
>>> is the right approach.
>>
>> Why?
>
> Elaborating on "additional overhead": You would generate additional
> FOs
> for non-existent table-cells. For each FO, a layout manager is created
> which will also need to be called. Table layout will need to run
> through
> more cells per row than necessary even though they're empty. More
> objects, more CPU cycles. Granted we're probably not talking about
> a lot
> of empty table-cells but still.
Sounds about right to me. Besides that, the instance size of a
TableCell is large enough to justify not generating any more of these
than strictly necessary. Let alone increase the amount of memory used
by the FO tree even further if we would also start generating
implicit empty Blocks...
To work around that, one would need to create a new FONode subclass,
especially meant to deal with missing cells (no need to reserve space
for references to properties and/or child-nodes). If that particular
type is then mapped to a dedicated (very minimal) LM implementation,
this could reduce the overhead somewhat.
On the other hand, all is well when you only consider very simple
cases, where there's a handful of empty positions in a whole table
(or none at all).
Once we start considering more exceptional cases, say a 20x200 grid,
with only a handful of effective cells per row, it becomes apparent
that generating dummy FONodes for the empty grid-units is probably
not the best of solutions (in terms of generality and scalability).
If there is only one position per row that is effectively occupied by
a table-cell, then we would, what, generate 19x200 'missing' cells
(and blocks)? Yuck!
> RowPainter is area-generation stage. My solution is just making sure
> areas are generated with the information that is available anyway.
> It's
> not doing anything in a place where it shouldn't do it.
Ultimately, that is the only place where this should have effects, I
think.
Seems like we don't really need dummy nodes for anything layout-
related, so why precisely should they exist at that point?
Unless a darn' *good* answer to this question is given (i.e. a point
other than a rigid adherence to the 'no-duplication' rule), I would
even veto the proposal of making any adjustments in the fo.table
package to solve this... but that's just me. ;-)
> So I'm not convinced that my solution is bad or that your solution is
> better.
The better approach is always the one that works for the simple as
well as the more difficult cases, without too big a drain on
resources for the latter.
I see Vincent's point, but I don't agree that the proposed solution
is actually improving anything. Seems like we should be looking for a
way to eliminate the code-duplication without touching any other
packages than is currently the case. Not possible? While I can't
really offer an alternative here, I don't believe that...
I'd like to point out that FOP in its entirety is a continuous work-
in-progress. The XSL-FO Rec is not a cake-walk to implement (some of
the commercial implementations even go so far as to simply be non-
compliant, to reduce complexity)
As such, there are always going to be 'unfinished' pieces of code, or
parts that can be improved upon. The base rule remains: if it works,
and it didn't work before, the balance is positive. If we take that
simple rule away, this will in the long run be counter-productive, I
think. No committer should
One of the consequences of being what we are, and having an
imagination, is that, no matter what, we can /always/ find something
that would be/look better... Good thing, it can be a goal for the
future.
Cheers
Andreas
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/
test/layoutengine/standard-testcases/
Posted by Vincent Hennebert <vh...@gmail.com>.
Hi Andreas,
Thanks for your answer (and sorry for the delay...).
While I still don’t agree with you, at that point I can agree to
disagree. Although I’m still not happy with your arguments, you do have
valid points, and in the end it’s one approach with drawbacks versus
another approach with other drawbacks.
A few comments below, but to sum up: I’ll see if I can provide
a satisfying fix for the FO tree (using a flyweight pattern if possible
—only one set of table-cell/block objects for all holes). Not right now,
however... Otherwise I’ll try to find a better fix in the layout engine,
but then it’s indeed best to wait for the implementation of the new
approach.
Andreas Delmelle wrote:
> On Sep 17, 2008, at 13:26, Vincent Hennebert wrote:
<snip />
>> Now /that/ is maybe the place for optimizations: improve the FO tree.
>> For example make sure that no object is created if a property has its
>> default value;
>
> Well, you'll be pleased to know that is *exactly* what happens now. 8-)
Oh, I see. Well, good news...
> My point is not so much the objects for the properties. It is the
> reference to them that is reserved in the containing FONode. /That/ is
> what makes the instance size of a Block rather large ATM, since it has
> quite a few of those.
... and then it’s even less a problem IMO. But I get your point.
<snip/>
> Sure. I very much agree that code duplication needs to be avoided. The
> architecture of FOP, OTOH, is precisely not to be viewed as one picture.
> Much effort has been made in the past to more clearly draw the
> distinction between the parsing/layout/rendering modules, exactly to
> make implementing new features easier. If a feature can be implemented
> by modifying one of those modules, then why spread the modifications the
> other(s)?
Well, absolutely: if the feature can be implemented in the FO tree only,
why spread it over the layout engine? Right now, EmptyGridUnits are
added at the FO tree stage /then/ handled at the layout stage.
<snip/>
> Ultimately, as I recall, it was always the long-term intention that
> the
> layout package would contain no hard references to FOP's own fo package
> (apart from in classes like LayoutManagerMapping, maybe), but the FO
> tree would be exposed as a set of interfaces (possibly to be made
> standard among FO processors; I recall Victor Mote trying to raise a
> discussion about this quite some time ago). One FO tree implementor may
> choose to use SAX events to build a FO tree structure (as FOP's does),
> another might choose to build a FO tree backed by a proprietary lazy DOM
> (or collection thereof?). Yet another may choose an entirely different
> approach, perhaps only remotely related to any known XML API. As long as
> it could be mapped to standard interfaces, it wouldn't matter /where/
> the FOs come from exactly, the layoutengine would ideally always be able
> to work with it.
Point, however standard interfaces would come with a contract: “The FO
tree /may/ return tables with holes in them”, or OTOH “The Layout Engine
/must/ be given a table without any hole in it”, etc.
<snip/>
Vincent
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/
Posted by Andreas Delmelle <an...@telenet.be>.
On Sep 17, 2008, at 13:26, Vincent Hennebert wrote:
Hi Vincent
> Andreas Delmelle wrote:
>>
>> Not necessarily. Maybe the author was just trying to be creative
>> (avoiding a trip through SVG, by using tables as a medium-resolution
>> grid; something like that?)
>
> I know users can be very creative when it comes to using tables.
> Still.
> Assuming there’s a use case for the example you describe, this will
> probably be to achieve a fancy layout in a magazine-like document. Not
> a 10 thousand page document then, and the overhead won’t be
> noticeable.
>
> Also, let’s not forget that we’re dealing with a corner case here. In
> most cases empty table-cell elements /will/ be added by the XSLT
> stylesheet that produces the FO document anyway. The fact that this
> issue occurred only recently is a confirmation of that. Hence why the
> mentioned overhead isn’t really an issue, and why specific code should
> be kept to a minimum.
It's still /unnecessary/ overhead, if you ask me...
I know I've been a big fan of normalizing the FO tree in general, but
still, if the cell wasn't there, then forcibly generating it (at
parse-time) is not really worth the trouble. It will likely cost more
than it saves.
>> <snip />
> Now /that/ is maybe the place for optimizations: improve the FO tree.
> For example make sure that no object is created if a property has its
> default value;
Well, you'll be pleased to know that is *exactly* what happens now. 8-)
Property instances for default values are created only once (in
FOPropertyMapping), and re-used for every occasion (in multi-session/
servlet context: created once at application startup, and re-used as
long as the JVM does not terminate). My point is not so much the
objects for the properties. It is the reference to them that is
reserved in the containing FONode. /That/ is what makes the instance
size of a Block rather large ATM, since it has quite a few of those.
The space taken up by the instances that those references point to
has already been optimized a great deal.
> or use a common read-only object for the default value.
> All the FO tree would benefit from that kind of optimization. But I’m
> not a specialist of that area...
If I'm correct, only very little can be done to improve efficiency
further in terms of memory usage in the FO tree /itself/.
The focus should begin to move, and it is. As long as there is no way
in which those FONodes are going to be released before much later,
when their areas have been added, the impact of further reductions
there is going to be marginal. You will get a hardly noticeable
improvement in smaller documents, and maybe room for one more page
for very large documents. Every extra FONode that can be added due to
more savings in the FO tree, costs almost twice as much further on
(one extra LM, and other layout-related objects)...
>>>> RowPainter is area-generation stage. My solution is just making
>>>> sure
>>>> areas are generated with the information that is available
>>>> anyway. It's
>>>> not doing anything in a place where it shouldn't do it.
>>
>> Ultimately, that is the only place where this should have effects,
>> I think.
>> Seems like we don't really need dummy nodes for anything layout-
>> related,
>> so why precisely should they exist at that point?
>> Unless a darn' *good* answer to this question is given (i.e. a point
>> other than a rigid adherence to the 'no-duplication' rule)
>
> But this is exactly that. A rigid, strict, blind adherence to the
> ‘no-duplication’ rule. Because code duplication almost always is a
> sign
> that something is wrong in the architecture.
> Because something that looks like just a little redundancy generally
> reveals a deeper problem, that grows with time and eventually may
> totally
> prevent the implementation of new features.
Sure. I very much agree that code duplication needs to be avoided.
The architecture of FOP, OTOH, is precisely not to be viewed as one
picture. Much effort has been made in the past to more clearly draw
the distinction between the parsing/layout/rendering modules, exactly
to make implementing new features easier. If a feature can be
implemented by modifying one of those modules, then why spread the
modifications the other(s)?
If architecture is a point, then making the adjustment only in the
layout package, if possible, is preferable. We should, as much as
possible, avoid making changes in the FO tree that the layoutengine
comes to depend on too much. IIRC, it was always the intention of
getting to a point where they are reasonably independent components:
one to build the FO tree, one to perform the layout for a given FO
subtree, one to render the resulting area tree, one to manage the
events produced by the others... This precisely to facilitate
implementation of new features, different types of renderer, etc.
Ultimately, as I recall, it was always the long-term intention that
the layout package would contain no hard references to FOP's own fo
package (apart from in classes like LayoutManagerMapping, maybe), but
the FO tree would be exposed as a set of interfaces (possibly to be
made standard among FO processors; I recall Victor Mote trying to
raise a discussion about this quite some time ago). One FO tree
implementor may choose to use SAX events to build a FO tree structure
(as FOP's does), another might choose to build a FO tree backed by a
proprietary lazy DOM (or collection thereof?). Yet another may choose
an entirely different approach, perhaps only remotely related to any
known XML API. As long as it could be mapped to standard interfaces,
it wouldn't matter /where/ the FOs come from exactly, the
layoutengine would ideally always be able to work with it. Not that
something like that seems very close ATM...
> <snip />
>
>> What is the more precise description of this 'additional
>> maintenance' in
>> this case?
>> Is it really /that/ much that it justifies the generation of
>> additional
>> relatively large and long-lived objects so early in the process?
>> (Objects that basically correspond to nothing at all; maybe due to
>> code
>> that was added just to make the code look cleaner... ;-))
>
> What do you mean?
What it says: adding some code that generates unnecessary overhead,
for the sake of the architecture only, is virtually always a bad
idea, unless there are other compelling reasons to do so.
<snip />
>>
>> Maybe if we leave it, but take note for now, a better solution will
>> become apparent once the refactoring is done (?)
>
> Maybe, but usually the discussion is carried out prior to the
> commit...
> And in that case proper TODOs are placed at proper places in the code.
> And possibly a bug report opened.
Point. I'm definitely not saying the response was unwarranted.
>> As such, there are always going to be 'unfinished' pieces of code, or
>> parts that can be improved upon. The base rule remains: if it
>> works, and
>> it didn't work before, the balance is positive. If we take that
>> simple
>> rule away, this will in the long run be counter-productive, I think.
>
> I have to strongly disagree with that. If it works, and it didn’t work
> before, /and/ the code is kept as clean, readable and maintainable as
> before, /then/ the balance is positive.
That's what review is there for. To make sure that the balance does
not tip over to the other side.
'clean, readable and maintainable' are far from absolutes. What works
for A might not work for B.
You're very free to point that out. I'm very free to do with it
whatever I wish (which obviously depends somewhat on the tenor of the
reaction... and my mood ;-)).
> Look: alright, the implementation of missing cells is not optimal,
> but it’s no big deal.
> Then we have to fix the current shortcomings of border resolution.
> Another slightly sub-optimal change, but again no big deal. Then we
> implement this functionality. And then that feature. Every time a
> small,
> maybe-not-optimal-change-but-no-big-deal. And we end up with
> a completely unmaintainable code base, where the only solution to
> further implement features is to re-write it from scratch. But I
> believe
> I’m repeating myself.
Heh, don't we all from time to time...?
My point is much the same: choosing an approach with a negligible
(but still additional) overhead, once, by itself is not much. When we
would make it a common practice, the code becomes stuffed with a lot
of negligible overhead that was designed in, that together could
mount up to a significant overhead, which further on becomes very
difficult to optimize, and would then warrant a rewrite/major
refactoring one way or the other.
Cheers
Andreas
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/
test/layoutengine/standard-testcases/
Posted by Vincent Hennebert <vh...@gmail.com>.
Hi Andreas,
(Answering both of your messages in one.)
Andreas Delmelle wrote:
> On Sep 16, 2008, at 12:05, Vincent Hennebert wrote:
>
>>
>> I think this is negligible, precisely because we’re not talking about
>> a lot of missing cells. This would start to be noticeable if we were
>> dealing with huge empty tables, and in that case there’s probably
>> something wrong with the source document.
>
> Not necessarily. Maybe the author was just trying to be creative
> (avoiding a trip through SVG, by using tables as a medium-resolution
> grid; something like that?)
I know users can be very creative when it comes to using tables. Still.
Assuming there’s a use case for the example you describe, this will
probably be to achieve a fancy layout in a magazine-like document. Not
a 10 thousand page document then, and the overhead won’t be noticeable.
Also, let’s not forget that we’re dealing with a corner case here. In
most cases empty table-cell elements /will/ be added by the XSLT
stylesheet that produces the FO document anyway. The fact that this
issue occurred only recently is a confirmation of that. Hence why the
mentioned overhead isn’t really an issue, and why specific code should
be kept to a minimum.
> Sounds about right to me. Besides that, the instance size of
> a TableCell
> is large enough to justify not generating any more of these than
> strictly necessary. Let alone increase the amount of memory used by the
> FO tree even further if we would also start generating implicit empty
> Blocks...
Now /that/ is maybe the place for optimizations: improve the FO tree.
For example make sure that no object is created if a property has its
default value; or use a common read-only object for the default value.
All the FO tree would benefit from that kind of optimization. But I’m
not a specialist of that area...
>>> RowPainter is area-generation stage. My solution is just making sure
>>> areas are generated with the information that is available anyway. It's
>>> not doing anything in a place where it shouldn't do it.
>
> Ultimately, that is the only place where this should have effects, I think.
> Seems like we don't really need dummy nodes for anything layout-related,
> so why precisely should they exist at that point?
> Unless a darn' *good* answer to this question is given (i.e. a point
> other than a rigid adherence to the 'no-duplication' rule)
But this is exactly that. A rigid, strict, blind adherence to the
‘no-duplication’ rule. Because code duplication almost always is a sign
that something is wrong in the architecture. Because something that
looks like just a little redundancy generally reveals a deeper problem,
that grows with time and eventually may totally prevent the
implementation of new features.
>> At any rate, this is not worth
>> the additional maintenance required by specific code.
>
> Could be right here, but such considerations are always worth some
> thought...
I already partly answered that above by mentioning the corner case; more
below.
> What is the more precise description of this 'additional maintenance' in
> this case?
> Is it really /that/ much that it justifies the generation of additional
> relatively large and long-lived objects so early in the process?
> (Objects that basically correspond to nothing at all; maybe due to code
> that was added just to make the code look cleaner... ;-))
What do you mean?
> If still yes, then is there really no other approach?
There is almost always another approach ;-). As I said I didn’t have the
opportunity to think much about that, and that’s the idea that
immediately came to my mind. Now there’s probably a way to handle that
in GridUnit, but the getCell() method only is already called in quite
a few places.
> If no, then would it be better to do it now (before refactoring the
> layoutengine) or after?
>
> Maybe if we leave it, but take note for now, a better solution will
> become apparent once the refactoring is done (?)
Maybe, but usually the discussion is carried out prior to the commit...
And in that case proper TODOs are placed at proper places in the code.
And possibly a bug report opened.
> <snip />
>> [Jeremias: ]
>>> RowPainter is area-generation stage. My solution is just making sure
>>> areas are generated with the information that is available anyway. It's
>>> not doing anything in a place where it shouldn't do it.
>>
>> RowPainter deals with rows, not cells. All it does is select the cells
>> for which it’s time to create areas, then it delegates to TableCellLM
>> the actual area creation. With the change we’re discussing there are now
>> two places where areas for cells are created: in TableCellLM /and/ in
>> RowPainter. This is redundant, confusing and increases the maintenance
>> code.
>
> Hmm... maybe an idea to consolidate those parts into a CellPainter,
> then? Just a vague idea (?)
Yes, something like that, probably.
> Also an additional object, but for the empty cells, can be tailored to
> be as lean/light as possible, and it is only created at the appropriate
> time in the process.
> The idea I offered earlier (a special type of FONode to represent empty
> cells) would still generate completely unnecessary TableCellLMs, and
> would be a pest to implement, if I estimate correctly (?)
Right. This is another form of redundancy, so not really desirable.
> As such, there are always going to be 'unfinished' pieces of code, or
> parts that can be improved upon. The base rule remains: if it works, and
> it didn't work before, the balance is positive. If we take that simple
> rule away, this will in the long run be counter-productive, I think.
I have to strongly disagree with that. If it works, and it didn’t work
before, /and/ the code is kept as clean, readable and maintainable as
before, /then/ the balance is positive. Look: alright, the
implementation of missing cells is not optimal, but it’s no big deal.
Then we have to fix the current shortcomings of border resolution.
Another slightly sub-optimal change, but again no big deal. Then we
implement this functionality. And then that feature. Every time a small,
maybe-not-optimal-change-but-no-big-deal. And we end up with
a completely unmaintainable code base, where the only solution to
further implement features is to re-write it from scratch. But I believe
I’m repeating myself.
Vincent
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/
Posted by Andreas Delmelle <an...@telenet.be>.
On Sep 16, 2008, at 12:05, Vincent Hennebert wrote:
>
> I think this is negligible, precisely because we’re not talking about
> a lot of missing cells. This would start to be noticeable if we were
> dealing with huge empty tables, and in that case there’s probably
> something wrong with the source document.
Not necessarily. Maybe the author was just trying to be creative
(avoiding a trip through SVG, by using tables as a medium-resolution
grid; something like that?)
> At any rate, this is not worth
> the additional maintenance required by specific code.
Could be right here, but such considerations are always worth some
thought...
What is the more precise description of this 'additional maintenance'
in this case?
Is it really /that/ much that it justifies the generation of
additional relatively large and long-lived objects so early in the
process? (Objects that basically correspond to nothing at all; maybe
due to code that was added just to make the code look cleaner... ;-))
If still yes, then is there really no other approach?
If no, then would it be better to do it now (before refactoring the
layoutengine) or after?
Maybe if we leave it, but take note for now, a better solution will
become apparent once the refactoring is done (?)
<snip />
> [Jeremias: ]
>> RowPainter is area-generation stage. My solution is just making sure
>> areas are generated with the information that is available anyway.
>> It's
>> not doing anything in a place where it shouldn't do it.
>
> RowPainter deals with rows, not cells. All it does is select the cells
> for which it’s time to create areas, then it delegates to TableCellLM
> the actual area creation. With the change we’re discussing there
> are now
> two places where areas for cells are created: in TableCellLM /and/
> in RowPainter. This is redundant, confusing and increases the
> maintenance code.
Hmm... maybe an idea to consolidate those parts into a CellPainter,
then? Just a vague idea (?)
Also an additional object, but for the empty cells, can be tailored
to be as lean/light as possible, and it is only created at the
appropriate time in the process.
The idea I offered earlier (a special type of FONode to represent
empty cells) would still generate completely unnecessary
TableCellLMs, and would be a pest to implement, if I estimate
correctly (?)
<snip />
Cheers
Andreas
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/
test/layoutengine/standard-testcases/
Posted by Vincent Hennebert <vh...@gmail.com>.
Jeremias Maerki wrote:
> On 11.09.2008 22:59:31 Vincent Hennebert wrote:
>> Jeremias Maerki wrote:
>>> I knew full well when I committed this that there's a certain
>>> probability that you're not going to be happy with how I dit it. It's
>>> hard to please you these days. So, is this post of yours a veto?
>> Depends. The thing is, this issue will most probably show up also with
>> the new layout approach, and it would be good to have it sorted out
>> before.
>>
>>
>>> At any rate, I don't think that generating FOs for missing table cells
>>> is the right approach.
>> Why?
>
> Elaborating on "additional overhead": You would generate additional FOs
> for non-existent table-cells. For each FO, a layout manager is created
> which will also need to be called. Table layout will need to run through
> more cells per row than necessary even though they're empty. More
> objects, more CPU cycles. Granted we're probably not talking about a lot
> of empty table-cells but still.
I think this is negligible, precisely because we’re not talking about
a lot of missing cells. This would start to be noticeable if we were
dealing with huge empty tables, and in that case there’s probably
something wrong with the source document. At any rate, this is not worth
the additional maintenance required by specific code. Other areas in the
code have much more potential for cpu and memory savings.
> RowPainter is area-generation stage. My solution is just making sure
> areas are generated with the information that is available anyway. It's
> not doing anything in a place where it shouldn't do it.
RowPainter deals with rows, not cells. All it does is select the cells
for which it’s time to create areas, then it delegates to TableCellLM
the actual area creation. With the change we’re discussing there are now
two places where areas for cells are created: in TableCellLM /and/ in RowPainter. This is redundant, confusing and increases the maintenance code.
> The change is
> confined to a narrow space. Your solution spans FO tree, layout and area
> generation.
Code modification would be in the FO tree only. We get the rest for
free, handled by code that exists for normal cells and is already
well tested. No need for additional testcases, except making sure that
TableCell elemnts are created at the right place. This is much lighter
in maintenance than a area tree testcase.
> So I'm not convinced that my solution is bad
See last sentence below.
> or that your solution is
> better.
>
>>> As can be seen in my commit, it is perfectly
>>> possible to do without the additional overhead involved with your
>>> solution.
>> But not without additional code duplication.
Vincent
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 11.09.2008 22:59:31 Vincent Hennebert wrote:
> Jeremias Maerki wrote:
> > I knew full well when I committed this that there's a certain
> > probability that you're not going to be happy with how I dit it. It's
> > hard to please you these days. So, is this post of yours a veto?
>
> Depends. The thing is, this issue will most probably show up also with
> the new layout approach, and it would be good to have it sorted out
> before.
>
>
> > At any rate, I don't think that generating FOs for missing table cells
> > is the right approach.
>
> Why?
Elaborating on "additional overhead": You would generate additional FOs
for non-existent table-cells. For each FO, a layout manager is created
which will also need to be called. Table layout will need to run through
more cells per row than necessary even though they're empty. More
objects, more CPU cycles. Granted we're probably not talking about a lot
of empty table-cells but still.
RowPainter is area-generation stage. My solution is just making sure
areas are generated with the information that is available anyway. It's
not doing anything in a place where it shouldn't do it. The change is
confined to a narrow space. Your solution spans FO tree, layout and area
generation.
So I'm not convinced that my solution is bad or that your solution is
better.
> > As can be seen in my commit, it is perfectly
> > possible to do without the additional overhead involved with your
> > solution.
>
> But not without additional code duplication.
>
>
> > On 11.09.2008 14:11:31 Vincent Hennebert wrote:
> >> Hi,
> >>
> >>> Author: jeremias
> >>> Date: Fri Aug 29 09:36:17 2008
> >>> New Revision: 690319
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=690319&view=rev
> >>> Log:
> >>> Added missing generation of areas for empty grid units in tables with collapsing border model.
> >> <snip/>
> >>
> >> I’m not happy with that change. Basically it’s code copy-pasted from
> >> several places and slightly modified to handle missing cells. I didn’t
> >> spend months trying to clean up the table layout code to see that kind
> >> of change introduced at the first opportunity...
> >>
> >> A bit of background first: distinction needs to be made between /empty/
> >> cells (existing table-cell elements with no content), and /missing/
> >> cells (missing table-cell element; e.g., only three cells on a row in
> >> a four-column table). The EmptyGridUnit class is meant to handle that
> >> latter case only (yes, it’s misnamed). I didn’t have the opportunity of
> >> addressing that issue when I worked on tables, so I left it as is.
> >>
> >> That EmptyGridUnit class doesn’t fit well in the big picture. It
> >> constantly requires special care just because you can’t get a non-null
> >> TableCell object from it. The only way I could think of to handle that
> >> properly is to ‘fix’ the FO tree, completing it by adding empty
> >> table-cell elements. Basically in FixedColRowGroupBuilder.endRow(),
> >> instead of filling the gaps with EmptyGridUnit objects, proper TableCell
> >> objects would be created in some way.
> >>
> >> To achieve that we need to be able to create FONode objects outside of
> >> the FOTreeBuilder machinery. Or to trigger the proper events on
> >> FOTreeBuilder so that new TableCell objects are ‘automatically’ created.
> >> I haven’t had the opportunity to look into it into more details, but I’d
> >> rather explore this possibility, instead of dealing with EmptyGridUnit.
> >> If FO tree specialists have any comments or suggestions...
> >>
> >>
> >> Thanks,
> >> Vincent
> >
> >
> >
> >
> > Jeremias Maerki
>
> Vincent
Jeremias Maerki
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/
test/layoutengine/standard-testcases/
Posted by Vincent Hennebert <vh...@gmail.com>.
Jeremias Maerki wrote:
> I knew full well when I committed this that there's a certain
> probability that you're not going to be happy with how I dit it. It's
> hard to please you these days. So, is this post of yours a veto?
Depends. The thing is, this issue will most probably show up also with
the new layout approach, and it would be good to have it sorted out
before.
> At any rate, I don't think that generating FOs for missing table cells
> is the right approach.
Why?
> As can be seen in my commit, it is perfectly
> possible to do without the additional overhead involved with your
> solution.
But not without additional code duplication.
> On 11.09.2008 14:11:31 Vincent Hennebert wrote:
>> Hi,
>>
>>> Author: jeremias
>>> Date: Fri Aug 29 09:36:17 2008
>>> New Revision: 690319
>>>
>>> URL: http://svn.apache.org/viewvc?rev=690319&view=rev
>>> Log:
>>> Added missing generation of areas for empty grid units in tables with collapsing border model.
>> <snip/>
>>
>> I’m not happy with that change. Basically it’s code copy-pasted from
>> several places and slightly modified to handle missing cells. I didn’t
>> spend months trying to clean up the table layout code to see that kind
>> of change introduced at the first opportunity...
>>
>> A bit of background first: distinction needs to be made between /empty/
>> cells (existing table-cell elements with no content), and /missing/
>> cells (missing table-cell element; e.g., only three cells on a row in
>> a four-column table). The EmptyGridUnit class is meant to handle that
>> latter case only (yes, it’s misnamed). I didn’t have the opportunity of
>> addressing that issue when I worked on tables, so I left it as is.
>>
>> That EmptyGridUnit class doesn’t fit well in the big picture. It
>> constantly requires special care just because you can’t get a non-null
>> TableCell object from it. The only way I could think of to handle that
>> properly is to ‘fix’ the FO tree, completing it by adding empty
>> table-cell elements. Basically in FixedColRowGroupBuilder.endRow(),
>> instead of filling the gaps with EmptyGridUnit objects, proper TableCell
>> objects would be created in some way.
>>
>> To achieve that we need to be able to create FONode objects outside of
>> the FOTreeBuilder machinery. Or to trigger the proper events on
>> FOTreeBuilder so that new TableCell objects are ‘automatically’ created.
>> I haven’t had the opportunity to look into it into more details, but I’d
>> rather explore this possibility, instead of dealing with EmptyGridUnit.
>> If FO tree specialists have any comments or suggestions...
>>
>>
>> Thanks,
>> Vincent
>
>
>
>
> Jeremias Maerki
Vincent
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/ test/layoutengine/standard-testcases/
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
I knew full well when I committed this that there's a certain
probability that you're not going to be happy with how I dit it. It's
hard to please you these days. So, is this post of yours a veto?
At any rate, I don't think that generating FOs for missing table cells
is the right approach. As can be seen in my commit, it is perfectly
possible to do without the additional overhead involved with your
solution. And I don't think the code is ugly because of that.
On 11.09.2008 14:11:31 Vincent Hennebert wrote:
> Hi,
>
> > Author: jeremias
> > Date: Fri Aug 29 09:36:17 2008
> > New Revision: 690319
> >
> > URL: http://svn.apache.org/viewvc?rev=690319&view=rev
> > Log:
> > Added missing generation of areas for empty grid units in tables with collapsing border model.
> <snip/>
>
> I’m not happy with that change. Basically it’s code copy-pasted from
> several places and slightly modified to handle missing cells. I didn’t
> spend months trying to clean up the table layout code to see that kind
> of change introduced at the first opportunity...
>
> A bit of background first: distinction needs to be made between /empty/
> cells (existing table-cell elements with no content), and /missing/
> cells (missing table-cell element; e.g., only three cells on a row in
> a four-column table). The EmptyGridUnit class is meant to handle that
> latter case only (yes, it’s misnamed). I didn’t have the opportunity of
> addressing that issue when I worked on tables, so I left it as is.
>
> That EmptyGridUnit class doesn’t fit well in the big picture. It
> constantly requires special care just because you can’t get a non-null
> TableCell object from it. The only way I could think of to handle that
> properly is to ‘fix’ the FO tree, completing it by adding empty
> table-cell elements. Basically in FixedColRowGroupBuilder.endRow(),
> instead of filling the gaps with EmptyGridUnit objects, proper TableCell
> objects would be created in some way.
>
> To achieve that we need to be able to create FONode objects outside of
> the FOTreeBuilder machinery. Or to trigger the proper events on
> FOTreeBuilder so that new TableCell objects are ‘automatically’ created.
> I haven’t had the opportunity to look into it into more details, but I’d
> rather explore this possibility, instead of dealing with EmptyGridUnit.
> If FO tree specialists have any comments or suggestions...
>
>
> Thanks,
> Vincent
Jeremias Maerki
Re: svn commit: r690319 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/table/
test/layoutengine/standard-testcases/
Posted by Vincent Hennebert <vh...@gmail.com>.
Hi,
> Author: jeremias
> Date: Fri Aug 29 09:36:17 2008
> New Revision: 690319
>
> URL: http://svn.apache.org/viewvc?rev=690319&view=rev
> Log:
> Added missing generation of areas for empty grid units in tables with collapsing border model.
<snip/>
I’m not happy with that change. Basically it’s code copy-pasted from
several places and slightly modified to handle missing cells. I didn’t
spend months trying to clean up the table layout code to see that kind
of change introduced at the first opportunity...
A bit of background first: distinction needs to be made between /empty/
cells (existing table-cell elements with no content), and /missing/
cells (missing table-cell element; e.g., only three cells on a row in
a four-column table). The EmptyGridUnit class is meant to handle that
latter case only (yes, it’s misnamed). I didn’t have the opportunity of
addressing that issue when I worked on tables, so I left it as is.
That EmptyGridUnit class doesn’t fit well in the big picture. It
constantly requires special care just because you can’t get a non-null
TableCell object from it. The only way I could think of to handle that
properly is to ‘fix’ the FO tree, completing it by adding empty
table-cell elements. Basically in FixedColRowGroupBuilder.endRow(),
instead of filling the gaps with EmptyGridUnit objects, proper TableCell
objects would be created in some way.
To achieve that we need to be able to create FONode objects outside of
the FOTreeBuilder machinery. Or to trigger the proper events on
FOTreeBuilder so that new TableCell objects are ‘automatically’ created.
I haven’t had the opportunity to look into it into more details, but I’d
rather explore this possibility, instead of dealing with EmptyGridUnit.
If FO tree specialists have any comments or suggestions...
Thanks,
Vincent