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 lf...@apache.org on 2008/06/16 16:52:15 UTC

svn commit: r668177 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/layoutengine/standard-testcases/

Author: lfurini
Date: Mon Jun 16 07:52:14 2008
New Revision: 668177

URL: http://svn.apache.org/viewvc?rev=668177&view=rev
Log:
Fixing the PageBreakingAlgorithm, replacing calls to getLineWidth() with getLineWidth(int) so as to take into account each page's real height.
This fixes the positioning of footnotes when the page bpd is not the same for all pages.

Added:
    xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml   (with props)
    xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml   (with props)
Modified:
    xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
    xmlgraphics/fop/trunk/status.xml

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java?rev=668177&r1=668176&r2=668177&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java Mon Jun 16 07:52:14 2008
@@ -325,7 +325,7 @@
                 // this page contains some footnote citations
                 // add the footnote separator width
                 actualWidth += footnoteSeparatorLength.opt;
-                if (actualWidth + allFootnotes <= getLineWidth()) {
+                if (actualWidth + allFootnotes <= getLineWidth(activeNode.line)) {
                     // there is enough space to insert all footnotes:
                     // add the whole allFootnotes length
                     actualWidth += allFootnotes;
@@ -337,7 +337,7 @@
                                 = checkCanDeferOldFootnotes(pageNode, elementIndex))
                             || newFootnotes)
                            && (footnoteSplit = getFootnoteSplit(pageNode,
-                                   getLineWidth() - actualWidth, canDeferOldFootnotes)) > 0) {
+                                   getLineWidth(activeNode.line) - actualWidth, canDeferOldFootnotes)) > 0) {
                     // it is allowed to break or even defer footnotes if either:
                     //  - there are new footnotes in the last piece of content, and
                     //    there is space to add at least a piece of the first one
@@ -683,7 +683,7 @@
         insertedFootnotesLength = lastNode.totalFootnotes;
         footnoteListIndex = lastNode.footnoteListIndex;
         footnoteElementIndex = lastNode.footnoteElementIndex;
-        int availableBPD = getLineWidth();
+        int availableBPD = getLineWidth(lastNode.line);
         int split = 0;
         KnuthPageNode prevNode = lastNode;
 
@@ -718,7 +718,7 @@
                 removeNode(prevNode.line, prevNode);
 
                 prevNode = node;
-                availableBPD = getLineWidth();
+                availableBPD = getLineWidth(node.line);
             }
         }
         // create the last node

Modified: xmlgraphics/fop/trunk/status.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/status.xml?rev=668177&r1=668176&r2=668177&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/status.xml (original)
+++ xmlgraphics/fop/trunk/status.xml Mon Jun 16 07:52:14 2008
@@ -53,6 +53,11 @@
   
   <changes>
     <release version="FOP Trunk" date="TBD">
+      <action context="Layout" dev="LF" type="fix">
+        Fixed an inconsistency in footnote handling that led to unnecessary
+        empty areas in pages whose last normal line contains footnotes
+        when the page bpd is not the same for all pages.
+      </action>
       <!-- change reverted, to be added back later
       <action context="Renderers" dev="AC" importance="high" type="add">
         Added SVG support for AFP (GOCA).

Added: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml?rev=668177&view=auto
==============================================================================
--- xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml (added)
+++ xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml Mon Jun 16 07:52:14 2008
@@ -0,0 +1,97 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<testcase>
+  <info>
+    <p>
+      This test checks the positioning of footnotes when the body bpd changes 
+      from page to page.
+    </p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+      <fo:layout-master-set>
+        <fo:simple-page-master master-name="first"
+                                page-height="200pt"
+                                page-width="140pt">
+          <fo:region-body background-color="lightgrey"/>
+        </fo:simple-page-master>
+        <fo:simple-page-master master-name="other"
+                                page-height="200pt"
+                                page-width="140pt">
+          <fo:region-body margin-top="20pt"
+                          background-color="lightgrey"/>
+        </fo:simple-page-master>
+        <fo:page-sequence-master master-name="all">
+          <fo:single-page-master-reference master-reference="first"/>
+          <fo:repeatable-page-master-reference master-reference="other"/>
+        </fo:page-sequence-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="all">
+        <fo:flow flow-name="xsl-region-body" font-family="Courier">
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 1.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 2.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 3.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 4.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 5.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 6.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 7.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 8.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 9.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 10.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 11.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 12.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 13.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 14.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 15.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 16.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 17.
+            <fo:footnote>
+              <fo:inline font-size="50%" vertical-align="top">1</fo:inline>
+              <fo:footnote-body>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height"><fo:inline font-size="50%" vertical-align="top">1</fo:inline> ftnt line 1/3.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 2/3.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 3/3.</fo:block>
+              </fo:footnote-body>
+            </fo:footnote>
+          </fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 18.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 19.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 20.</fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="3" xpath="count(//pageViewport)"/>
+    <!-- body height for page 1 -->
+    <eval expected="200000" xpath="//pageViewport[1]/page/regionViewport/@bpd"/>
+    <!-- body height for page 2 -->
+    <eval expected="180000" xpath="//pageViewport[2]/page/regionViewport/@bpd"/>
+    <!-- last content line in page 2 -->
+    <eval expected="line 17. " xpath="//pageViewport[2]/page/regionViewport/regionBody/mainReference/span/flow/block[last()]/lineArea/text[1]"/>
+    <!-- last footnote line in page 2 -->
+    <eval expected="ftnt line 2/3." xpath="//pageViewport[2]/page/regionViewport/regionBody/footnote//block[last()]/lineArea/text[1]"/>
+    <!-- body height for page 3 -->
+    <eval expected="180000" xpath="//pageViewport[3]/page/regionViewport/@bpd"/>
+    <!-- first content line in page 3 -->
+    <eval expected="line 18." xpath="//pageViewport[3]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/lineArea/text[1]"/>
+    <!-- footnote line in page 3 -->
+    <eval expected="ftnt line 3/3." xpath="//pageViewport[3]/page/regionViewport/regionBody/footnote//block[1]/lineArea/text[1]"/>
+  </checks>
+</testcase>

Propchange: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd.xml
------------------------------------------------------------------------------
    svn:keywords = Revision Id

Added: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml?rev=668177&view=auto
==============================================================================
--- xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml (added)
+++ xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml Mon Jun 16 07:52:14 2008
@@ -0,0 +1,102 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<testcase>
+  <info>
+    <p>
+      This test checks the positioning of footnotes when the body bpd changes 
+      from page to page, in particular when footnote pages are created.
+    </p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+      <fo:layout-master-set>
+        <fo:simple-page-master master-name="first"
+                                page-height="200pt"
+                                page-width="140pt">
+          <fo:region-body background-color="lightgrey"/>
+        </fo:simple-page-master>
+        <fo:simple-page-master master-name="second"
+                                page-height="200pt"
+                                page-width="140pt">
+          <fo:region-body margin-top="60pt"
+                          background-color="lightgrey"/>
+        </fo:simple-page-master>
+        <fo:simple-page-master master-name="other"
+                                page-height="200pt"
+                                page-width="140pt">
+          <fo:region-body margin-top="20pt"
+                          background-color="lightgrey"/>
+        </fo:simple-page-master>
+        <fo:page-sequence-master master-name="all">
+          <fo:single-page-master-reference master-reference="first"/>
+          <fo:single-page-master-reference master-reference="second"/>
+          <fo:repeatable-page-master-reference master-reference="other"/>
+        </fo:page-sequence-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="all">
+        <fo:flow flow-name="xsl-region-body" font-family="Courier">
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 1.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 2.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 3.</fo:block>
+          <fo:block line-height="20pt" line-stacking-strategy="font-height">line 4.
+            <fo:footnote>
+              <fo:inline font-size="50%" vertical-align="top">1</fo:inline>
+              <fo:footnote-body>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height"><fo:inline font-size="50%" vertical-align="top">1</fo:inline> ftnt line 1.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 2.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 3.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 4.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 5.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 6.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 7.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 8.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 9.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 10.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 11.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 12.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 13.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 14.</fo:block>
+                <fo:block background-color="pink" line-height="20pt" line-stacking-strategy="font-height">ftnt line 15.</fo:block>
+              </fo:footnote-body>
+            </fo:footnote>
+          </fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="3" xpath="count(//pageViewport)"/>
+    <!-- body height for page 1 -->
+    <eval expected="200000" xpath="//pageViewport[1]/page/regionViewport/@bpd"/>
+
+    <!-- body height for page 2 -->
+    <eval expected="140000" xpath="//pageViewport[2]/page/regionViewport/@bpd"/>
+    <!-- first footnote line in page 2 -->
+    <eval expected="ftnt line 7." xpath="//pageViewport[2]/page/regionViewport/regionBody/footnote//block[1]/lineArea/text[1]"/>
+    <!-- last footnote line in page 2 -->
+    <eval expected="ftnt line 13." xpath="//pageViewport[2]/page/regionViewport/regionBody/footnote//block[last()]/lineArea/text[1]"/>
+
+    <!-- body height for page 3 -->
+    <eval expected="180000" xpath="//pageViewport[3]/page/regionViewport/@bpd"/>
+    <!-- first footnote line in page 3 -->
+    <eval expected="ftnt line 14." xpath="//pageViewport[3]/page/regionViewport/regionBody/footnote//block[1]/lineArea/text[1]"/>
+    <!-- last footnote line in page 3 -->
+    <eval expected="ftnt line 15." xpath="//pageViewport[3]/page/regionViewport/regionBody/footnote//block[last()]/lineArea/text[1]"/>
+  </checks>
+</testcase>

Propchange: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: xmlgraphics/fop/trunk/test/layoutengine/standard-testcases/footnote_changing-page-bpd_2.xml
------------------------------------------------------------------------------
    svn:keywords = Revision Id



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: svn commit: r668177 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/layoutengine/standard-testcases/

Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Why deprecate? It's not part of our external API. We can simply remove
the method if it's dangerous.

On 16.06.2008 17:28:01 Luca Furini wrote:
> On Mon, Jun 16, 2008 at 4:52 PM,  <lf...@apache.org> wrote:
> 
> > Fixing the PageBreakingAlgorithm, replacing calls to getLineWidth() with getLineWidth(int)
> > so  as to take into account each page's real height.
> > This fixes the positioning of footnotes when the page bpd is not the same for all pages.
> 
> This was a little nasty bug I stumbled upon a few days ago, and it
> took me some time to track back where the problem was ...
> 
> The PageBreakingAlgorithm, in particular the computeDifference()
> method, had some calls to getLineWidth() without parameters and some
> other with an int parameter indicating the page.
> When the page bpd changes from page to page, the two methods returned
> different values, with the effect that the algorithm first believed it
> could place a whole footnote in the page, and then found out that this
> led to an overflow.
> 
> In order to avoid similar problems, the parameter-less getLineWidth()
> method could maybe be deprecated?
> 
> Regards
>     Luca




Jeremias Maerki


Re: svn commit: r668177 - in /xmlgraphics/fop/trunk: ./ src/java/org/apache/fop/layoutmgr/ test/layoutengine/standard-testcases/

Posted by Luca Furini <lf...@cs.unibo.it>.
On Mon, Jun 16, 2008 at 4:52 PM,  <lf...@apache.org> wrote:

> Fixing the PageBreakingAlgorithm, replacing calls to getLineWidth() with getLineWidth(int)
> so  as to take into account each page's real height.
> This fixes the positioning of footnotes when the page bpd is not the same for all pages.

This was a little nasty bug I stumbled upon a few days ago, and it
took me some time to track back where the problem was ...

The PageBreakingAlgorithm, in particular the computeDifference()
method, had some calls to getLineWidth() without parameters and some
other with an int parameter indicating the page.
When the page bpd changes from page to page, the two methods returned
different values, with the effect that the algorithm first believed it
could place a whole footnote in the page, and then found out that this
led to an overflow.

In order to avoid similar problems, the parameter-less getLineWidth()
method could maybe be deprecated?

Regards
    Luca