[ 
https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13834162#comment-13834162
 ] 

Vincent Hennebert commented on FOP-2293:
----------------------------------------

Hi Seifeddine,

thanks for your patch. I'd have to look at it in more details before applying 
it, but from a quick look I have the following comments:
* It would be better to store the selected variant on the BestPageRecords 
instead of a separate map, the same way as footnotes are handled. You would 
just have a {{variant}} field in PageBreakingAlgorithm that would be reset at 
the beginning of computeDifference, and set to the proper value in 
handleBestFitPenalty. Then you add a parameter to the KnuthPageNode constructor 
for the variant, and you should be done.
* 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.
* in handleBestFitPenalty: the idea is there, but some adjustments will be 
needed:
** you must use the computeDifference and computeAdjustmentRatio methods from 
PageBreakingAlgorithm and not BreakingAlgorithm, because you must take 
footnotes into account. You could move the content of computeDifference into a 
new method that will be called by both computeDifference and 
handleBestFitPenalty. computeDifference would then just contain the test 
whether we are dealing with a special penalty or not.
** I thought the idea was to select the first variant that fits? In which case 
you can break out of the loop as soon as you find an adjustmentRatio >= -1
* 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?

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_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)

Reply via email to