[ https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828626#comment-13828626 ]
Seifeddine Dridi commented on FOP-2293: --------------------------------------- {quote} 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. {quote} I don't see things like that. A penalty is used to separate lines/paragraphs, but what if an FO contains only a multi-switch. Without the empty box at the start, the layout manager would think that the best-fit penalty is useless and so will proceed to deleting it, since there is really nothing to "break". {quote} 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. {quote} I don't fully agree with you. While I think there might be inconsistencies between the information stored by the breaking algorithm and the WhiteSpaceManager class, I also believe that the type of information each one needs are not necessarily the same. I could have just augmented the KnuthNode like you said, but I have no control over the created Knuth nodes and they can be deleted/replaced at any time without my notice -- I prefer to leave my code separated from the Knuth algorithm and whenever an information is needed it will be requested-- that's why I created my own classes instead of relying on the existing ones. This will also ease maintenance later. {quote} in computeDifference: I’m not too sure of the meaning of the added if (element instanceof BestFitPenalty) test. What do you have in mind? {quote} Penalties which are too long (r < -1) are disabled very early, and they won't get registered in WhiteSpaceManager (see handleBestFitPenalty()). {quote} 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? {quote} It's used to detect if a normal content appeared after an already registered best-fit penalty, in which case we either have to pick another variant hoping that it will be long enough to force the algorithm to make a new page, or remove the currently active one altogether. {quote} 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. {quote} I'm aware of the potential problems with space-specifiers. Unfortunately, the space-resolution rules defined in the XSL spec are not trivial to understand, but I'm progressing nevertheless. Thanks for your review, I appreciate your effort. > 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, 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)