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.
>>
>> 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
>
<?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 text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text<fo:footnote color="blue"> <fo:inline>1</fo:inline> <fo:footnote-body> <fo:block> First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. First footnote. </fo:block> </fo:footnote-body> </fo:footnote>. Some text. Some text. Some text. Some text<fo:footnote color="red"> <fo:inline>2</fo:inline> <fo:footnote-body> <fo:block> Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. Second footnote. </fo:block> </fo:footnote-body> </fo:footnote>. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. Some text. </fo:block> </fo:flow> </fo:page-sequence> </fo:root>
noBreakBetween.pdf
Description: Adobe PDF document
