With 2 alternative s-p-m witch body have unequal left/right margins but equal i-p-d, I get an UnsupportedOperationException when content overflows on b-p-d axis. This doesn't occur when margins are the same.
Created attachment 26867 [details] use-case that demonstrates the issue
At first glance, this seems to be due to a rounding issue. The IPD for the first page's body region resolves to 510.237pt, while the second has only 510.236pt. As a result, the PageBreakingAlgorithm sees a difference, and restarts, while there is actually no reason to. A quick fix would be to only take into account differences greater than or equal to, say, 0.5pt. The 'proper' fix would likely involve a full investigation on where exactly this happens, and a battery of tests to make sure that equal widths in different units of measurement always evaluate to the exact same value in 1/1000pt.
(In reply to comment #2) > > A quick fix would be to only take into account differences greater than or > equal to, say, 0.5pt. To fix the rounding issue, was looking at fo.properties.FixedLength.convert(). There is a suspicious plain cast from double to int. In case of the first page we get the following values in mpt: page-width = 210mm = 595275(.5907) margin-left = 29mm = 82204(.72243) margin-right = 1mm = 2834(.64567) => 595275 - 82204 - 2834 = 510237 For the second: page-width = 210mm = 595275(.5907) margin-left = 30mm = 85039(.3701) margin-right = 0 => 595275 - 85039 = 510236 Rounding the values would also be wrong. In this case, that would simply flip the results. :-/ Scientists would reason: P1 = 595275(+1) - 82204(+1) - 2834(+1) = 510237(+/-3) P2 = 595275(+1) - 85039(+1) = 510236(+/-2) => P1 ~ P2, or: close enough However, even after eliminating the difference of 1mpt, I get a NullPointerException. Could be my local copy, though. Will retry with a fresh trunk later...
> Scientists would reason: > P1 = 595275(+1) - 82204(+1) - 2834(+1) = 510237(+/-3) > P2 = 595275(+1) - 85039(+1) = 510236(+/-2) Errm... Sleepy scientists. ;-) 510237(+/-1) and 510236(+/-0)
I have run into this before myself. For me an acceptable workaround was to change the units of left/right margins to match and avoid the rounding errors, e.g. instead of left-margin="10pt" right-margin="10mm" I changed the units to match left-margin="10pt" right-margin="30pt" The rounding errors are then avoided and the changing IPD algorithm is not started. It will be difficult to avoid rounding errors when the user specifies different units on the different measurements involved. The idea of using a within 1pt rule seems like a bit of hack to me...
(In reply to comment #5) > I have run into this before myself. For me an acceptable workaround was to > change the units of left/right margins to match and avoid the rounding errors, In this case, even using "mm" everywhere leads to the issue, and it will be hard to fix this so it works for all cases, unless we either start storing the mpt values as double, and truncate only the result, or calculate with the original value, including the units. In that case, it would work here, too: 210 - 30 = 210 - 1 - 29 = 180. Using a lower limit would indeed be a hack (hence: quick fix), but it could be argued that such small differences will normally not be the result from actual differences, intended by the author/user.
> (In reply to comment #5) > > I have run into this before myself. For me an acceptable workaround was to > > change the units of left/right margins to match and avoid the rounding errors, > <snip /> > Using a lower limit would indeed be a hack (hence: quick fix), but it could be > argued that such small differences will normally not be the result from actual > differences, intended by the author/user. BTW, the earlier value of 0.5pt was just arbitrary. Thinking more about it, 0.005pt would seem reasonable enough. That gives enough wiggle-room for the roundings (unless you have expressions with more than 5 of such operands, obviously) and 5/72000in (~0.2µm) can hardly be considered a change that would affect layout in such a way that a restart is justified. If all else fails, I assume that values specified in "pt" would also alleviate the trouble. At least, the conversion factor is a round figure there...
(In reply to comment #3) > page-width = 210mm = 595275(.5907) (...) > => 595275 - 82204 - 2834 = 510237 Hmm? 210 * 72000 / 25.4 = 595275.5905512 and: rounding page-with should give 595276 mpt Note that when using rounded rather than truncated values, the problem remains. (In reply to comment #6) > (...) unless we either start storing the > mpt values as double, and truncate only the result, or calculate with the > original value, including the units. In that case, it would work here, too: 210 > - 30 = 210 - 1 - 29 = 180. Due to conversion side effects, I fully agree with that. There should remain only one value to be rounded, rather 3 in this use-case. (In reply to comment #5) > I have run into this before myself. For me an acceptable workaround was to > change the units of left/right margins to match and avoid the rounding errors, > (...) I changed the units to > match left-margin="10pt" right-margin="30pt" Too, this is acceptable for me. I tried 'in' witch gives expected behavior. Finally, since 'mm' to 'in' conversion is not based on an integer ratio, the 2 alternatives are: - avoid such conversion (workaround), - convert floats.
(In reply to comment #8) > (In reply to comment #3) > > page-width = 210mm = 595275(.5907) > (...) > > => 595275 - 82204 - 2834 = 510237 > Hmm? > 210 * 72000 / 25.4 = 595275.5905512 Yes, for the entire page. 595275.5907 is the result of 210 * 2834.64567 (see fo.properties.FixedLength) Subtracting the left and right margins, gives you 510237 (if all values are first converted, then truncated). > and: rounding page-with should give 595276 mpt Rounding would, but FOP currently just casts from double to int (which is the same as truncating, IIRC) > > Note that when using rounded rather than truncated values, the problem remains. Indeed, as I pointed out, the values then switch places, that is: 510236 for page 1, and 510237 for page 2, so the difference of 1mpt remains. > Too, this is acceptable for me. I tried 'in' witch gives expected behavior. > > Finally, since 'mm' to 'in' conversion is not based on an integer ratio... Note: FOP does no "mm to in" conversion. All units are converted/normalized to the unofficial "mpt".
resetting P2 open bugs to P3 pending further review
andreas, any proposed resolution for this?
(In reply to comment #11) > andreas, any proposed resolution for this? the ideal solution should be to use double rather than int for millipoint values, since either cm-to-mp or mm-to-mp conversions give float results.
(In reply to comment #11) > andreas, any proposed resolution for this? Two proposals were already mentioned, admittedly rather vague. All that's left is the decision, which should be pretty simple. I would likely have committed the fix and closed the issue last year, if others had not insisted that FOP needs to avoid rounding altogether... There is the so-called 'ideal' or 'proper' solution, which would be a more long-term effort that involves a thorough impact analysis. A switch to values in whatever unit, stored internally as float or double, will have consequences all over the codebase, and for what? Just for the sake of theoretical, mathematical precision --beyond the third decimal, for a value expressed in pt...? Please! :-/ This would cost quite some time and effort for a prize that is hardly worth it in this context. I do not see why FOP would even /need/ that level of precision, given the scale. It's not like FOP is in the business of splitting atoms, so settling on 1/1000pt, internally stored as integers is really not that bad. We are only talking about a few tenths of a µm of difference due to loss in precision, here. Oh well, maybe it's just me... A more 'appropriate' solution, at least from a practical, cost/benefit perspective, would be to simply allow for a margin in the particular comparison triggering the reported issue. The line of code in question in PageBreakingAlgorithm[*] could be made to consider a value of 510237(mpt) as 'equal' to 510236(mpt). Sounds reasonable, keeping in mind that: 1° 'mpt' is an unofficial unit anyway, so FOP determines the calculation rules; why not have "1=2" when counting in those units? 2° converted back to standard units, the loss of 0.001pt would only yield a discernible difference provided that the output resolution is --yep, 72 *thousand* dpi. The latter proposal is a single-line fix for this particular bug entry. The attachment could then basically serve as the only test case, extended with a few other cases, trying out different combinations of attributes/unit specs that yield the largest conceivable rounding differences when run through FixedLength.convert(). [*] line 1138: 'Math.abs (...) <= m' instead of '... != 0', where m is half the amount of margin, in 'mpt', within which two page-widths are considered identical. As a suggestion, I mentioned 0.005pt earlier, or around 0.2µm. Interestingly, I found out later that typical light microscopes, assuming visible range light, have a theoretical resolution limit of just about that value. Might count as an argument pro: if your output target supported such a high pixel density, you would probably even miss it /even/ if you looked at it through a conventional microscope.
(In reply to comment #13) > Just for the sake of theoretical, mathematical precision --beyond the third > decimal, for a value expressed in pt...? Please! :-/ Hmm, many countries use the metric system, so the mathematical precision is not only a theoretical goal, but a true life need. IMHO, I think that mp as FOP standard unit is not appropriate. We should take benefit if the inner FOP unit was based on mm/pt ratio: 254mm equals 720pt (= 10in) for example, given this new unit (fd, for "FOP dot size"): 1pt = 127fd 1in = 9144fd 1mm = 360fd pro: we can keep integers values AND mathematical precision note: if fd precision is not enough, a decimal multiple or other may be used (x25 if we want that 1/300in gives an integer value when converted in fd).
http://svn.apache.org/viewvc?rev=1328581&view=rev Don't restart layout unless abs(ipd difference)>1 in order to prevent rounding issues from triggering false restart. Note that this change did not produce a junit regression, so either this corner case is (1) not sufficiently tested or (2) we will receive regression reports from users or (3) we may be safe with this change. Time will tell. Use of different units and different rounding policies may be further explored in future revisions.