[
https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13827905#comment-13827905
]
Vincent Hennebert commented on FOP-2293:
----------------------------------------
Hi Seifeddine,
thanks for your patch. I’ve partially applied it in [rev.
1543891|http://svn.apache.org/r1543891].
The approach is taking shape. A few comments:
In BestFitLayoutUtils:
* calculateLength: there’s no need to add a dummy penalty at the beginning of
the list. Yes, there are issues with space resolution, but if you add the
penalty you will run into some bugs and if you remove it you will run into
other bugs. The real solution to the problem is to change space resolution to
perform partial resolution on a subset of the Knuth sequence, and refine it
later on. But this is out of the scope of this work and can be left as is for
now (for that matter, footnotes have been suffering from the same problem for
years).
* getKnuthList: there’s no need to add an empty box before the BestFitPenalty.
A penalty doesn’t need to be preceded by a box to be a legal break point, only
a glue does.
In PageBreakingAlgorithm:
* The WhiteSpaceManager class in PageBreakingAlgorithm looks problematic to me.
Information about a page break can now be found at two places: in this class,
and in KnuthPageNode. This will inevitably create inconsistencies and
difficulties to manage state that is distributed over several entities. This is
illustrated by the fact that you have to manage a list of alternatives that
(IIUC) will depend on the selected break point. That list is already there in
the form of KnuthPageNodes! The KnuthPageNode class should just be augmented to
contain the selected variant of the multi-switch, if any.
* in computeDifference: I’m not too sure of the meaning of the added {{if
(element instanceof BestFitPenalty)}} test. What do you have in mind? Also, the
second test {{if (diff > 0 && dynamicContentBPD > 0 && !(element instanceof
BestFitPenalty))}}: is it to handle the case where a multi-switch is in the
middle of a page? In that case, you can’t simply deactivate a variant. It’s
perfectly possible to have one layout where the multi-switch is at the bottom
of a page, and one where it’s in the middle of a page. Which layout will be
selected in the end depends on what follows. The middle-of-page case will be
handled by inserting a glue at the right place in the Knuth sequence, but you
can leave that for now and concentrate on page breaks.
* in handleBestFitPenalty: again, just because in one case there is no space
for a variant doesn’t mean that there won’t be space in any other cases. See
uploaded file two-valid-variants.fo. Comment out the multi-switch and play with
the space-before.minimum value: if left to 20pt, the variant with 2 lines on
the first page is selected, if set to 15pt, the variant with 3 lines on the
first page is selected. Once you re-enable the multi-switch, that will have an
impact on which variant will be selected: the one-line variant in the first
case, the two-line variant in the second case.
I really encourage you to create a simple layout test to check your
implementation. This two-valid-variants example could be a good start. Also,
you may want to run a test coverage tool to ensure that the code you write is
actually run and tested.
Thanks,
Vincent
> Whitespace management extension
> -------------------------------
>
> Key: FOP-2293
> URL: https://issues.apache.org/jira/browse/FOP-2293
> Project: Fop
> Issue Type: New Feature
> Components: general
> Affects Versions: trunk
> Reporter: Seifeddine Dridi
> Priority: Minor
> Labels: XSL-FO
> Fix For: trunk
>
> Attachments: FO_multi-switch_best-fit_ext_rev2.patch,
> FO_multi-switch_best-fit_ext_rev3.patch,
> FO_multi-switch_best-fit_ext_rev4.patch,
> FO_multi-switch_best-fit_ext_rev5.patch, FO_multi-switch_test.fo,
> FO_multi-switch_with_best-fit_extension.patch, bestfit.fo, doc.pdf,
> multi-switch_bestfit.fo, multiple-feasible-nodes.fo, patch-rev1.1.patch,
> patch-rev1.patch, patch-rev2.patch, patch.patch
>
>
> I have been working on an extension for whitespace management, similar to
> what's described here:
> http://wiki.apache.org/xmlgraphics-fop/WhitespaceManagement
> The logic of the extension is very simple: the user defines a set of
> alternatives that he wishes to insert at the end of a page, then if there is
> enough space left, FOP will pick the alternative that best matches the user's
> selection criteria (first fit, smallest fit, biggest fit).
> This is my first work on FOP and it took me almost 2 months to reach this
> stage in development. But it's not the end of course, so I'm relying on your
> feedback to improve it.
> Thank you
--
This message was sent by Atlassian JIRA
(v6.1#6144)
