subject:"Footnotes in the float branch"

Re: Footnotes in the float branch

2007-03-27 Thread Vincent Hennebert

Hi Luca,

Luca Furini a écrit :
Hi all

I recently had the time (and the pleasure) to look at before-float
implementation branch, and I played a bit with it.

I focused on the handling of footnotes, as I noticed that sometimes they
were placed on a page following their citations without a real necessity
to do it; as I wrote some time ago (and I rememeber there was some
consesuns on this) this behaviour is acceptable for before floats, but
is probably not what a user would expect for footnotes.

I have tried to fix this in the PageBreakingAlgorithm, computing a
minimum required index for footnotes, so that no page break will be
considered that unnecessarily defers some old footnotes to the next page.

I'm attaching a diff file showing the changes (or maybe should I just
apply it?); after applying the patch, there are 4 more passing testcases
(foonote_footnote-separator, footnote_large, footnote_positioning_{4,5})
and no regressions. Testcases footnote_positioning_{2,3} still generate
some run-time exception, and in the next days I'm going to see what's
wrong with them.

I add just a few comments about the new classes: I must admit that it
took me a while to see and understand the interaction between the
PageBreakingAlgorithm and the Footnotes / BeforeFloats Record, together
with their inner Footnotes / BeforeFloats Progress.

In particular, at the beginning I thought the *Progress classes were
just convenience classes to get pieces of footnotes and floats without
directly fiddling with element lists, and I found only later that their
methods can actually create new active nodes.

Actually all the node creation logic lies in the ActiveNodeRecorder
class (handleNode method). But it is true that the *Progress classes
make calls to this method and that it may be confusing.

Another thing that I find a bit strange is that the
PageBreakingAlgorithm does not directly interact with the before floats,
as the calls to BeforeFloatsProgress.consider() are hidden in the
FootnotesProgress class.

Yes and that's unfortunate. I realized that only later on. The rationale
was to extract the handling of footnotes and before-floats from the
PageBreakingAlgorithm class, which is already large enough, and to have
dedicated classes for that. Among other things PageBreakingAlgorithm
wouldn't have to bother adding the footnote/before-float separator or
not, when, etc.

So, I was wondering whether it wouldn't be more clear to have the
PageBreakingAlgorit control all the node creation logic, after having
accessed information about footnotes and floats that could be placed in
the page via the helper classes.

Yes, something like that. Eventually we would have a layout engine
taking elements from several flows (normal flow, footnotes,
before-floats, side-floats...) and wiring all those flows in a proper
way.

WDYT?

I had a look at your patch and have several comments:
- I see you re-enabled the noBreakBetween method; I don't think it's
a good solution because it artificially prevents some nodes to be
created, which even if bad may be necessary for some complex
documents. See for example the attached fo file. I also documented
a similar problem on the wiki [1]. While it makes the testcases work
it actually creates some bad layout in other cases.
- I'm a bit reluctant about the newFootnotesCount field as it
re-introduces footnotes-related code in the PageBreakingAlgorithm.
Likewise, the findMinimumFootnoteIndex shouldn't be in the
PageBreakingAlgorithm class, if any.
- there are checkstyle issues in your patch (tab characters, lines too
long, missing javadocs)

My feeling is that the Knuth algorithm can nicely handle such problems
already as is. It's just a matter of defining the right demerits for
deferred footnotes, and give a chance to too-short nodes with
non-deferred footnotes to be considered WRT normal nodes with deferred
ones. I seem to remember that there was also a problem with flushing
floats on the last page (footnotes were unnecessarily deferred). I'd
have to dig deeper into that. I'll try to illustrate my ideas in a patch
in the next days.

Cheers,
Vincent

[1]
http://wiki.apache.org/xmlgraphics-fop/GoogleSummerOfCode2006/FloatsImplementationProgress/ImplementingBeforeFloats#head-40ade416f873071dac75bd80dbd57c592efa3277

Re: Footnotes in the float branch

2007-03-27 Thread Vincent Hennebert

Grmblmblm... and the attached fo file, of course...

Vincent

Vincent Hennebert a écrit :
Hi Luca,

Luca Furini a écrit :
Hi all

I recently had the time (and the pleasure) to look at before-float
implementation branch, and I played a bit with it.

Actually all the node creation logic lies in the ActiveNodeRecorder
class (handleNode method). But it is true that the *Progress classes
make calls to this method and that it may be confusing.

WDYT?

Cheers,
Vincent

[1]
http://wiki.apache.org/xmlgraphics-fop/GoogleSummerOfCode2006/FloatsImplementationProgress/ImplementingBeforeFloats#head-40ade416f873071dac75bd80dbd57c592efa3277

?xml version=1.0 encoding=UTF-8?
fo:root xmlns:fo=http://www.w3.org/1999/XSL/Format;
fo:layout-master-set
fo:simple-page-master master-name=default
page-width=12cm page-height=5cm
fo:region-body/
/fo:simple-page-master
/fo:layout-master-set
fo:page-sequence master-reference=default
fo:static-content flow-name=xsl-footnote-separator
fo:block___/fo:block
/fo:static-content
fo:flow flow-name=xsl-region-body
widows=1 orphans=1
fo:block
Some

Re: Footnotes in the float branch

2007-03-27 Thread Luca Furini


Vincent Hennebert wrote:


Hi Luca,


Hi!


I had a look at your patch and have several comments:
- I see you re-enabled the noBreakBetween method; I don't think it's
  a good solution because it artificially prevents some nodes to be
  created, which even if bad may be necessary for some complex
  documents. See for example the attached fo file.


Right, it is quite an unlucky document!

Anyway, I still think that a footnote should be placed in a page following 
its citation only if there isn't really any other option: for example, the 
citation is inside a large block of unbreakable text, and the footnote 
itself is a large unbreakable block, and their cumulative height is taller 
than the page height (a situation that will surely happen sooner or later 
;-) , but is quite more unlikely than your example).


I think your example would not look so bad in the context of a page with 
some book-like width and height: yes, there would be quite a large space 
between the last content line and the first footnote line, but I think 
many users would prefer such an output to one having the footnote placed 
in the following page.



I also documented
  a similar problem on the wiki [1]. While it makes the testcases work
  it actually creates some bad layout in other cases.


The one in the 4.1 / footnote section? It's a very interesting one, 
although I think it's quite another story.


While in the previous example we have two valid options, and the algorithm 
chooses the ugliest one, here we have the algorithm a priori discarding 
the option that would be the best one. I think we could call this a bug, 
as there can be no doubt concerning what a user would expect.


I'm attaching [check; double check; look again; yes, it's there!] a 
testcase showing this kind of layout problem. Trunk leaves an empty space 
between the last content line and the first footnote one, the float branch 
places two more content lines, filling the empty space, and the patched 
branch behaves the same way.



[snip on the other good remarks]

My feeling is that the Knuth algorithm can nicely handle such problems
already as is. It's just a matter of defining the right demerits for
deferred footnotes, and give a chance to too-short nodes with
non-deferred footnotes to be considered WRT normal nodes with deferred
ones.


Demerits could not be enough: if there isn't any object with some stretch 
or shrink and the footnotes / floats do not fit exactly in the page but 
the content lines do, too-short nodes will only be considered when there 
is a restart and there isn't any deactivated node. Maybe we should be less 
restrictive on the ratio-based selection criterion.



I seem to remember that there was also a problem with flushing
floats on the last page (footnotes were unnecessarily deferred). I'd
have to dig deeper into that. I'll try to illustrate my ideas in a patch
in the next days.


Ok, I'm looking forward to see it!

Regards
Luca

footnote_positioning_6.xml
Description: application/xml


footnote_positioning_6.patched.pdf
Description: Adobe PDF document

Footnotes in the float branch

2007-03-26 Thread Luca Furini


Hi all

I recently had the time (and the pleasure) to look at before-float 
implementation branch, and I played a bit with it.


I focused on the handling of footnotes, as I noticed that sometimes they 
were placed on a page following their citations without a real necessity 
to do it; as I wrote some time ago (and I rememeber there was some 
consesuns on this) this behaviour is acceptable for before floats, but is 
probably not what a user would expect for footnotes.


I have tried to fix this in the PageBreakingAlgorithm, computing a 
minimum required index for footnotes, so that no page break will be 
considered that unnecessarily defers some old footnotes to the next page.


I'm attaching a diff file showing the changes (or maybe should I just 
apply it?); after applying the patch, there are 4 more passing testcases 
(foonote_footnote-separator, footnote_large, footnote_positioning_{4,5}) 
and no regressions. Testcases footnote_positioning_{2,3} still generate 
some run-time exception, and in the next days I'm going to see what's 
wrong with them.


I add just a few comments about the new classes: I must admit that it took 
me a while to see and understand the interaction between the 
PageBreakingAlgorithm and the Footnotes / BeforeFloats Record, together 
with their inner Footnotes / BeforeFloats Progress.


In particular, at the beginning I thought the *Progress classes were just 
convenience classes to get pieces of footnotes and floats without 
directly fiddling with element lists, and I found only later that their 
methods can actually create new active nodes.


Another thing that I find a bit strange is that the PageBreakingAlgorithm 
does not directly interact with the before floats, as the calls to 
BeforeFloatsProgress.consider() are hidden in the FootnotesProgress 
class.


So, I was wondering whether it wouldn't be more clear to have the 
PageBreakingAlgorit control all the node creation logic, after having 
accessed information about footnotes and floats that could be placed in 
the page via the helper classes.


WDYT?

Regards
Luca

Re: Footnotes in the float branch

2007-03-26 Thread Luca Furini


On Mon, 26 Mar 2007, Luca Furini wrote:


I'm attaching a diff file showing the changes 


Well, *now* I'm attaching bla bla :-)

Regards
LucaIndex: src/java/org/apache/fop/layoutmgr/breaking/FootnotesRecord.java
===
--- src/java/org/apache/fop/layoutmgr/breaking/FootnotesRecord.java 
(revision 521755)
+++ src/java/org/apache/fop/layoutmgr/breaking/FootnotesRecord.java 
(working copy)
@@ -91,6 +91,21 @@
 addSeparator();
 }
 }
+
+/**
+ * 
+ */
+public void handleDeferredFootnotes(int requestedLastIndex) {
+   boolean separatorAlreadyAdded = (alreadyInserted.getLength()  
0);
+   // check if we must add more footnotes
+   while (lastInsertedIndex  requestedLastIndex) {
+   next();
+   }
+   // if needed, add the separator
+   if (!separatorAlreadyAdded  alreadyInserted.getLength()  0) {
+   addSeparator();
+   }
+}
 
 /**
  * If the current page is a float-only page, handles the splitting of 
the last
Index: src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java
===
--- src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java(revision 
521755)
+++ src/java/org/apache/fop/layoutmgr/BreakingAlgorithm.java(working copy)
@@ -545,6 +545,17 @@
 log.debug(Could not find a set of breaking points  + 
threshold);
 return 0;
 }
+// lastDeactivated was a good break, while lastTooShort and 
lastTooLong 
+// were bad breaks since the beginning;
+// if it is not the node we just restarted from, 
lastDeactivated can 
+// replace either lastTooShort or lastTooLong
+if (lastDeactivated != null  lastDeactivated != lastForced) {
+if (lastDeactivated.adjustRatio  0) {
+lastTooShort = lastDeactivated;
+} else {
+lastTooLong = lastDeactivated;
+}
+}
 if (lastTooShort == null || lastForced.position == 
lastTooShort.position) {
 if (isPartOverflowRecoveryActivated()) {
 if (this.lastRecovered == null) {
Index: src/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java
===
--- src/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java 
(revision 521755)
+++ src/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java 
(working copy)
@@ -1285,7 +1285,12 @@
 (new KnuthGlue(lineStartBAP, 0, 0,
new LeafPosition(this, -1), false));
 } else {
+// the first penalty is necessary in order to avoid the glue 
to be a feasible break
+// while we are ignoring hyphenated breaks
 hyphenElements.add
+(new KnuthPenalty(0, KnuthElement.INFINITE, false,
+new LeafPosition(this, -1), false));
+hyphenElements.add
 (new KnuthGlue(0, 3 * 
LineLayoutManager.DEFAULT_SPACE_WIDTH, 0,
 new LeafPosition(this, -1), false));
 hyphenElements.add
Index: src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
===
--- src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
(revision 521755)
+++ src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
(working copy)
@@ -77,7 +77,7 @@
 /**
  * Are footnotes-only pages allowed?
  */
-public static final boolean FOOTNOTES_ONLY_PAGES_ALLOWED = true;
+public static final boolean FOOTNOTES_ONLY_PAGES_ALLOWED = false;
 
 /**
  * Additional demerits for an underfull page, which however has an 
acceptable fill ratio.
@@ -115,6 +115,14 @@
 private BeforeFloatsRecord beforeFloatsRecord;
 private FootnotesRecord.FootnotesProgress footnotesProgress;
 private BeforeFloatsRecord.BeforeFloatsProgress beforeFloatsProgress;
+// number of new footnotes met since the last feasible break
+private int newFootnotesCount = 0;
+// the method noBreakBetween(int, int) uses these variables 
+// to store parameters and result of the last call, in order
+// to reuse them and take less time
+private int storedPrevBreakIndex = -1;
+private int storedBreakIndex = -1;
+private boolean storedValue = false;
 
 private ActiveNodeRecorder activeNodeRecorder = new ActiveNodeRecorder();
 
@@ -682,6 +690,7 @@
 if (box instanceof KnuthBlockBox

Re: Footnotes in the float branch

Re: Footnotes in the float branch

Re: Footnotes in the float branch

Footnotes in the float branch

Re: Footnotes in the float branch

5 matches

Site Navigation

Mail list logo

Footer information