On 13.06.2006 17:58:11 Vincent Hennebert wrote: > Hi, > > > ------- Additional Comments From [EMAIL PROTECTED] 2006-06-12 12:45 ------- > > (In reply to comment #0) > > > >>This patch isn't really meant to be applied... Rather to be reviewed by > >>interested parties to check if I'm not wrong. Changelog: > >>* javadocs for the Knuth line- and page-breaking algorithms. Some items are > >>marked with double question marks because I haven't found out yet what is > >>their > >>purpose. I will probably find eventually, but if anybody has immediate hints > >>they will be welcome. > > > > > > KnuthBlockBox: > > bpdim seems to be used in concert with the proprietary display-align="fill" > > value Luca implemented. See AbstractBreaker.optimizeLineLength(). If I > > understand it right it is somehow used to make sure all the pages have more > > or > > less the same amount of content (in bpd). > > OK. Actually this is the natural width (without stretching nor > shrinking) of the line represented by this box. This field apparently > exists because it isn't possible to get the min/opt/max values stored in > a MinOptMax object. Otherwise it could be retrieved from the opt of the > ipdRange field. It may perhaps be useful to add such methods to > MinOptMax?
Aha. But then we'd better rename the variable to a more speaking name. MinOptMax should only be used if we also use the min and max values because simple types are generally favored over objects (speed/memory). > > pos and bAux are defined in ListElement/KnuthElement. > > Hmmm. Does a Position object represent the index of the Knuth element > (here KnuthBlockBox) in the sequence managed by the corresponding LM? Yes. The Position is used so the LM knows later what part of the FO to paint in addAreas(). > What does it mean that a box is auxiliary? It's just a special marker that is used in certain areas. See: http://www.nabble.com/Re%3A-Knuth-algorithm-problem-p1045573.html > > > BreakingAlgorithm: > > alignment: EN_BEFORE is not used. EN_START is used instead, since the class > > is > > used in both ipd and bpd. EN_BEFORE is mapped into EN_START. Actually, > > alignment > > uses a slightly different set of value than the FO properties, so reusing > > the > > integer constants may not be the best thing, but we're not under Java 5, > > yet, > > where we could use enums. > > bFirst is used for the text-indent property so only the first paragraph of a > > block is indented. See block_text-indent.xml. > > You probably mean the first /line/ of a block? No, I mean paragraph. You can have multiple paragraphs in a block if line breaks survive or you have nested block-level elements. > > > partOverflowRecovery is used in page breaking to defer an element which > > would > > overflow the available BPD to the next page if it's the only element in a > > part > > (=line or page). > > I'm still a bit unsure of what 'part' means in the javadoc of > BreakingAlgorithm.isPartOverflowRecoveryActivated. A line/page? A > word/block? A Knuth box? A line or page depending on the direction the breaking algorithm works. I just needed to find a term to match "line" and "page". Maybe there's a better one. A part is a subsequence of Knuth elements formed by breaking decisions of the breaking algorithm. > > >>* some methods have been marked deprecated because AFAICT they are not > >>called > >>anywhere. If this is agreed I'll remove them in my next patch > > > > > > +1 > > > > > >>* bugfix? In the last for loop of the method > >>layoutmgr.PageBreakingAlgorithm.noBreakBetween I think the exit condition > >>should > >>be a strict comparison ('<' instead of '<='). Confirmation? > > > > > > not sure. :-( > > The code in the for loop checks if the element pointed to by index is a > legal breakpoint. If the exit comparison isn't strict index may reach > the value breakIndex, which by definition is a legal break. I guess the > purpose of the noBreakBetween method is to check that there is no legal > break between the two given breakpoints, /excluded/. The line > storedValue = (index == breakIndex) > would confirm that. > > > > > > > >>* the javadoc comments for some methods have been removed because they will > >>inherit them from their super-class > > > > > > I think Checkstyle will bark about that. If you do Ctrl-J in Eclipse, you > > get an > > automatic @see entry which satisfies Checkstyle. @inheritDoc does not work > > in > > every Java version. > > In fact checkstyle doesn't complain. It seems to be smart enough to > detect that there is a javadoc for the original version of a redefined > method. In such cases javadoc copies the definition from the > super-class, and that's also what Eclipse does in the tooltip. I may put > @see statements, but I think it doesn't really make sense. Ah ok, then maybe an older Checkstyle version complained about that. At least, I had something like that in my mind. > > > > > > >>* some checkstyle fixes > > > > > > HTH > > Updated patch follows. > Thanks, > Vincent Thanks, Vincent. I'll look at it right away. Jeremias Maerki
