Re: DO NOT REPLY [Bug 31206] - [PATCH] Improvements over the new line breaking algorithm

[Luca Furini <lf...@cs.unibo.it>]

Simon Pepping wrote:

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

You are right, it's quite strange, but the LineLM still uses a few methods
inherited from InlineStackingLM. This was not due to the new algorithm, it
was already in the "old" code; I'm going to see if they still do something
useful ...

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

There isn't at the moment any practical gain, I just thought that, as a
text node has no children, a TextLM is a (special case of) LeafNodeLM.

> 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.

I decided to change and use a constant because the important thing is to
have the same value used by every LM, but this isn't the perfect solution;
if we try to center a short object (a single word, for example) in a long
line, it is likely that the algorithm fails because there isn't enough
stretchability in the line.
Maybe it's better to have the LineLM compute a value depending on the line
lenght and the maximum adjustment ratio; the child LM should ask the
LineLM for this value.

> 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.

The second version of the patch, which I'm going to attach to the Bugzilla
issue, fixes these errors.

It also implement the "vertical-align" property: now the values of top,
bottom, middle and baseline should be supported.
I made a few tests with fo:inline, fo:character and fo:external-graphic,
and it seems to work.

IMPORTANT: I had to revert Finn Bock's changes to the PDFRenderer (dated
2004/09/22 13:22:16), otherwise leaders with svg use-content produce
errors in the pdf output.
There isn't any run-time error, but when I try to open the pdf file, I get
these warnings:
 - There was an error processing a page. Wrong operand type.
 - Illegal operation 'q' inside a text object.
 - Wrong operand type.
and the page with the svg leaders is left empty.
I think it could be something involving the saveGraphicsState() method.

Still to be done:
  - remove unused methods and variables
  - simplify InlineStackingLM methods as suggested by Simon
I'll try and fix these points as soon as possible.

Regards
    Luca

Re: DO NOT REPLY [Bug 31206] - [PATCH] Improvements over the new line breaking algorithm

[Simon Pepping <sp...@leverkruid.nl>]

Luca,

I will try to look at your patch later this week.

Regards, Simon

-- 
Simon Pepping
home page: http://www.leverkruid.nl

Re: DO NOT REPLY [Bug 31206] - [PATCH] Improvements over the new line breaking algorithm

[Finn Bock <bc...@worldonline.dk>]

[Luca]

> IMPORTANT: I had to revert Finn Bock's changes to the PDFRenderer (dated
> 2004/09/22 13:22:16), otherwise leaders with svg use-content produce
> errors in the pdf output.
> There isn't any run-time error, but when I try to open the pdf file, I get
> these warnings:
>  - There was an error processing a page. Wrong operand type.
>  - Illegal operation 'q' inside a text object.
>  - Wrong operand type.
> and the page with the svg leaders is left empty.
> I think it could be something involving the saveGraphicsState() method.

I've just now fixed this issue so that your patch work with the PDFRenderer.

regards,
finn