Created attachment 12716 [details]
patch file

Created attachment 12717 [details]
Fo file: leaders with use-content and dots

Created attachment 12718 [details]
Fo file: centered text with larger or smaller inlines

I forgot to say that I fix a bug in the TextLM: KnuthBoxes generated by the 
TLM had height = 0, so the line height was not always correct.

Luca,

I have a few remarks:

1. InlineStackingLM implements InlineLM.  LineLM extends
InlineStackingLM and thus implements InlineLM. IMHO it should
not. Implementing InlineLM should be equivalent to
generatesInlineAreas returning true.

2. TextLM now extends LeafNodeLM instead of AbstractLM. What is the
gain?  I see no related changes in TextLM.

3. In LineLM:
    // this constant is used to create elements when text-align is center:
    // every TextLM descendant of LineLM must use the same value, 
    // otherwise the line breaking algorithm does not find the right
    // break point
    public static final int DEFAULT_SPACE_WIDTH = 3336;
    private static final int INFINITE_RATIO = 1000;

If these are static final, they might be better placed in
InlineLM. Alternatively, they might be attributes of the LineLM
object, which allows changing them per paragraph, e.g. depending on
the font. But then the problem arises how to propagate them to the
descendant LMs.

4. The textheight of the large font is rather large. The property
lineheight is not followed (reproduce existing behaviour).

5. LineLayoutManager:675: line is always 3, so that firstElementIndex
= 1 for the first line, and the first box is skipped in the line
height calculation.

I attach a test fo that illustrates the text and line height issues.

Regards, Simon

Created attachment 12784 [details]
test fo that illustrates the text and line height issues

I'm going to attach a revised version of the improvement patch:

- it fixes the line-height calculation errors
- better handling of preserved linefeed

I'm going to attach some test files too.

Regards
    Luca

Created attachment 13099 [details]
revised patch

Created attachment 13100 [details]
leaders with pattern = rule, dots and use-content with svg graphics

Created attachment 13101 [details]
vertical alignment (top, bottom, middle, baseline) of inlines

Created attachment 13102 [details]
vertical alignment of images

Created attachment 13103 [details]
file 15px.gif used in the test fo file

Created attachment 13104 [details]
file 20px.gif used in the test fo file

Created attachment 13105 [details]
file 30px.gif used in the test fo file

Luca,

Leaders with content: in the test file leaders.fo I get PDF errors in
the SVG parts.

I have not checked the calculations of the vertical alignment. The
provided test fo files, and some test files I created myself, behave
well under the new code. I trust the code is OK.

As Finn pointed out, vertical-align is a shorthand property in
XSL. The values top, bottom, middle and baseline map to the values
before-edge, after-edge, middle and baseline of the property
alignment-baseline, with the properties alignment-adjust,
baseline-shift and dominant-baseline being at their default
values. Therefore I think if the current property vertical-align is
renamed to alignment-baseline, the code will keep working fine, and
the way is free to implement vertical-align as a shorthand property.

InlineLM is now a class (created by Finn on Oct. 19). The hierarchy
is:

InlineStackingLM
- InlineLM
- LineLM

The class InlineLM is at the position in the hierarchy where I think
InlineLM interface should be. Therefore I think InlineLM class can
well be merged with the proposed InlineLM interface.

I think the code in this patch is OK for committing to CVS. The items
noted above can be addressed over time.  However, the patch as it is
cannot be applied to the current version of CVS HEAD, due to the newly
created InlineLM class and extensive changes in property retrieval. If
you submit a patch of the same code against the current CVS HEAD, I
will commit it. Or you can keep it until you have write access to CVS
yourself.

Regards, Simon

I have created a new patch against HEAD.
I have tested it and it seems to work, but at the moment I'm not yet ready to 
commit it, sorry; maybe someone could do this in my place?

A few points which may need further work:

- I renamed the new interface from InlineLM to InlineLevelLM. Merging it with 
the new InlineLM class would involve having TextLM and LeafNodeLM extend 
InlineLM instead of AbstractLM, and this seems a bit strange to me

- the offset of the areas is set according to the value of the vertical-align 
property instead of alignment-baseline

- the offset of the viewport area generated by svg graphics is not set, 
because I couldn't find its height

- the offset of textAreas is set by the TextLM, which creates them; but 
vertical-align is a property of inline level formatting objects only, so the 
TextLM checks if the parent node is an Inline

- the main problem in applying the changes to the new code involved leaders 
with use-content; in the old code, the method LeaderLM.getLeaderInlineArea() 
created a ContentLM and an InlineStackingLM:
            LeaderLM
                |
                |
            ContentLM
                |
                |
         InlineStackingLM
            ____|____
            |   |   |
     (LMs handling the use-content)
but now InlineStackingLM does no more implement getNextKnuthElements(), and 
the InlineLM constructor requires an Inline node; I temporarily fixed this by 
adding a constructor for InlineLM with a Leader parameter, but I don't like 
this very much.

Regards,
    Luca

Created attachment 13414 [details]
new interface InlineLevelLayoutManager.java

Created attachment 13415 [details]
updated patch to existing files

Applied. 

I made BasicLinkLM also extend InlineLM.

Thanks. Simon Pepping

batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed