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

2004-10-26 Thread Simon Pepping
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

2004-10-15 Thread Luca Furini

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

2004-10-15 Thread Finn Bock
[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