Bug 51245

Summary: [PATCH] Text is strongly clipped by block-container border when display-align is "after".
Product: Fop - Now in Jira Reporter: Alexander Chingarev <alexander.chingarev>
Component: generalAssignee: fop-dev
Status: NEEDINFO ---    
Severity: normal    
Priority: P2    
Version: all   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: source fo and destination pdf
Patched AbstractBreaker.java
Patched BlockContainerLayoutManager.java

Description Alexander Chingarev 2011-05-23 13:46:06 UTC
See attachment: zip includes source fo file and dstination pdf.

If display-align is "after" and actual text height is a bit bigger than block-container height then text is shifted down and clipped by the block-container border.
Comment 1 Alexander Chingarev 2011-05-23 13:46:44 UTC
Created attachment 27046 [details]
source fo and destination pdf
Comment 2 Vincent Hennebert 2011-07-04 18:17:23 UTC
Hi Alexander,

Thanks for your simple test case. In short, this is because the content of the block-container doesn't fit within the given height. You must have seen the following warning given by FOP:
"Content overflows the viewport of an fo:block-container in block-progression direction by 600 millipoints. Content will be clipped."
If you extend the height of the block-container to 58pt then you should get what you want.

If you're happy with that solution, brilliant. If for some reason you don't want to extend the height of the block-container accordingly, then you are hitting a bug in FOP whose gory details are below.

In BlockContainerLM.getNextKnuthElementsAbsolute, FOP creates an instance of BlockContainerBreaker that in turn creates a PageBreakingAlgorithm instance. Since the content doesn't fit in the block-container, the algorithm breaks it into two parts. The second part just contains the last line, so most of the second part is filled with empty space (42.6pt in the present case). And this is this amount that is being taken into account to shift the content down to the bottom of the b-c. This is done in AbstractBreaker.addAreas ("if (pbp.difference != 0 && displayAlign == Constants.EN_AFTER) {"). 0 should be used instead.

I don't currently have an idea of how to fix this issue. This part of the code is very tricky and even a minor modification may lead to regressions.

Vincent
Comment 3 Alexander Chingarev 2011-07-08 09:30:36 UTC
Hi Vincent,

Thank you very much for so detailed explanation. Unfortunately we cannot extend the block-container.

But it gives me some thoughts on how to deal with the issue. At least I can try to pop up the warning.

Kind Regards,
Alexander
Comment 4 Alexander Chingarev 2011-08-12 09:55:07 UTC
Created attachment 27381 [details]
Patched AbstractBreaker.java
Comment 5 Alexander Chingarev 2011-08-12 09:55:38 UTC
Hi Vincent,

We dived into internals of page breaking algorithm and seems it is not correctly applied to fo:block-container.

We overrided doLayout method in BlockContainerLayoutManager.java so it implements different page pareaking logics. The only difference in overriden function is:
int optimalPageCount = alg.findBreakingPoints(effectiveList, 1, true,      BreakingAlgorithm.ONLY_FORCED_BREAKS);
instead of
int optimalPageCount = alg.findBreakingPoints(effectiveList, 1, true,      BreakingAlgorithm.ALL_BREAKS);

Seems it fixes the issue. 

Can you please take a look at the fix? Do you see any potencial regressions of it?

Modified AbstractBreaker.java and BlockContainerLayoutManager.java are attached.

Kind Regards,
Alexander
Comment 6 Alexander Chingarev 2011-08-12 09:56:22 UTC
Created attachment 27382 [details]
Patched BlockContainerLayoutManager.java
Comment 7 Vincent Hennebert 2011-10-12 18:57:29 UTC
Hi Alexander,

From which Subversion revision of FOP did you work? When I diff the attached files against the Trunk I find many differences that are clearly not related to your changes.

It would be really helpful if you could attach a diff instead of the whole files. You can do this by checking out the latest Trunk [1], applying your changes and then running 'svn diff'.

[1] http://xmlgraphics.apache.org/fop/download.html#source

Thanks,
Vincent
Comment 8 Glenn Adams 2012-04-07 01:41:20 UTC
resetting P2 open bugs to P3 pending further review
Comment 9 Glenn Adams 2012-04-08 09:10:51 UTC
increase priority due to presence of a patch
Comment 10 Glenn Adams 2012-04-08 20:27:45 UTC
please provide a single patch file containing context diffs (not entire files) against a known SVN revision; please ensure that only the intended fix diffs are present in the patch
Comment 11 Glenn Adams 2012-04-24 05:53:38 UTC
(In reply to comment #10)
> please provide a single patch file containing context diffs (not entire files)
> against a known SVN revision; please ensure that only the intended fix diffs
> are present in the patch

Alexander, I am still awaiting your input as requested above. if I see no further input by April 30, I will close this bug due to lack of requested information. Regards, Glenn