Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
[Manuel] Agree, and I have a solution for that ready to go. I think this deserves some further comment / discussion. One, IMO important, reason I want to do the evaluation during the FO parsing stage is that once we are in the LMs we lost the property inheritance information. That is something like: fo:block font-size=120% fo:blockText/fo:block /fo:block looks to the LM like: fo:block font-size=120% fo:block font-size=120%Text/fo:block /fo:block True, but the percentbase of the second 120% should be the same as the base of the first. They are both 120% of a value on fo:flow. So when evaluated, the result values will be the same. All inheritance must be resolve by the property system. Otherwise it is a bug. The reason for keeping the values as percentage is to support font-size=120% + 1pt and to *really* open the discussion it should be noted that the spec does *not* require additive operation on relative lengths. Only multiplications are required. Once the expression engine in fop tried to use that by calculating a factor to be applied to relative lengths. Does svn view have an attic? http://svn.apache.org/viewcvs.cgi/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/ If it does I could propably find the code, it wasn't pretty. regards, finn
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
On Tue, 30 Aug 2005 04:05 pm, Finn Bock wrote: [Manuel] Agree, and I have a solution for that ready to go. I think this deserves some further comment / discussion. One, IMO important, reason I want to do the evaluation during the FO parsing stage is that once we are in the LMs we lost the property inheritance information. That is something like: fo:block font-size=120% fo:blockText/fo:block /fo:block looks to the LM like: fo:block font-size=120% fo:block font-size=120%Text/fo:block /fo:block True, but the percentbase of the second 120% should be the same as the base of the first. Yes, but this is exactly the problem. The real percentbase, that is the actual value to use, is determined in the general case during layout. So the LM just sees 'font-size=120%' and now has to figure out the base to use. But it doesn't know that the 120% was actually inherited in the 2nd case and that it would need to backtrack to the parent to determine the base from there. They are both 120% of a value on fo:flow. So when evaluated, the result values will be the same. All inheritance must be resolve by the property system. Otherwise it is a bug. The reason for keeping the values as percentage is to support font-size=120% + 1pt and to *really* open the discussion it should be noted that the spec does *not* require additive operation on relative lengths. Only multiplications are required. I wouldn't worry about that. The current system handles it fine. Once the expression engine in fop tried to use that by calculating a factor to be applied to relative lengths. Does svn view have an attic? http://svn.apache.org/viewcvs.cgi/xmlgraphics/fop/trunk/src/java/org/ apache/fop/fo/ If it does I could propably find the code, it wasn't pretty. regards, finn Manuel
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Manuel Mall wrote: PS: I can imagine Victor smiling now :-) : I told you so that not leaving all property resolution to the LM stage is going to cause trouble. Not just Victor, Manuel. But please press on re-discovering the wheel. Peter smime.p7s Description: S/MIME Cryptographic Signature
RE: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Manuel Mall wrote: PS: I can imagine Victor smiling now :-) : I told you so that not leaving all property resolution to the LM stage is going to cause trouble. Trust me, I'm not smiling. As one of our U.S. Presidents was known for saying, I feel your pain. In fact, it might be fairly said that FOray exists because I have a lower toleration for such pain than the other FOP developers. Victor Mote
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
On Tue, 30 Aug 2005 08:43 pm, Victor Mote wrote: Manuel Mall wrote: PS: I can imagine Victor smiling now :-) : I told you so that not leaving all property resolution to the LM stage is going to cause trouble. Trust me, I'm not smiling. As one of our U.S. Presidents was known for saying, I feel your pain. In fact, it might be fairly said that FOray exists because I have a lower toleration for such pain than the other FOP developers. Fair enough, btw its not painful for me at all. I have joined the project with the goal to help to get FOP 1.0 out of the door in a relatively short timeframe (meaning months not years). To achieve that I am more than prepared to be pragmatic in the sense of getting things working satisfactorily than getting things perfect and at their most elegant. That doesn't mean I want to hack and slash. Good design is very important me. But there is still this goal to aim for as well and compromise may be in order at times. And isn't there still FOP 2.0 down the track for another round of serious redesign and refactoring if so desired? Hope this makes sense - especially to the active committers. Victor Mote Manuel
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Manuel Mall wrote: Fair enough, btw its not painful for me at all. I have joined the project with the goal to help to get FOP 1.0 out of the door in a relatively short timeframe (meaning months not years). To achieve that I am more than prepared to be pragmatic in the sense of getting things working satisfactorily than getting things perfect and at their most elegant. That doesn't mean I want to hack and slash. Good design is very important me. But there is still this goal to aim for as well and compromise may be in order at times. This is an excellent goal. Sometimes it is all too easy to aim for 100% perfection and then FOP gets stuck - just like it has been for 2 years prior to 2005. Compromise is the key to getting 1.0 out of the door. And isn't there still FOP 2.0 down the track for another round of serious redesign and refactoring if so desired? Hope this makes sense - especially to the active committers. Perfect sense. Keep up the great contributions! Chris
RE: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Chris Bowditch wrote: This is an excellent goal. Sometimes it is all too easy to aim for 100% perfection and then FOP gets stuck - just like it has been for 2 years prior to 2005. Compromise is the key to getting 1.0 out of the door. This is grossly foul. I don't remember anyone striving for 100% perfection when FOP was stuck. FOP got stuck in large part because 1) it first forked itself (no pun intended), and 2) the perceived leader of the project, who was to make that strategy work, suddenly dropped completely off of the edge of the world. The *ability* to compromise and to allow for multiple solutions to the same problem was one of the driving forces behind the modularization effort. Arggh. I have no problem with you guys sailing your ship however you see fit. But I reserve the right to set the record straight when my own efforts are misrepresented. Victor Mote
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Victor Mote wrote: Chris Bowditch wrote: This is an excellent goal. Sometimes it is all too easy to aim for 100% perfection and then FOP gets stuck - just like it has been for 2 years prior to 2005. Compromise is the key to getting 1.0 out of the door. This is grossly foul. I don't remember anyone striving for 100% perfection when FOP was stuck. FOP got stuck in large part because 1) it first forked itself (no pun intended), and 2) the perceived leader of the project, who was to make that strategy work, suddenly dropped completely off of the edge of the world. The *ability* to compromise and to allow for multiple solutions to the same problem was one of the driving forces behind the modularization effort. Arggh. I have no problem with you guys sailing your ship however you see fit. But I reserve the right to set the record straight when my own efforts are misrepresented. Sorry - that was not intended. I never had any opposition to what you tried to do, but I did feel some people objected to ideas because they didn't yield 100% compliance. Regardless, my main point in this thread was simply that compromise is the key to getting to a release. Which is one of your points above, right? Chris
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Chris Bowditch wrote: Manuel Mall wrote: Fair enough, btw its not painful for me at all. I have joined the project with the goal to help to get FOP 1.0 out of the door in a relatively short timeframe (meaning months not years). To achieve that I am more than prepared to be pragmatic in the sense of getting things working satisfactorily than getting things perfect and at their most elegant. That doesn't mean I want to hack and slash. Good design is very important me. But there is still this goal to aim for as well and compromise may be in order at times. This is an excellent goal. Sometimes it is all too easy to aim for 100% perfection and then FOP gets stuck - just like it has been for 2 years prior to 2005. Compromise is the key to getting 1.0 out of the door. Couldn't let that one go by, Chris. These issues, which were critical to the creation of both Folio and FOray, were being thrashed out around the beginning of 2003. You've been around long enough to know that, haven't you? So for more than two years, the pragmatic Fop developers stuck their heads in the sand. The project stalled. Point the finger at those who refused to see what is now coming to the surface, and what will still have to be solved. How pragmatic. And isn't there still FOP 2.0 down the track for another round of serious redesign and refactoring if so desired? Hope this makes sense - especially to the active committers. Perfect sense. Keep up the great contributions! Perfect, in some imperfect sense of the word. Chris Peter -- Peter B. West http://cv.pbw.id.au/ Folio http://defoe.sourceforge.net/folio/ smime.p7s Description: S/MIME Cryptographic Signature
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Peter B. West wrote: snip/ Couldn't let that one go by, Chris. These issues, which were critical to the creation of both Folio and FOray, were being thrashed out around the beginning of 2003. You've been around long enough to know that, haven't you? So for more than two years, the pragmatic Fop developers stuck their heads in the sand. The project stalled. Point the finger at those who refused to see what is now coming to the surface, and what will still have to be solved. How pragmatic. Yes I was around back in 2003. My apologies for offending anyone I wasn't clear enough with my offhand remarks. And yes I have buried my head in the sand as you put it. Not because I wanted to, but because my employers expect me to spend time elsewhere. I'm not going to add any more to this discussion, as I don't want to cause any further offense. Chris
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Chris Bowditch wrote: Peter B. West wrote: snip/ Couldn't let that one go by, Chris. These issues, which were critical to the creation of both Folio and FOray, were being thrashed out around the beginning of 2003. You've been around long enough to know that, haven't you? So for more than two years, the pragmatic Fop developers stuck their heads in the sand. The project stalled. Point the finger at those who refused to see what is now coming to the surface, and what will still have to be solved. How pragmatic. Yes I was around back in 2003. My apologies for offending anyone I wasn't clear enough with my offhand remarks. And yes I have buried my head in the sand as you put it. Not because I wanted to, but because my employers expect me to spend time elsewhere. I'm not going to add any more to this discussion, as I don't want to cause any further offense. Chris No offence taken. Peter -- Peter B. West http://cv.pbw.id.au/ Folio http://defoe.sourceforge.net/folio/ smime.p7s Description: S/MIME Cryptographic Signature
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
[Jeremias Maerki on ] Yeah: Wow! Finn, you're the specialist here. Can you take the lead on this one? I'm hardly a specialist on the layout system, and that is where a good sized part of Manuels patch is, but in the property system I do not like the hack to LineHeightPropertyMaker and FontSizePropertyMaker that test for a PercentLength since it will not work for expressions. I assume that a FixedLength is returned because somebody (BlockLM.createLineManager?) tries to resolve line-height before a context is available. The solution is to delay resolution of line-height, by passing the Length into the LineLM. Eventually the no-args getValue() and getNumericValue() should be removed. And a throws PropertyException added to the Length.getValue() signature. But that is for later. In the layout system, I find to strange to see calls to the initialize() method from addAreas() and getParentArea(). I would rather see a public initialize() method that was called once from the outside, perhaps from AbstractLayoutManager.getChildLM() before a LM is traversed by getNextKnuthElements. But not all agree: http://marc.theaimsgroup.com/?l=fop-devm=111811772615007w=2 Other than that, the patch looks ok. There is one thing I would have made differently: Instead of setting an PercentBase.XXX integer on the PropertyMakers I would set an instance of an implementation of PercentBase that implemented the rule by that property. So there would be a subclass of PercentBase for each of the rules you have identified in the PropertyHandling/Percentages wiki. That way the code for dealing with percentage rules can be placed in the property package instead of the LM package. The LM package only supplied the context that the rules can work on. But since I haven't written that code, I don't get to decide. regards, finn
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
Finn, thanks a lot for looking at this. On Tue, 30 Aug 2005 03:19 am, Finn Bock wrote: [Jeremias Maerki on ] Yeah: Wow! Finn, you're the specialist here. Can you take the lead on this one? I'm hardly a specialist on the layout system, and that is where a good sized part of Manuels patch is, but in the property system I do not like the hack to LineHeightPropertyMaker and FontSizePropertyMaker that test for a PercentLength since it will not work for expressions. I assume that a FixedLength is returned because somebody (BlockLM.createLineManager?) tries to resolve line-height before a context is available. The solution is to delay resolution of line-height, by passing the Length into the LineLM. Agree, and I have a solution for that ready to go. Eventually the no-args getValue() and getNumericValue() should be removed. And a throws PropertyException added to the Length.getValue() signature. But that is for later. There are quite a few legal situations where a context is not needed. It is a arguable if calling getValue(null) is superior to getValue(). In the layout system, I find to strange to see calls to the initialize() method from addAreas() and getParentArea(). I would rather see a public initialize() method that was called once from the outside, perhaps from AbstractLayoutManager.getChildLM() before a LM is traversed by getNextKnuthElements. But not all agree: http://marc.theaimsgroup.com/?l=fop-devm=111811772615007w=2 In this case I went with what was there. Other than that, the patch looks ok. There is one thing I would have made differently: Instead of setting an PercentBase.XXX integer on the PropertyMakers I would set an instance of an implementation of PercentBase that implemented the rule by that property. So there would be a subclass of PercentBase for each of the rules you have identified in the PropertyHandling/Percentages wiki. That way the code for dealing with percentage rules can be placed in the property package instead of the LM package. The LM package only supplied the context that the rules can work on. But since I haven't written that code, I don't get to decide. I did consider that. One reason I didn't do that was to avoid cross linking the packages. The rules which require navigation to an ancestor do in this implementation require access to methods in the LMs. By having this in the Property packages would mean importing LM stuff into the Property packages. Something I was trying to avoid. regards, finn
Re: DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
On Tue, 30 Aug 2005 08:27 am, Manuel Mall wrote: Finn, thanks a lot for looking at this. On Tue, 30 Aug 2005 03:19 am, Finn Bock wrote: [Jeremias Maerki on ] snip/ I'm hardly a specialist on the layout system, and that is where a good sized part of Manuels patch is, but in the property system I do not like the hack to LineHeightPropertyMaker and FontSizePropertyMaker that test for a PercentLength since it will not work for expressions. I assume that a FixedLength is returned because somebody (BlockLM.createLineManager?) tries to resolve line-height before a context is available. The solution is to delay resolution of line-height, by passing the Length into the LineLM. Agree, and I have a solution for that ready to go. I think this deserves some further comment / discussion. One, IMO important, reason I want to do the evaluation during the FO parsing stage is that once we are in the LMs we lost the property inheritance information. That is something like: fo:block font-size=120% fo:blockText/fo:block /fo:block looks to the LM like: fo:block font-size=120% fo:block font-size=120%Text/fo:block /fo:block but that is clearly incorrect as we should inherit the calculated value not the specified value. If we do the resolution at FO parsing time the LM see: !-- assuming we have a 12pt base font-- fo:block font-size=14.4pt fo:block font-size=14.4ptText/fo:block /fo:block and all is fine. The same applies to line-height. Luckily the vast majority of properties which allow percentage specification don't inherit by default (exceptions are: provisional-label-separation and provisional-distance-between-starts). However, most properties allow the inherit keyword. This will currently give wrong results, e.g.: fo:block margin=10% padding=5pt fo:block margin=inheritText/fo:block /fo:block would be seen by the LM as: fo:block margin=10% padding=5pt fo:block margin=10%Text/fo:block /fo:block resulting in the wrong margin being applied to the inner block. This is a pretty hard problem to solve I think given that we have lost the information if a value is specified or inherited at the LM stage. However, it may(?) not occur very often in practice so I am tempted to ignore it for the time being. Manuel PS: I can imagine Victor smiling now :-) : I told you so that not leaving all property resolution to the LM stage is going to cause trouble. snip/ regards, finn Manuel
DO NOT REPLY [Bug 36379] New: - [PATCH] Revised percentage resolution system
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=36379. 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=36379 Summary: [PATCH] Revised percentage resolution system Product: Fop Version: 1.0dev Platform: Other OS/Version: other Status: NEW Severity: normal Priority: P2 Component: page-master/layout AssignedTo: fop-dev@xml.apache.org ReportedBy: [EMAIL PROTECTED] Wow, this is the big one (from my perspective any way). See http://marc.theaimsgroup.com/?t=11243792742r=1w=2 for some background info. This patch affects a lot of classes (most in a minor way) especially in the LM section. It passes all layout engine tests and I have compared the area trees with the current trunk code. I therefore sincerely hope it is not a step backwards. There are also some steps forward. Percentages on leader-length and leader-pattrern-width should work. background-position-x/y percentages should also work in a generic way. Percentages relative to cell dimensions should be ok. There is more work to be done in this area. This is not the final word on percentages. But because of its size I would like to get this reviewed and if accepted put into SVN so my code base doesn't branch too badly from the main stream. BTW, I haven't got rid completely of the LayoutDimension stuff. It is still used for tables columns with proportional-column-width() specs. This still needs to be looked at. -- 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.
