Hi all I am looking at the code of LineLayoutManager and TextLayoutManager, in order to see why text justification doesn't work, and I think I have found some inconsistencies (or maybe I am just misunderstanding the code). The LineLayoutManager.makeLineBreak() method calculates two adjustment factors: - "ipdAdjust", defined "stretch or shrink factor" by a comment - "dAdjust", defined "space adjustment"; it is calculated only if text- align="justify" They are both calculated using the "actual" width of the line (given by the inline objects) and the "target" width (the width that the line shall have); actual and target are MinOptMax objects. The ipdAdjust factor, if I have not mistaken its role, should be used when the actual width has different .min, .opt and .max values (in simple situations they are equal one another). Every number y in the range [actual.min, actual.max] can be written as y = actual.opt + (actual.opt - actual.min)*x if actual.min <= y <= actual.opt, with -1 <= x <= 0 or y = actual.opt + (actual.max - actual.opt)*x if actual.opt <= y <= actual.max, with 0 <= x <= 1 If target.opt is in the range [actual.min, actual.max], meaning that there is a possible actual width perfectly filling the line , then ipdAdjust is its corresponding "x" factor. Otherwise, the actual width is chosen which is nearest to target.opt: actual.min or actual.max. maybe-error 1: it seems to me that there is an error in this part of the code (comments added where the supposed error is): int targetWith = target.opt; int realWidth = actual.opt; if (actual.opt > targetWith) { if (actual.opt - targetWith < (actual.opt - actual.min)) { ipdAdjust = -(actual.opt - targetWith) / (float)(actual.opt - actual.min); realWidth = targetWith; } else { ipdAdjust = -1; realWidth = actual.max; /* <- error?*/ // targetWith is closer to actual.min than to actual.max // and -1 corresponds to actual.min // // targetWith // | // ---o----------------------------------- // ^ ^ ^ // actual.min | | actual.opt | actual.max } } else { if (targetWith - actual.opt < actual.max - actual.opt) { ipdAdjust = (targetWith - actual.opt) / (float)(actual.max - actual.opt); realWidth = targetWith; } else { ipdAdjust = 1; realWidth = actual.min; /* <- error?*/ // targetWith is closer to actual.max than to actual.min // and 1 corresponds to actual.max // // targetWith // | // ------------------------------------o-- // ^ ^ ^ // actual.min | | actual.opt | actual.max } } maybe-error 2: the dAdjust formula is now: dAdjust = (targetWith - realWidth) / realWidth; and in my tests its value is always 0.0. Adding an explicit cast to float, the value is right: dAdjust = (float)(targetWith - realWidth) / realWidth; These adjustment factors are then stored in a LayoutContext object and given to the TextLayoutManager addAreas method, which should: I) re-compute the "real" width using the ipdAdjustment factor II) re-create the difference between real and target width using the spaceAdjustment factor III) divide that difference by the number of spaces ... maybe-error 3: now, the TextLayoutManager uses only the space adjustment, and the recreated difference (called iAdjust) differs from the original one. I'm going to attach: - a simple fo file with a justified block of text - a patch which simply adds a few System.err.println to the old code, to show the computed adjustments and difference to fill - a patch with the proposed changes and System.err.println to show the newly computed adjustments Anyway, there must be something else that goes wrong, because the resulting pdf is still not justified. Bye Luca
Created attachment 11083 [details] only a block of justified text to see the calculated adjustments
Created attachment 11084 [details] a few println in the old code to show the calculated adjustments
Created attachment 11085 [details] proposed changes to the formulas and some println
Excellent work. Text justification is one of FOP development priorities. Priorities for layout work are defined here: http://xml.apache.org/fop/design/layout.html#status-todo I went through a similar thought process to you when I analysed the code in Text and Line LMs. I'm taking a closer look at your patch now. I believe the reason why justification still doesnt work after correcting the issues you've found is because TextLM.addAreas doesnt create separate areas for each word - it creates one big area in some cases for whole line, so there is no opportunity to add space adjust for justification. Your patch looks like it is the first piece of the puzzle to get Justification working
Patch applied, many thanks.
Hi again I have tried to make PDFRenderer use the adjustment factor without splitting the text in different areas, and it seems it now works (except when hyphenation is done, but that's another bug). Only a quick note: as the pdf operator controlling space adjust (Tw) can take a float, space adjustment could be stored as double instead of casting it into an int. I am going to attach the suggested patch. Bye Luca
Created attachment 11134 [details] patch to pdf renderer, so that it uses the adjustment factor
Hi Luca, thanks for your new suggested patch. Since this bug is closed I have created a new bug for your new patch. http://issues.apache.org/bugzilla/show_bug.cgi?id=28208
batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed