You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by je...@apache.org on 2005/01/07 09:21:21 UTC

cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

jeremias    2005/01/07 00:21:21

  Modified:    src/java/org/apache/fop/fo/properties CommonMarginBlock.java
               src/java/org/apache/fop/layoutmgr TraitSetter.java
                        BlockLayoutManager.java
  Log:
  Bugfix for start-indent calculation for nested blocks. The inherited start-indent wasn't taken into account as described in 5.3.2 of the spec.
  Minor style and javadoc improvements on the way.
  
  Revision  Changes    Path
  1.5       +34 -2     xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java
  
  Index: CommonMarginBlock.java
  ===================================================================
  RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java,v
  retrieving revision 1.4
  retrieving revision 1.5
  diff -u -r1.4 -r1.5
  --- CommonMarginBlock.java	28 Oct 2004 10:00:24 -0000	1.4
  +++ CommonMarginBlock.java	7 Jan 2005 08:21:21 -0000	1.5
  @@ -1,5 +1,5 @@
   /*
  - * Copyright 1999-2004 The Apache Software Foundation.
  + * Copyright 1999-2005 The Apache Software Foundation.
    * 
    * Licensed under the Apache License, Version 2.0 (the "License");
    * you may not use this file except in compliance with the License.
  @@ -70,6 +70,16 @@
       public Length endIndent;
   
       /**
  +     * The inherited "start-indent" property.
  +     */
  +    public Length inheritedStartIndent;
  +
  +    /**
  +     * The inherited "end-indent" property.
  +     */
  +    public Length inheritedEndIndent;
  +
  +    /**
        * Create a CommonMarginBlock object.
        * @param pList The PropertyList with propery values.
        */
  @@ -84,5 +94,27 @@
   
           startIndent = pList.get(Constants.PR_START_INDENT).getLength();
           endIndent = pList.get(Constants.PR_END_INDENT).getLength();
  +        
  +        if (!pList.getFObj().generatesReferenceAreas()) {
  +            inheritedStartIndent = pList.getParentPropertyList()
  +                    .get(Constants.PR_START_INDENT).getLength();
  +            inheritedEndIndent = pList.getParentPropertyList()
  +                    .get(Constants.PR_END_INDENT).getLength();
  +        }
  +    }
  +    
  +    /** @see java.lang.Object#toString() */
  +    public String toString() {
  +        return "CommonMarginBlock:\n" 
  +            + "Margins (top, bottom, left, right): (" 
  +            + marginTop + ", " + marginBottom + ", " 
  +            + marginLeft + ", " + marginRight + ")\n"
  +            + "Space (before, after): (" 
  +            + spaceBefore + ", " + spaceAfter + ")\n" 
  +            + "Indents (start, end): ("
  +            + startIndent + ", " + endIndent + ")\n"
  +            + "Indents inherited (start, end): (" 
  +            + inheritedStartIndent + ", " + inheritedEndIndent + ")\n";
       }
  +    
   }
  
  
  
  1.11      +33 -22    xml-fop/src/java/org/apache/fop/layoutmgr/TraitSetter.java
  
  Index: TraitSetter.java
  ===================================================================
  RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/layoutmgr/TraitSetter.java,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- TraitSetter.java	20 Oct 2004 11:55:32 -0000	1.10
  +++ TraitSetter.java	7 Jan 2005 08:21:21 -0000	1.11
  @@ -1,5 +1,5 @@
   /*
  - * Copyright 1999-2004 The Apache Software Foundation.
  + * Copyright 1999-2005 The Apache Software Foundation.
    *
    * Licensed under the Apache License, Version 2.0 (the "License");
    * you may not use this file except in compliance with the License.
  @@ -33,6 +33,8 @@
        * Sets border and padding traits on areas.
        * @param area area to set the traits on
        * @param bpProps border and padding properties
  +     * @param bNotFirst True if the area is not the first area
  +     * @param bNotLast True if the area is not the last area
        */
       public static void setBorderPaddingTraits(Area area,
               CommonBorderPaddingBackground bpProps, boolean bNotFirst, boolean bNotLast) {
  @@ -96,45 +98,45 @@
        * Add borders to an area.
        * Layout managers that create areas with borders can use this to
        * add the borders to the area.
  -     * @param curBlock area to set the traits on
  +     * @param area the area to set the traits on.
        * @param bordProps border properties
        */
  -    public static void addBorders(Area curBlock, CommonBorderPaddingBackground bordProps) {
  +    public static void addBorders(Area area, CommonBorderPaddingBackground bordProps) {
           BorderProps bps = getBorderProps(bordProps, CommonBorderPaddingBackground.BEFORE);
           if (bps.width != 0) {
  -            curBlock.addTrait(Trait.BORDER_BEFORE, bps);
  +            area.addTrait(Trait.BORDER_BEFORE, bps);
           }
           bps = getBorderProps(bordProps, CommonBorderPaddingBackground.AFTER);
           if (bps.width != 0) {
  -            curBlock.addTrait(Trait.BORDER_AFTER, bps);
  +            area.addTrait(Trait.BORDER_AFTER, bps);
           }
           bps = getBorderProps(bordProps, CommonBorderPaddingBackground.START);
           if (bps.width != 0) {
  -            curBlock.addTrait(Trait.BORDER_START, bps);
  +            area.addTrait(Trait.BORDER_START, bps);
           }
           bps = getBorderProps(bordProps, CommonBorderPaddingBackground.END);
           if (bps.width != 0) {
  -            curBlock.addTrait(Trait.BORDER_END, bps);
  +            area.addTrait(Trait.BORDER_END, bps);
           }
   
           int padding = bordProps.getPadding(CommonBorderPaddingBackground.START, false);
           if (padding != 0) {
  -            curBlock.addTrait(Trait.PADDING_START, new java.lang.Integer(padding));
  +            area.addTrait(Trait.PADDING_START, new java.lang.Integer(padding));
           }
   
           padding = bordProps.getPadding(CommonBorderPaddingBackground.END, false);
           if (padding != 0) {
  -            curBlock.addTrait(Trait.PADDING_END, new java.lang.Integer(padding));
  +            area.addTrait(Trait.PADDING_END, new java.lang.Integer(padding));
           }
   
           padding = bordProps.getPadding(CommonBorderPaddingBackground.BEFORE, false);
           if (padding != 0) {
  -            curBlock.addTrait(Trait.PADDING_BEFORE, new java.lang.Integer(padding));
  +            area.addTrait(Trait.PADDING_BEFORE, new java.lang.Integer(padding));
           }
   
           padding = bordProps.getPadding(CommonBorderPaddingBackground.AFTER, false);
           if (padding != 0) {
  -            curBlock.addTrait(Trait.PADDING_AFTER, new java.lang.Integer(padding));
  +            area.addTrait(Trait.PADDING_AFTER, new java.lang.Integer(padding));
           }
       }
   
  @@ -150,10 +152,10 @@
        * Add background to an area.
        * Layout managers that create areas with a background can use this to
        * add the background to the area.
  -     * @param curBlock the current block
  +     * @param area the area to set the traits on
        * @param backProps the background properties
        */
  -    public static void addBackground(Area curBlock, CommonBorderPaddingBackground backProps) {
  +    public static void addBackground(Area area, CommonBorderPaddingBackground backProps) {
           Trait.Background back = new Trait.Background();
           back.setColor(backProps.backgroundColor);
   
  @@ -169,7 +171,7 @@
           }
   
           if (back.getColor() != null || back.getURL() != null) {
  -            curBlock.addTrait(Trait.BACKGROUND, back);
  +            area.addTrait(Trait.BACKGROUND, back);
           }
       }
   
  @@ -177,29 +179,38 @@
        * Add space to a block area.
        * Layout managers that create block areas can use this to add space
        * outside of the border rectangle to the area.
  -     * @param curBlock the current block.
  +     * @param area the area to set the traits on.
  +     * @param bpProps the border, padding and background properties
        * @param marginProps the margin properties.
        */
  -    public static void addMargins(Area curBlock,
  +    public static void addMargins(Area area,
                                     CommonBorderPaddingBackground bpProps,
                                     CommonMarginBlock marginProps) {
  -        int spaceStart = marginProps.startIndent.getValue() 
  +        int spaceStart = marginProps.startIndent.getValue()
  +                            - marginProps.inheritedStartIndent.getValue()
                               - bpProps.getBorderStartWidth(false)
                               - bpProps.getPaddingStart(false);
           if (spaceStart != 0) {
  -            curBlock.addTrait(Trait.SPACE_START, new Integer(spaceStart));
  +            area.addTrait(Trait.SPACE_START, new Integer(spaceStart));
           }
   
           int spaceEnd = marginProps.endIndent.getValue()
  +                            - marginProps.inheritedEndIndent.getValue()
                               - bpProps.getBorderEndWidth(false)
                               - bpProps.getPaddingEnd(false);
           if (spaceEnd != 0) {
  -            curBlock.addTrait(Trait.SPACE_END, new Integer(spaceEnd));
  +            area.addTrait(Trait.SPACE_END, new Integer(spaceEnd));
           }
       }
   
  -    public static void addBreaks(Area curArea,  int breakBefore, int breakAfter) {
  -        curArea.addTrait(Trait.BREAK_AFTER, new Integer(breakAfter));
  -        curArea.addTrait(Trait.BREAK_BEFORE, new Integer(breakBefore));
  +    /**
  +     * Sets the traits for breaks on an area.
  +     * @param area the area to set the traits on.
  +     * @param breakBefore the value for break-before
  +     * @param breakAfter the value for break-after
  +     */
  +    public static void addBreaks(Area area,  int breakBefore, int breakAfter) {
  +        area.addTrait(Trait.BREAK_AFTER, new Integer(breakAfter));
  +        area.addTrait(Trait.BREAK_BEFORE, new Integer(breakBefore));
       }
   }
  
  
  
  1.35      +41 -11    xml-fop/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java
  
  Index: BlockLayoutManager.java
  ===================================================================
  RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java,v
  retrieving revision 1.34
  retrieving revision 1.35
  diff -u -r1.34 -r1.35
  --- BlockLayoutManager.java	7 Dec 2004 20:11:21 -0000	1.34
  +++ BlockLayoutManager.java	7 Jan 2005 08:21:21 -0000	1.35
  @@ -1,5 +1,5 @@
   /*
  - * Copyright 1999-2004 The Apache Software Foundation.
  + * Copyright 1999-2005 The Apache Software Foundation.
    *
    * Licensed under the Apache License, Version 2.0 (the "License");
    * you may not use this file except in compliance with the License.
  @@ -19,7 +19,6 @@
   package org.apache.fop.layoutmgr;
   
   import java.util.ListIterator;
  -import java.util.ArrayList;
   import java.util.List;
   
   import org.apache.fop.datatypes.PercentBase;
  @@ -39,6 +38,7 @@
       
       private Block curBlockArea;
   
  +    /** Iterator over the child layout managers. */
       protected ListIterator proxyLMiter;
   
       /* holds the (one-time use) fo:block space-before
  @@ -61,8 +61,13 @@
   
       private int iStartPos = 0;
   
  +    /** The list of child BreakPoss instances. */
       protected List childBreaks = new java.util.ArrayList();
   
  +    /**
  +     * Creates a new BlockLayoutManager.
  +     * @param inBlock the block FO object to create the layout manager for.
  +     */
       public BlockLayoutManager(org.apache.fop.fo.flow.Block inBlock) {
           super(inBlock);
           fobj = inBlock;
  @@ -98,7 +103,7 @@
   
           public ProxyLMiter() {
               super(BlockLayoutManager.this);
  -            listLMs = new ArrayList(10);
  +            listLMs = new java.util.ArrayList(10);
           }
   
           public boolean hasNext() {
  @@ -143,7 +148,7 @@
       private LineLayoutManager createLineManager(LayoutManager firstlm) {
           LineLayoutManager llm;
           llm = new LineLayoutManager(fobj, lineHeight, lead, follow, middleShift);
  -        List inlines = new ArrayList();
  +        List inlines = new java.util.ArrayList();
           inlines.add(firstlm);
           while (proxyLMiter.hasNext()) {
               LayoutManager lm = (LayoutManager) proxyLMiter.next();
  @@ -158,12 +163,18 @@
           return llm;
       }
   
  +    /**
  +     * @see org.apache.fop.layoutmgr.LayoutManager#getNextBreakPoss(org.apache.fop.layoutmgr.LayoutContext)
  +     */
       public BreakPoss getNextBreakPoss(LayoutContext context) {
           LayoutManager curLM; // currently active LM
   
           int ipd = context.getRefIPD();
  -        int iIndents = fobj.getCommonMarginBlock().startIndent.getValue() + fobj.getCommonMarginBlock().endIndent.getValue();
  -        int bIndents = fobj.getCommonBorderPaddingBackground().getBPPaddingAndBorder(false);
  +        int iIndents = fobj.getCommonMarginBlock().startIndent.getValue();
  +        iIndents += fobj.getCommonMarginBlock().endIndent.getValue();
  +        iIndents -= fobj.getCommonMarginBlock().inheritedStartIndent.getValue();
  +        iIndents -= fobj.getCommonMarginBlock().inheritedEndIndent.getValue();
  +        //int bIndents = fobj.getCommonBorderPaddingBackground().getBPPaddingAndBorder(false);
           ipd -= iIndents;
   
           MinOptMax stackSize = new MinOptMax();
  @@ -258,6 +269,9 @@
           return breakPoss;
       }
   
  +    /**
  +     * @see org.apache.fop.layoutmgr.LayoutManager#addAreas(org.apache.fop.layoutmgr.PositionIterator, org.apache.fop.layoutmgr.LayoutContext)
  +     */
       public void addAreas(PositionIterator parentIter,
                            LayoutContext layoutContext) {
           getParentArea(null);
  @@ -309,16 +323,22 @@
        * Finally, based on the dimensions of the parent area, it initializes
        * its own area. This includes setting the content IPD and the maximum
        * BPD.
  +     * @param childArea area to get the parent area for
        */
       public Area getParentArea(Area childArea) {
           if (curBlockArea == null) {
               curBlockArea = new Block();
   
               // set traits
  -            TraitSetter.addBorders(curBlockArea, fobj.getCommonBorderPaddingBackground());
  -            TraitSetter.addBackground(curBlockArea, fobj.getCommonBorderPaddingBackground());
  -            TraitSetter.addMargins(curBlockArea, fobj.getCommonBorderPaddingBackground(), fobj.getCommonMarginBlock());
  -            TraitSetter.addBreaks(curBlockArea, fobj.getBreakBefore(), fobj.getBreakAfter());
  +            TraitSetter.addBorders(curBlockArea, 
  +                    fobj.getCommonBorderPaddingBackground());
  +            TraitSetter.addBackground(curBlockArea, 
  +                    fobj.getCommonBorderPaddingBackground());
  +            TraitSetter.addMargins(curBlockArea, 
  +                    fobj.getCommonBorderPaddingBackground(), 
  +                    fobj.getCommonMarginBlock());
  +            TraitSetter.addBreaks(curBlockArea, 
  +                    fobj.getBreakBefore(), fobj.getBreakAfter());
   
               // Set up dimensions
               // Must get dimensions from parent area
  @@ -337,13 +357,20 @@
               if (parentwidth == 0) {
                   parentwidth = referenceIPD;
               }
  -            parentwidth -= fobj.getCommonMarginBlock().startIndent.getValue() + fobj.getCommonMarginBlock().endIndent.getValue();
  +            parentwidth -= fobj.getCommonMarginBlock().startIndent.getValue();
  +            parentwidth -= fobj.getCommonMarginBlock().endIndent.getValue();
  +            parentwidth += fobj.getCommonMarginBlock().inheritedStartIndent.getValue();
  +            parentwidth += fobj.getCommonMarginBlock().inheritedEndIndent.getValue();
  +            
               curBlockArea.setIPD(parentwidth);
               setCurrentArea(curBlockArea); // ??? for generic operations
           }
           return curBlockArea;
       }
   
  +    /**
  +     * @see org.apache.fop.layoutmgr.LayoutManager#addChild(org.apache.fop.area.Area)
  +     */
       public void addChild(Area childArea) {
           if (curBlockArea != null) {
               if (childArea instanceof LineArea) {
  @@ -354,6 +381,9 @@
           }
       }
   
  +    /**
  +     * @see org.apache.fop.layoutmgr.LayoutManager#resetPosition(org.apache.fop.layoutmgr.Position)
  +     */
       public void resetPosition(Position resetPos) {
           if (resetPos == null) {
               reset(null);
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: fop-cvs-unsubscribe@xml.apache.org
For additional commands, e-mail: fop-cvs-help@xml.apache.org


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Jeremias Maerki <de...@greenmail.ch>.
On 07.01.2005 10:49:02 Finn Bock wrote:
> [Jeremias]
> 
> > would you please check if it is acceptable to put the inherited values
> > directly into the CommonMarginBlock? It might have been cleaner to
> > always get the value via the parent FO but I think in this case it helps
> > simplifying the code in TraitSetter and BlockLayoutManager.
> 
> It looks wrong, it feels wrong, but I'm not at all sure if it is wrong.

Same here. :-)

> But I would be tempted to drop the use of space-[start,end] traits and 
> instead let the renderers use [start,end]-indent traits and the 
> reference rectangle to calculate the start position of the content 
> rectangle.
>
> Alternatively calculate space-[start,end] based on the 
> [start,end]-indent traits on the parent area rather then the parent fo.

I'll see what I can do. I got a great environment to test now. ;-)
Thanks for your insight.

Jeremias Maerki


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Jeremias Maerki <de...@greenmail.ch>.
I think I got it. As soon as I started working with start|end-indent
wherever possible everthing started clicking in place and got simpler.

Thanks again for your patience and for your helpful advice!


Jeremias Maerki


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Jeremias Maerki <de...@greenmail.ch>.
Funny! I just came to the same conclusion a few minutes ago. Simon's
last comment brought me to that.

Simon:
"I see no mention in section 5 of the spec that the trait value for
start-indent is different from the computed property value."

I then checked the BlockLayoutManager and realized that what Finn is now
saying is exactly what's suboptimal about the current approach. I'm just
now reworking the code to implement what Finn suggests. It also lets me
forget about that silly and complicated MarginPropertyMaker I started to
write.

Thanks Guys helping me on the right track!

On 12.01.2005 08:42:24 Finn Bock wrote:
> [Simon]
> 
> >>>There does not seem to be a need to add
> >>>the inherited value later; the property maker already has done so. See
> >>>IndentPropertyMaker.compute(PropertyList). It uses
> >>>propertyList.getInherited(baseMaker.propId).getNumeric()) to get the
> >>>inherited value. Earlier FOP developers understood this part well.
> 
> [Jeremias]
> 
> >>I understand, but I think you're talking exclusively about the property
> >>resolution phase (right?) while I found that I need the computed value
> >>of the margin property (not only the explicit one as is currently the
> >>case) and the inherited start-indent for the layout manager code and to
> >>set traits correctly.
> 
> [Simon]
> 
> > PropertyList().get(Constants.PR_START_INDENT) gets the computed value,
> > that is, the value after property refinement. It is not the raw value
> > stated in the FO file; FOP does not store that at all. FOP tries to do
> > property refinement immediately. If that is not possible because the
> > computed value depends on a trait of an area, FOP stores an
> > expression, which can be computed at layout time.
> > 
> > I see no mention in section 5 of the spec that the trait value for
> > start-indent is different from the computed property value.
> 
> The problem that Jeremias is trying to address is computing 
> space-[start,end] traits. Since only start-indent was computed correctly 
> he tried to compute space-[start,end] traits based on the 
> [start,end]-indent and to do that the inherited indent values (which has 
> just been added in IndentPropertyMaker) must be substracted.
> 
> Which may indicate that calculating space-[start,end] traits based on 
> indents is probably not the right approach.
> 
> I think the solution would be to:
> - Store the [start,end]-indent traits.
> - Let LayoutContent.refIPD be the reference IPD rather than the content
>    ipd of the parent.
> - let the renderer keep track of the reference rect and use the
>    start-indent trait to locate the position of the content rectangle.



Jeremias Maerki


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Finn Bock <bc...@worldonline.dk>.
[Simon]

>>>There does not seem to be a need to add
>>>the inherited value later; the property maker already has done so. See
>>>IndentPropertyMaker.compute(PropertyList). It uses
>>>propertyList.getInherited(baseMaker.propId).getNumeric()) to get the
>>>inherited value. Earlier FOP developers understood this part well.

[Jeremias]

>>I understand, but I think you're talking exclusively about the property
>>resolution phase (right?) while I found that I need the computed value
>>of the margin property (not only the explicit one as is currently the
>>case) and the inherited start-indent for the layout manager code and to
>>set traits correctly.

[Simon]

> PropertyList().get(Constants.PR_START_INDENT) gets the computed value,
> that is, the value after property refinement. It is not the raw value
> stated in the FO file; FOP does not store that at all. FOP tries to do
> property refinement immediately. If that is not possible because the
> computed value depends on a trait of an area, FOP stores an
> expression, which can be computed at layout time.
> 
> I see no mention in section 5 of the spec that the trait value for
> start-indent is different from the computed property value.

The problem that Jeremias is trying to address is computing 
space-[start,end] traits. Since only start-indent was computed correctly 
he tried to compute space-[start,end] traits based on the 
[start,end]-indent and to do that the inherited indent values (which has 
just been added in IndentPropertyMaker) must be substracted.

Which may indicate that calculating space-[start,end] traits based on 
indents is probably not the right approach.

I think the solution would be to:
- Store the [start,end]-indent traits.
- Let LayoutContent.refIPD be the reference IPD rather than the content
   ipd of the parent.
- let the renderer keep track of the reference rect and use the
   start-indent trait to locate the position of the content rectangle.

regards,
finn

Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Simon Pepping <sp...@leverkruid.nl>.
On Tue, Jan 11, 2005 at 09:25:50AM +0100, Jeremias Maerki wrote:
> 
> On 10.01.2005 22:00:01 Simon Pepping wrote:
> > There does not seem to be a need to add
> > the inherited value later; the property maker already has done so. See
> > IndentPropertyMaker.compute(PropertyList). It uses
> > propertyList.getInherited(baseMaker.propId).getNumeric()) to get the
> > inherited value. Earlier FOP developers understood this part well.
> 
> I understand, but I think you're talking exclusively about the property
> resolution phase (right?) while I found that I need the computed value
> of the margin property (not only the explicit one as is currently the
> case) and the inherited start-indent for the layout manager code and to
> set traits correctly.

PropertyList().get(Constants.PR_START_INDENT) gets the computed value,
that is, the value after property refinement. It is not the raw value
stated in the FO file; FOP does not store that at all. FOP tries to do
property refinement immediately. If that is not possible because the
computed value depends on a trait of an area, FOP stores an
expression, which can be computed at layout time.

I see no mention in section 5 of the spec that the trait value for
start-indent is different from the computed property value.

Regards, Simon

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


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Jeremias Maerki <de...@greenmail.ch>.
On 10.01.2005 22:00:01 Simon Pepping wrote:
> Section 5.3.2 of the spec is really hard to understand. I combine it
> with 5.1.4 about Inheritance. Then my guess is this:
> 
> <fo:block margin-left="1pc">A test file
>   <fo:inline-container start-indent="1.5pc">
>     <fo:block margin-left="1pc">A test file</fo:block>
>   </fo:inline-container>
> </fo:block>
> 
> The computed value of start-indent on the outer block is 'start-indent
> = inherited_value_of(start-indent) + margin-corresponding +
> padding-corresponding + border-corresponding-width' = 0 + 1pc + 0 +
> 0. The computed value of start-indent on the inner block is
> 'start-indent = inherited_value_of(start-indent) +
> margin-corresponding + padding-corresponding +
> border-corresponding-width' = 1.5pc + 1pc + 0 + 0.
> 
> In this case:
> 
> <fo:block margin-left="1pc">A test file
>   <fo:inline-container>
>     <fo:block margin-left="1pc">A test file</fo:block>
>   </fo:inline-container>
> </fo:block>
> 
> the computed value of start-indent on the outer block is 'start-indent
> = inherited_value_of(start-indent) + margin-corresponding +
> padding-corresponding + border-corresponding-width' = 0 + 1pc + 0 +
> 0. The computed value of start-indent on the inner block is
> 'start-indent = inherited_value_of(start-indent) +
> margin-corresponding + padding-corresponding +
> border-corresponding-width' = 1pc + 1pc + 0 + 0. The inherited value
> uses the calculated value (sect. 5.1.4). That is the value that should
> be returned by
> pList.getParentPropertyList().get(Constants.PR_START_INDENT).getLength().

I agree with your interpretation.

> The inherited value should not be stored, but used in the computation
> of the property value. This should be implemented by the property
> maker.

In the meantime I've rewritten that part and removed the two fields I
introduced on CommonMarginBlock. But the code needs some more work until
I'm ready to check in. All testcases pass again but I don't have any
tests for writing mode changes which is important for the
MarginPropertyMaker I wrote to get the computed margin values.

> When I run the above examples in a debugger, I find that the computed
> start-indent values CommonMarginBlock.startIndent are exactly like I
> argue above they should be.

Yes.

> There does not seem to be a need to add
> the inherited value later; the property maker already has done so. See
> IndentPropertyMaker.compute(PropertyList). It uses
> propertyList.getInherited(baseMaker.propId).getNumeric()) to get the
> inherited value. Earlier FOP developers understood this part well.

I understand, but I think you're talking exclusively about the property
resolution phase (right?) while I found that I need the computed value
of the margin property (not only the explicit one as is currently the
case) and the inherited start-indent for the layout manager code and to
set traits correctly.

> If you find wrong results, then the problem must be elsewhere.

Yes, in the layout managers.

> Is there a book or treatise on these subjects, where we can read how a
> knowledgeable author interprets these difficult parts of the spec?

I don't know. We're probably stuck with the FO-related mailing lists.

Please have patience with me while I'm learning my way into layout. I
can already feel my understanding of the whole thing growing from hour
to hour and I'm grateful for any insight you can provide. I'm sorry for
being a moving target.

It's too bad that the ICLayoutManager is still empty so your examples
above cannot be tested, yet, but I have other examples that help show
similar effects with what already works.

Jeremias Maerki


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Finn Bock <bc...@worldonline.dk>.
[Jeremias]

> would you please check if it is acceptable to put the inherited values
> directly into the CommonMarginBlock? It might have been cleaner to
> always get the value via the parent FO but I think in this case it helps
> simplifying the code in TraitSetter and BlockLayoutManager.

It looks wrong, it feels wrong, but I'm not at all sure if it is wrong.

But I would be tempted to drop the use of space-[start,end] traits and 
instead let the renderers use [start,end]-indent traits and the 
reference rectangle to calculate the start position of the content 
rectangle.

Alternatively calculate space-[start,end] based on the 
[start,end]-indent traits on the parent area rather then the parent fo.

regards,
finn

Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Glen Mazza <gr...@yahoo.com>.
--- Glen Mazza <gr...@yahoo.com> wrote:

> BTW, would Jeremias' proposal 
> effect future
  ^^^^^^

oops....affect ;)

Glen


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Glen Mazza <gr...@yahoo.com>.
BTW, would Jeremias' proposal effect future
implementation of the property value functions[1]?

Thanks,
Glen

[1]
http://www.w3.org/TR/2001/REC-xsl-20011015/slice5.html#section-N8624-Property-Value-Functions

--- Simon Pepping <sp...@leverkruid.nl> wrote:

> Section 5.3.2 of the spec is really hard to
> understand. I combine it
> with 5.1.4 about Inheritance. Then my guess is this:
> 
> <fo:block margin-left="1pc">A test file
>   <fo:inline-container start-indent="1.5pc">
>     <fo:block margin-left="1pc">A test
> file</fo:block>
>   </fo:inline-container>
> </fo:block>
> 
> The computed value of start-indent on the outer
> block is 'start-indent
> = inherited_value_of(start-indent) +
> margin-corresponding +
> padding-corresponding + border-corresponding-width'
> = 0 + 1pc + 0 +
> 0. The computed value of start-indent on the inner
> block is
> 'start-indent = inherited_value_of(start-indent) +
> margin-corresponding + padding-corresponding +
> border-corresponding-width' = 1.5pc + 1pc + 0 + 0.
> 
> In this case:
> 
> <fo:block margin-left="1pc">A test file
>   <fo:inline-container>
>     <fo:block margin-left="1pc">A test
> file</fo:block>
>   </fo:inline-container>
> </fo:block>
> 
> the computed value of start-indent on the outer
> block is 'start-indent
> = inherited_value_of(start-indent) +
> margin-corresponding +
> padding-corresponding + border-corresponding-width'
> = 0 + 1pc + 0 +
> 0. The computed value of start-indent on the inner
> block is
> 'start-indent = inherited_value_of(start-indent) +
> margin-corresponding + padding-corresponding +
> border-corresponding-width' = 1pc + 1pc + 0 + 0. The
> inherited value
> uses the calculated value (sect. 5.1.4). That is the
> value that should
> be returned by
>
pList.getParentPropertyList().get(Constants.PR_START_INDENT).getLength().
> 
> The inherited value should not be stored, but used
> in the computation
> of the property value. This should be implemented by
> the property
> maker.
> 
> When I run the above examples in a debugger, I find
> that the computed
> start-indent values CommonMarginBlock.startIndent
> are exactly like I
> argue above they should be. There does not seem to
> be a need to add
> the inherited value later; the property maker
> already has done so. See
> IndentPropertyMaker.compute(PropertyList). It uses
>
propertyList.getInherited(baseMaker.propId).getNumeric())
> to get the
> inherited value. Earlier FOP developers understood
> this part well.
> 
> If you find wrong results, then the problem must be
> elsewhere.
> 
> Is there a book or treatise on these subjects, where
> we can read how a
> knowledgeable author interprets these difficult
> parts of the spec?
> 
> Regards, Simon
> 
> On Fri, Jan 07, 2005 at 09:26:15AM +0100, Jeremias
> Maerki wrote:
> > Finn or Simon,
> > 
> > would you please check if it is acceptable to put
> the inherited values
> > directly into the CommonMarginBlock? It might have
> been cleaner to
> > always get the value via the parent FO but I think
> in this case it helps
> > simplifying the code in TraitSetter and
> BlockLayoutManager.
> > 
> > On 07.01.2005 09:21:21 jeremias wrote:
> > > jeremias    2005/01/07 00:21:21
> > > 
> > >   Modified:   
> src/java/org/apache/fop/fo/properties
> CommonMarginBlock.java
> > >                src/java/org/apache/fop/layoutmgr
> TraitSetter.java
> > >                         BlockLayoutManager.java
> > >   Log:
> > >   Bugfix for start-indent calculation for nested
> blocks. The inherited start-indent wasn't taken into
> account as described in 5.3.2 of the spec.
> > >   Minor style and javadoc improvements on the
> way.
> > 
> > <snip/>
> > 
> > >   Revision  Changes    Path
> > >   1.5       +34 -2    
>
xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java
> > >   
> > >   Index: CommonMarginBlock.java
> > >  
>
===================================================================
> > >   RCS file:
>
/home/cvs/xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java,v
> > >   retrieving revision 1.4
> > >   retrieving revision 1.5
> > >   diff -u -r1.4 -r1.5
> > >   --- CommonMarginBlock.java	28 Oct 2004
> 10:00:24 -0000	1.4
> > >   +++ CommonMarginBlock.java	7 Jan 2005 08:21:21
> -0000	1.5
> > >   @@ -1,5 +1,5 @@
> > >    /*
> > >   - * Copyright 1999-2004 The Apache Software
> Foundation.
> > >   + * Copyright 1999-2005 The Apache Software
> Foundation.
> > >     * 
> > >     * Licensed under the Apache License, Version
> 2.0 (the "License");
> > >     * you may not use this file except in
> compliance with the License.
> > >   @@ -70,6 +70,16 @@
> > >        public Length endIndent;
> > >    
> > >        /**
> > >   +     * The inherited "start-indent" property.
> > >   +     */
> > >   +    public Length inheritedStartIndent;
> > >   +
> > >   +    /**
> > >   +     * The inherited "end-indent" property.
> > >   +     */
> > >   +    public Length inheritedEndIndent;
> > >   +
> > >   +    /**
> > >         * Create a CommonMarginBlock object.
> > >         * @param pList The PropertyList with
> propery values.
> > >         */
> > >   @@ -84,5 +94,27 @@
> > >    
> > >            startIndent =
> pList.get(Constants.PR_START_INDENT).getLength();
> > >            endIndent =
> pList.get(Constants.PR_END_INDENT).getLength();
> > >   +        
> > >   +        if
> (!pList.getFObj().generatesReferenceAreas()) {
> > >   +            inheritedStartIndent =
> pList.getParentPropertyList()
> > >   +                   
> .get(Constants.PR_START_INDENT).getLength();
> > >   +            inheritedEndIndent =
> pList.getParentPropertyList()
> > >   +                   
> .get(Constants.PR_END_INDENT).getLength();
> > >   +        }
> > >   +    }
> > 
> > <snip/>
> > 
> > 
> > 
> > Jeremias Maerki
> > 
> 
> -- 
> Simon Pepping
> home page: http://www.leverkruid.nl
> 
> 


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Simon Pepping <sp...@leverkruid.nl>.
Section 5.3.2 of the spec is really hard to understand. I combine it
with 5.1.4 about Inheritance. Then my guess is this:

<fo:block margin-left="1pc">A test file
  <fo:inline-container start-indent="1.5pc">
    <fo:block margin-left="1pc">A test file</fo:block>
  </fo:inline-container>
</fo:block>

The computed value of start-indent on the outer block is 'start-indent
= inherited_value_of(start-indent) + margin-corresponding +
padding-corresponding + border-corresponding-width' = 0 + 1pc + 0 +
0. The computed value of start-indent on the inner block is
'start-indent = inherited_value_of(start-indent) +
margin-corresponding + padding-corresponding +
border-corresponding-width' = 1.5pc + 1pc + 0 + 0.

In this case:

<fo:block margin-left="1pc">A test file
  <fo:inline-container>
    <fo:block margin-left="1pc">A test file</fo:block>
  </fo:inline-container>
</fo:block>

the computed value of start-indent on the outer block is 'start-indent
= inherited_value_of(start-indent) + margin-corresponding +
padding-corresponding + border-corresponding-width' = 0 + 1pc + 0 +
0. The computed value of start-indent on the inner block is
'start-indent = inherited_value_of(start-indent) +
margin-corresponding + padding-corresponding +
border-corresponding-width' = 1pc + 1pc + 0 + 0. The inherited value
uses the calculated value (sect. 5.1.4). That is the value that should
be returned by
pList.getParentPropertyList().get(Constants.PR_START_INDENT).getLength().

The inherited value should not be stored, but used in the computation
of the property value. This should be implemented by the property
maker.

When I run the above examples in a debugger, I find that the computed
start-indent values CommonMarginBlock.startIndent are exactly like I
argue above they should be. There does not seem to be a need to add
the inherited value later; the property maker already has done so. See
IndentPropertyMaker.compute(PropertyList). It uses
propertyList.getInherited(baseMaker.propId).getNumeric()) to get the
inherited value. Earlier FOP developers understood this part well.

If you find wrong results, then the problem must be elsewhere.

Is there a book or treatise on these subjects, where we can read how a
knowledgeable author interprets these difficult parts of the spec?

Regards, Simon

On Fri, Jan 07, 2005 at 09:26:15AM +0100, Jeremias Maerki wrote:
> Finn or Simon,
> 
> would you please check if it is acceptable to put the inherited values
> directly into the CommonMarginBlock? It might have been cleaner to
> always get the value via the parent FO but I think in this case it helps
> simplifying the code in TraitSetter and BlockLayoutManager.
> 
> On 07.01.2005 09:21:21 jeremias wrote:
> > jeremias    2005/01/07 00:21:21
> > 
> >   Modified:    src/java/org/apache/fop/fo/properties CommonMarginBlock.java
> >                src/java/org/apache/fop/layoutmgr TraitSetter.java
> >                         BlockLayoutManager.java
> >   Log:
> >   Bugfix for start-indent calculation for nested blocks. The inherited start-indent wasn't taken into account as described in 5.3.2 of the spec.
> >   Minor style and javadoc improvements on the way.
> 
> <snip/>
> 
> >   Revision  Changes    Path
> >   1.5       +34 -2     xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java
> >   
> >   Index: CommonMarginBlock.java
> >   ===================================================================
> >   RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java,v
> >   retrieving revision 1.4
> >   retrieving revision 1.5
> >   diff -u -r1.4 -r1.5
> >   --- CommonMarginBlock.java	28 Oct 2004 10:00:24 -0000	1.4
> >   +++ CommonMarginBlock.java	7 Jan 2005 08:21:21 -0000	1.5
> >   @@ -1,5 +1,5 @@
> >    /*
> >   - * Copyright 1999-2004 The Apache Software Foundation.
> >   + * Copyright 1999-2005 The Apache Software Foundation.
> >     * 
> >     * Licensed under the Apache License, Version 2.0 (the "License");
> >     * you may not use this file except in compliance with the License.
> >   @@ -70,6 +70,16 @@
> >        public Length endIndent;
> >    
> >        /**
> >   +     * The inherited "start-indent" property.
> >   +     */
> >   +    public Length inheritedStartIndent;
> >   +
> >   +    /**
> >   +     * The inherited "end-indent" property.
> >   +     */
> >   +    public Length inheritedEndIndent;
> >   +
> >   +    /**
> >         * Create a CommonMarginBlock object.
> >         * @param pList The PropertyList with propery values.
> >         */
> >   @@ -84,5 +94,27 @@
> >    
> >            startIndent = pList.get(Constants.PR_START_INDENT).getLength();
> >            endIndent = pList.get(Constants.PR_END_INDENT).getLength();
> >   +        
> >   +        if (!pList.getFObj().generatesReferenceAreas()) {
> >   +            inheritedStartIndent = pList.getParentPropertyList()
> >   +                    .get(Constants.PR_START_INDENT).getLength();
> >   +            inheritedEndIndent = pList.getParentPropertyList()
> >   +                    .get(Constants.PR_END_INDENT).getLength();
> >   +        }
> >   +    }
> 
> <snip/>
> 
> 
> 
> Jeremias Maerki
> 

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


Re: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr TraitSetter.java BlockLayoutManager.java

Posted by Jeremias Maerki <de...@greenmail.ch>.
Finn or Simon,

would you please check if it is acceptable to put the inherited values
directly into the CommonMarginBlock? It might have been cleaner to
always get the value via the parent FO but I think in this case it helps
simplifying the code in TraitSetter and BlockLayoutManager.

On 07.01.2005 09:21:21 jeremias wrote:
> jeremias    2005/01/07 00:21:21
> 
>   Modified:    src/java/org/apache/fop/fo/properties CommonMarginBlock.java
>                src/java/org/apache/fop/layoutmgr TraitSetter.java
>                         BlockLayoutManager.java
>   Log:
>   Bugfix for start-indent calculation for nested blocks. The inherited start-indent wasn't taken into account as described in 5.3.2 of the spec.
>   Minor style and javadoc improvements on the way.

<snip/>

>   Revision  Changes    Path
>   1.5       +34 -2     xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java
>   
>   Index: CommonMarginBlock.java
>   ===================================================================
>   RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/fo/properties/CommonMarginBlock.java,v
>   retrieving revision 1.4
>   retrieving revision 1.5
>   diff -u -r1.4 -r1.5
>   --- CommonMarginBlock.java	28 Oct 2004 10:00:24 -0000	1.4
>   +++ CommonMarginBlock.java	7 Jan 2005 08:21:21 -0000	1.5
>   @@ -1,5 +1,5 @@
>    /*
>   - * Copyright 1999-2004 The Apache Software Foundation.
>   + * Copyright 1999-2005 The Apache Software Foundation.
>     * 
>     * Licensed under the Apache License, Version 2.0 (the "License");
>     * you may not use this file except in compliance with the License.
>   @@ -70,6 +70,16 @@
>        public Length endIndent;
>    
>        /**
>   +     * The inherited "start-indent" property.
>   +     */
>   +    public Length inheritedStartIndent;
>   +
>   +    /**
>   +     * The inherited "end-indent" property.
>   +     */
>   +    public Length inheritedEndIndent;
>   +
>   +    /**
>         * Create a CommonMarginBlock object.
>         * @param pList The PropertyList with propery values.
>         */
>   @@ -84,5 +94,27 @@
>    
>            startIndent = pList.get(Constants.PR_START_INDENT).getLength();
>            endIndent = pList.get(Constants.PR_END_INDENT).getLength();
>   +        
>   +        if (!pList.getFObj().generatesReferenceAreas()) {
>   +            inheritedStartIndent = pList.getParentPropertyList()
>   +                    .get(Constants.PR_START_INDENT).getLength();
>   +            inheritedEndIndent = pList.getParentPropertyList()
>   +                    .get(Constants.PR_END_INDENT).getLength();
>   +        }
>   +    }

<snip/>



Jeremias Maerki