DO NOT REPLY [Bug 39777] [PATCH] GSoC: floats implementation
https://issues.apache.org/bugzilla/show_bug.cgi?id=39777 Glenn Adams gad...@apache.org changed: What|Removed |Added Status|ASSIGNED|NEW --- Comment #15 from Glenn Adams gad...@apache.org 2012-04-11 06:17:36 UTC --- change status from ASSIGNED to NEW for consistency -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-31 09:34 --- Applied the latest two patches to a temporary branch (Temp_Floats): http://svn.apache.org/viewvc?rev=427052view=rev I managed to reenable one of the disabled test cases because you were fooled by the default values for widows and orphans. Having only 3 lines in a block does not allow any break possibilities with default widows and orphans. 4 lines creates one break possibility in the middle. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
I managed to reenable one of the disabled test cases because you were fooled by the default values for widows and orphans. Having only 3 lines in a block does not allow any break possibilities with default widows and orphans. 4 lines creates one break possibility in the middle. Indeed, yes. Well, thanks! Vincent
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-31 10:25 --- Vincent, would you please add a testcase for the outstanding bug? * bug when the whole normal content is typeset and there are both footnotes and floats left to be placed; Whenever possible with reasonable effort, I create a testcase before I start bugfixing. That documents the bug with a concrete and debug-friendly example and lets you know when you've finished fixing the bug. Even if you don't fix the bug right away, it is still helpful, because anyone else can start on the bugfix right away if he/she has time. Test first, as in XP. Only good experiences so far. Note that adding a disabled testcase to the list automatically updates the website with known issues (at least after next site deployment, see http://xmlgraphics.apache.org/fop/knownissues.html). -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-31 18:08 --- Created an attachment (id=18666) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18666action=view) Disabled testcases for (known) non-working features; patch against the Temp_Floats branch. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
On 25.07.2006 18:59:00 Vincent Hennebert wrote: DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-23 19:47 --- I finally had a chance to take a first look. What I've seen so far looks pretty nice. A first simple test behaved as would be expected. Making a before-float too big to fit on a page (although there are break points inside the content) results in an OutOfMemoryError (probably due to an infinite loop). It would be good if you would write a set of test cases for before-floats as your next task. This is to document what works and what doesn't and to give you and us more confidence when doing further chances in the code. Testcases should be ready tomorrow. Saw them, thanks a lot! I'll take a look tomorrow afternoon or Saturday. Finally, would you compile a list of classes you propose to move into the breaking package? The idea itself is worth investigating since the layoutmgr package has already grown rather big again. On a general matter, I would put in the breaking subpackage all of the classes from layoutmgr and its subpackages which are related to the Knuth approach: the algorithm as well as the various Knuth elements. A quick look gave me the following classes: AbstractBreaker BalancingColumnBreakingAlgorithm BreakingAlgorithm BlockKnuthSequence InlineKnuthSequence KnuthBlockBox KnuthBox KnuthElement KnuthGlue KnuthPenalty KnuthPossPosIter KnuthSequence PageBreakingAlgorithm inline.KnuthInlineBox Ok, that's only strictly the pure Knuth stuff. There are a few other classes we could talk about relocating, too. I'll try to categorize: Element List Base class: ListElement (superclass for KnuthElement and UnresolvedListElement) Unresolved stuff: SpaceResolver UnresolvedListElement and subclasses Utilities: ElementList*.java AreaAdditionUtil LMiter MinOptMaxUtil TraitSetter block-level LMs: BlockLayoutManager BlockContainerLayoutManager outer-level LMs: Flow-, StaticContent-, PageSequence-LM LM-Infrastructure: the rest Maybe some of those classes could also be moved to some new subpackages to clean the whole thing up a little more. This will probably create many access problems (with currently package-private members). But this will be an opportunity to clean up the whole thing a bit, I think. Too much package-private access is bad anyway and is often done in order to avoid writing accessor methods. So I guess this will be a good thing. In a second step there is also a number of inner classes which might be extracted and transformed into a top-level class of the new breaking subpackage. I'm mainly thinking of inline.LineLayoutManager.LineBreakingAlgorithm. I guess there are reasons why they currently are inner classes, but it may be conceptually better anyway to separate them from their surrounding class. This would have to be studied more deeply. Yep, LineLayoutManager could probably do with some slimming down, too. As for when to apply the change, we may probably wait for the integration of Simon's work. I created the package just because I had a new class to put in it and thought that I might as well directly put it in the right package. I agree. No good throwing unnecessary stones in Simon's path. But, Simon, do you have an idea when you'll be ready? So, WDYT? What we need to decide now is whether to put the changes in a branch until they stabilize or if we put it in Trunk. I'd prefer a branch for now so in case I have time to finish the work on 0.93, we don't have any problems from this end. A branch would be fine I think, as this would allow me to submit more gradual patches. Good. I'll do that, then. ASAP. Damn busy week. And hot here. Tonight was the first thunderstorm in many days of pure sunshine. Sorry, I'm not more responsive this week. Jeremias Maerki
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-26 16:06 --- Created an attachment (id=18644) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18644action=view) Testcases for before-floats Testcases for basic features of before-floats. There are also some disabled testcases for features known to be broken. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-25 08:28 --- (In reply to comment #7) Thanks for your comments, Simon. A few answers: 2. Wiki page: The argument for an inner class in bullet 3 only refers to an object inner class. Therefore contradicts bullet 4, a static inner class, and is invalid. Well, the private fields of any ProgressInfo instance are directly accessible within OutOfLineRecord. This avoids using getter and setter methods everywhere in OutOfLineRecord, for the progressInfo field as well as the prevProgress parameters of the get[Footnote|Float]Split methods. That's what I meant. 3. fo:float seems to be the only FO that can be placed in inline and block content. This causes a problem because FOP distinguishes between inline and block LMs rather much. I do not think that this problem needs to be solved in this project. It should be fine to make an implementation which works for inline fo:float FOs. Ok, noted. On the tests I've done this seems to work, but I don't know if it will in all cases. I think the solving of this problem may be done together with the implementation of the keep-with-previous property for the anchor element. I put it on my TODO-list. 4. Infos is not correct english; better call it ProgressInfo. cumulatedLengths is better changed to cumulativeLengths. Thanks, always glad to improve my english ;-) 5. The implementation does not use the max and min property of the BPD of a float. In a text with little stretch and shrink, e.g. an adapted version of the test file footnote_basic.xml, there is no possibility to place the floats with less than infinite badness, and they are moved towards the end of the page sequence. Yes. If I'm correct the shrink/stretch capabilities of footnotes aren't considered either. More generally there is a problem with too short nodes (considered as such because no stretching is possible), which are handled separately from the other nodes. The algorithm may prefer a normal node with a deferred float instead of a too short node with the float on the same page. I'm thinking about an improved algorithm which would handle such cases. I think I will give some ideas on the Wiki page soon, after I've submitted testcases. But before implementing it I'll perhaps work on side-floats, don't know yet. 6. The OutOfLineRecord.getFootnoteSplit and OutOfLineRecord.getFloatSplit methods suggest that there is room for two subclasses. It would be nice if the two conditional blocks in PageBreakingAlgorithm.computeDifference, if (floats.existing()) and if (footnotes.existing()) could be merged, and the difference in the logic be moved to the footnotes and floats objects. But that is finishing touch. I agree (that some further refactoring is possible). As this part of the code may change if an improved algorithm is to be implemented, I didn't put too much effort in it. This will belong to a second refactoring step. Thanks, Vincent -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #18617|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2006-07-25 08:32 --- Created an attachment (id=18637) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18637action=view) The very same patch, with some english corrections as suggested by Simon Pepping * (OutOfLineRecord.)ProgressInfos renamed into ProgressInfo * (OutOfLineRecord.)cumulatedLengths renamed into cumulativeLengths -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-23 19:47 --- I finally had a chance to take a first look. What I've seen so far looks pretty nice. A first simple test behaved as would be expected. Making a before-float too big to fit on a page (although there are break points inside the content) results in an OutOfMemoryError (probably due to an infinite loop). It would be good if you would write a set of test cases for before-floats as your next task. This is to document what works and what doesn't and to give you and us more confidence when doing further chances in the code. Testcases should be ready tomorrow. Finally, would you compile a list of classes you propose to move into the breaking package? The idea itself is worth investigating since the layoutmgr package has already grown rather big again. On a general matter, I would put in the breaking subpackage all of the classes from layoutmgr and its subpackages which are related to the Knuth approach: the algorithm as well as the various Knuth elements. A quick look gave me the following classes: AbstractBreaker BalancingColumnBreakingAlgorithm BreakingAlgorithm BlockKnuthSequence InlineKnuthSequence KnuthBlockBox KnuthBox KnuthElement KnuthGlue KnuthPenalty KnuthPossPosIter KnuthSequence PageBreakingAlgorithm inline.KnuthInlineBox This will probably create many access problems (with currently package-private members). But this will be an opportunity to clean up the whole thing a bit, I think. In a second step there is also a number of inner classes which might be extracted and transformed into a top-level class of the new breaking subpackage. I'm mainly thinking of inline.LineLayoutManager.LineBreakingAlgorithm. I guess there are reasons why they currently are inner classes, but it may be conceptually better anyway to separate them from their surrounding class. This would have to be studied more deeply. As for when to apply the change, we may probably wait for the integration of Simon's work. I created the package just because I had a new class to put in it and thought that I might as well directly put it in the right package. So, WDYT? What we need to decide now is whether to put the changes in a branch until they stabilize or if we put it in Trunk. I'd prefer a branch for now so in case I have time to finish the work on 0.93, we don't have any problems from this end. A branch would be fine I think, as this would allow me to submit more gradual patches. Vincent
Re: DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
No need to worry about that one too much anymore. License headers are going to change (again) and the copyright year vanishes. Cliff Schmidt hasn't sent the info mail, yet, but the decision is official: http://www.apache.org/legal/src-headers.html BTW, if anyone wants to volunteer for doing the header change, there are nice ready-to-use scripts in the committers repo. :-) On 24.07.2006 22:21:38 bugzilla wrote: 1. Copyright year on new classes should be only 2006. Jeremias Maerki
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-23 19:47 --- I finally had a chance to take a first look. What I've seen so far looks pretty nice. A first simple test behaved as would be expected. Making a before-float too big to fit on a page (although there are break points inside the content) results in an OutOfMemoryError (probably due to an infinite loop). It would be good if you would write a set of test cases for before-floats as your next task. This is to document what works and what doesn't and to give you and us more confidence when doing further chances in the code. Finally, would you compile a list of classes you propose to move into the breaking package? The idea itself is worth investigating since the layoutmgr package has already grown rather big again. What we need to decide now is whether to put the changes in a branch until they stabilize or if we put it in Trunk. I'd prefer a branch for now so in case I have time to finish the work on 0.93, we don't have any problems from this end. Keep up the good work. This looks very promising. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-07-19 10:13 --- Created an attachment (id=18617) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18617action=view) First patch for before-floats implementation Basic implementation of before-floats. Known problems are: * bug when the whole normal content is typeset and there are both footnotes and floats left to be placed; * floats too big to even fit on one page alone are not handled yet; * the keep-with-previous property for the generated inline anchor-area is not implemented; * some clean-up in LayoutManagers should be possible; * others that I may have missed... Changelog: * Created a breaking subpackage where, if this is agreed, all classes related to the Knuth approach will eventually be placed. * Created an OutOfLineRecord class for dealing with both before-floats and footnotes. It is basically made of code extracted from PageBreakingAlgorithm that was related to footnote handling. Most of it could also be used for floats. (pending: documentation and testcases) -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
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
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |ASSIGNED --- Additional Comments From [EMAIL PROTECTED] 2006-06-14 07:49 --- (In reply to comment #3) Created an attachment (id=18456) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18456action=view) [edit] Updated patch, whith some additions and corrections I think this patch may be applied. It contains new javadoc comments and adds some precisions to previous ones. Deprecated elements have been removed. Patch applied. Thank you! Precious work for anyone who wants to dive in here. http://svn.apache.org/viewvc?rev=414135view=rev -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
Re: DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
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?
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?
What does it mean that a box is auxiliary?
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?
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?
* 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.
* some checkstyle fixes
HTH
Updated patch follows.
Thanks,
Vincent
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 [EMAIL PROTECTED] changed: What|Removed |Added Attachment #18447|0 |1 is obsolete|| --- Additional Comments From [EMAIL PROTECTED] 2006-06-13 16:03 --- Created an attachment (id=18456) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18456action=view) Updated patch, whith some additions and corrections I think this patch may be applied. It contains new javadoc comments and adds some precisions to previous ones. Deprecated elements have been removed. -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
DO NOT REPLY [Bug 39777] - [PATCH] GSoC: floats implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=39777. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=39777 --- Additional Comments From [EMAIL PROTECTED] 2006-06-11 16:00 --- Created an attachment (id=18447) -- (http://issues.apache.org/bugzilla/attachment.cgi?id=18447action=view) proposed patch -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug, or are watching the assignee.
