[
https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13837021#comment-13837021
]
Vincent Hennebert commented on FOP-2293:
----------------------------------------
Hi Seifeddine,
this is looking good. Just a few things:
{quote}
bq. This dynamicNode boolean doesn't seem necessary to me. Why would you prefer
a node to another just because it corresponds to a multi-switch? It's better to
rely on the progress made in the Knuth sequence like is done in the default
implementation.
That perplexed me for a while. For instance, if we have 2 possible layouts: one
where the 1st variant of the dynamic content doesn't fit and another case where
the 2nd variant fits perfectly. The first possibility should have fewer total
demerits than the second one. Which one should we choose?
{quote}
Not sure what you mean here. If the 1st variant doesn't fit... well it doesn't
fit :-) Did you mean, where the 1st variant _does_ fit?
I might see where you're going though: what if with one active node, it's the
1st variant that fits, and with another active node, it's the 2nd? I see two
possibilities: either select only one best node (per fitness class), that is
the one with the fewest total demerits. Or select a best node per fitness class
_and_ per selected variant. But that will increase the number of active nodes,
and in practice might have little use.
For now, I would remove that dynamicNode boolean and compute demerits as usual.
{quote}
bq. findBestFitPenalty: not sure why this method is necessary. If a node
contains a non-null variant, that necessarily means that the element pointed to
by the position field is the corresponding BestFitPenalty?
Not necessarily. First, because the par member variable is susceptible to
change, hence invalidating the position field in the Knuth node. Second, there
is an infinitely stretching glue at the end of the Knuth sequence which makes
the algorithm create a new finalizing node.
{quote}
I don't have any case in mind where the par variable is being modified _while_
the breaking algorithm is run? As to the second point, if the node doesn't
point to the special penalty, that means that the multi-switch doesn't end the
page. So it shouldn't be taken into account, not through the penalty at least.
We are in the "middle of the page" case that we can tackle later on by adding a
glue "at the right place" in the Knuth sequence.
And a couple other things:
* why this resetCurrentVariant? Can you not just reset it at the beginning of
computeDifference?
* multi-switch_best-fit.xml: I know that it's possible to check on the Knuth
sequence, but in practice it's rarely done, and not ideal. We've had cases in
the past where, after a slight alteration in the Knuth element generation, we
had to re-visit all those test cases and update the checks, a rather painful
process. A unit test would probably be better suited, where you can mock
anything you want (using Mockito) and have more targeted checks. That said,
this test case is definitely a good start to add checks on the area tree.
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_best-fit_ext_rev6.patch,
> FO_multi-switch_best-fit_ext_rev7.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, two-valid-variants.fo
>
>
> 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)
