More collapsing borders... (RE: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr/table TableContentLayoutManager.java)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

More collapsing borders... (RE: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr/table TableContentLayoutManager.java)

Andreas L Delmelle
> -----Original Message-----
> From: [hidden email] [mailto:[hidden email]]
>

Hi Jeremias and others interested in table-borders,


(Sorry for the --again-- long post, but...)
The following comment in the code (TCLM) got me wondering...

> //Create empty grid units to hold resolved borders of neighbouring cells
> //TODO maybe this needs to be done differently (and sooner)

... if we're on the same frequency here --and to avoid re-inventing the
wheel ;-)
Have you also considered --even if only partly-- normalizing table borders
during the FOTree-building? (Or do you only mean 'sooner' in the layout
process?)

This is something I'm still pondering over, as, for instance the following
fragment (supposing a collapsing model, easy case: no borders specified
higher up):

<fo:table-row border="solid 4pt">
  <fo:table-cell border="solid 2pt">
    <!-- block content -->
  </fo:table-cell>
  <!-- more cells -->
</fo:table-row>
<fo:table-row>
  <fo:table-cell border="solid 3pt">
    <!-- block content -->
  </fo:table-cell>
  <!-- more cells -->
</fo:table-row>

AFAICT could be expanded in one go into:
(I'm not completely sure, but aren't the shorthands expanded internally? If
so, can we use that to our advantage?)

<fo:table-row border-before="solid 4pt"
              border-after="solid 4pt"
              border-start="solid 4pt"
              border-end="solid 4pt">
  <fo:table-cell border-before="solid 4pt"
                 border-after="solid 4pt"
                 border-start="solid 4pt"
                 border-end="...">
    <!-- block content -->
  </fo:table-cell>
  <!-- more cells -->
</fo:table-row>
<fo:table-row border-before="solid 4pt"
              border-start="solid 3pt">
  <fo:table-cell border-before="solid 4pt"
                 border-after="solid 3pt"
                 border-start="solid 3pt"
                 border-end="...">
    <!-- block content -->
  </fo:table-cell>
  <!-- more cells -->
</fo:table-row>

Well, apart from a few liberties I may have taken here, I think the idea is
clear enough that, *if* this could be pulled off, this may greatly simplify
the related portions in the layout code... Evidently, at that point we would
only be able to compare with cells that were previously read --events
already occured--, so at first sight it only solves part of the problem, but
anyway...

In the simplest cases --i.e. no breaks and no spans-- the right
border-widths need only be _read_ by the LM from the FObjs, and the LM only
needs to decide whether to add (separated borders) or take into account the
one with the highest precedence (collapsing borders).

Further motivation: in principle, only the widths of the borders should be
relevant to the layout code. The current fact that there's an explicit
reference to the border-*styles* in an essentially layout-related class...
Well, somehow it does not sit completely right with me --could be a matter
of taste, but IMO, layout needs to concern itself only with sizes...

The only real hard work in layout would be limited to the two difficult
cases --breaks/spans--, but exactly because only these two would need to be
handled during layout, they would become clearly outlined in the code
--and because the lion's share of the decisions are already made before the
LMs kick off, even this 'hard work' could turn out to be quite
straightforward.

What would it mean for layout if large parts of the 'determineWinner()'
functionality would actually shift somewhere to the Property subsystem and
the FOTree (? where they are ultimately still accessible to Layout if
needed?)

Anyway, the more I read the parts in the XSL-FO and CSS Rec about borders,
the more I get the impression that the collapsing model works:
- partly on the level of the bare properties/FOTree
  the question: which borders are dominant in the general cases?
- and partly on the level of layout
  the question: what is the size of the space assigned
                to the applicable borders in this case?

ATM it's only a thought. Just checking to see if, in anyone's opinion, it
would be worth digging deeper into... if treating the above two questions
separately helps to avoid (or even merely alleviate) certain headaches, all
the better.


Cheers,

Andreas

Reply | Threaded
Open this post in threaded view
|

Re: More collapsing borders... (RE: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr/table TableContentLayoutManager.java)

Jeremias Maerki
Hi Andreas

On 17.05.2005 23:24:55 Andreas L. Delmelle wrote:

> > -----Original Message-----
> > From: [hidden email] [mailto:[hidden email]]
> >
>
> Hi Jeremias and others interested in table-borders,
>
>
> (Sorry for the --again-- long post, but...)
> The following comment in the code (TCLM) got me wondering...
>
> > //Create empty grid units to hold resolved borders of neighbouring cells
> > //TODO maybe this needs to be done differently (and sooner)
>
> ... if we're on the same frequency here --and to avoid re-inventing the
> wheel ;-)
> Have you also considered --even if only partly-- normalizing table borders
> during the FOTree-building? (Or do you only mean 'sooner' in the layout
> process?)

I meant sooner in the layout process, i.e. in the TableRowIterator. I
added that in in a sort of ad-hoc fashion when I needed to fix the
problem of grid units not being present.

> This is something I'm still pondering over, as, for instance the following
> fragment (supposing a collapsing model, easy case: no borders specified
> higher up):
>
> <fo:table-row border="solid 4pt">
>   <fo:table-cell border="solid 2pt">
>     <!-- block content -->
>   </fo:table-cell>
>   <!-- more cells -->
> </fo:table-row>
> <fo:table-row>
>   <fo:table-cell border="solid 3pt">
>     <!-- block content -->
>   </fo:table-cell>
>   <!-- more cells -->
> </fo:table-row>
>
> AFAICT could be expanded in one go into:
> (I'm not completely sure, but aren't the shorthands expanded internally? If
> so, can we use that to our advantage?)

AFAIK, they are expanded while setting up the CommonBorderPaddingBackground
instances in the FO nodes. This class is already mutable so you could,
in theory, manipulate the instances held by the FO nodes.

A bigger problem, one I'm working around by working on the grid unit
list, is the addition of cells to the FO tree if that's necessary. I
experimented with this in another place and dropped it because of the
property expansion mechanism.

> <fo:table-row border-before="solid 4pt"
>               border-after="solid 4pt"
>               border-start="solid 4pt"
>               border-end="solid 4pt">
>   <fo:table-cell border-before="solid 4pt"
>                  border-after="solid 4pt"
>                  border-start="solid 4pt"
>                  border-end="...">
>     <!-- block content -->
>   </fo:table-cell>
>   <!-- more cells -->
> </fo:table-row>
> <fo:table-row border-before="solid 4pt"
>               border-start="solid 3pt">
>   <fo:table-cell border-before="solid 4pt"
>                  border-after="solid 3pt"
>                  border-start="solid 3pt"
>                  border-end="...">
>     <!-- block content -->
>   </fo:table-cell>
>   <!-- more cells -->
> </fo:table-row>
>
> Well, apart from a few liberties I may have taken here, I think the idea is
> clear enough that, *if* this could be pulled off, this may greatly simplify
> the related portions in the layout code...

Greatly simplified or simply done elsewhere? I keep coming back to
handling break cases. Two places in which borders are resolved remain,
nonetheless (break vs. non-break).

> Evidently, at that point we would
> only be able to compare with cells that were previously read --events
> already occured--, so at first sight it only solves part of the problem, but
> anyway...
>
> In the simplest cases --i.e. no breaks and no spans-- the right
> border-widths need only be _read_ by the LM from the FObjs, and the LM only
> needs to decide whether to add (separated borders) or take into account the
> one with the highest precedence (collapsing borders).
>
> Further motivation: in principle, only the widths of the borders should be
> relevant to the layout code.

That's true, if by layout code you mean only the table stepper, i.e. the
generation of the combined element list.

> The current fact that there's an explicit
> reference to the border-*styles* in an essentially layout-related class...
> Well, somehow it does not sit completely right with me --could be a matter
> of taste, but IMO, layout needs to concern itself only with sizes...

That was a natural outcome since there are certain specialities in the
"collapse" model. If you look closely there's a little special case
where the collapsed border between two cells has the same style and
width but potentially different colors. If you follow through the rules
1 to 5 you end up in rule 5 "solid 0.5pt red" specified in one cell,
"solid 0.5pt green" in the neighbouring cell. See
table-border-collapse1.xml in testcases for an example. There's no rule
that says how to decide which of the two wins.

Furthermore, looking at those rules again, you will see that the style
is used to determine winners, styles having implicit precedence values.
That's why you need them aside from having to pipe such information
through to the addAreas() stage where the traits are set.

If you're talking about the border resolution you can't avoid looking at
the style, too. After that, in the table stepper, only the widths are
relevant.

> The only real hard work in layout would be limited to the two difficult
> cases --breaks/spans--, but exactly because only these two would need to be
> handled during layout, they would become clearly outlined in the code
> --and because the lion's share of the decisions are already made before the
> LMs kick off, even this 'hard work' could turn out to be quite
> straightforward.

I still think you're not quite where I am. Take the following methods:
- TableContentLayoutManager.resolveNormalBeforeAfterBordersForRowGroup()
- TableRowIterator.resolveStartEndBorders()

These are the two isolated places where the border resolution for
non-break cases are done. After that, the table stepper simply fetches
the table widths from the grid units to do the right calculations.

The part where I gave up for now was how exactly to handle the
border resolution for break situations and how to bring the values into
the overall calculation.

I get the impression that you're trying to solve a problem that is
already solved, but if you find a better solution (i.e. one that makes
the code faster and/or less memory consuming and/or clearer/easier) then
I'm all for it. I've taken the straight-forward approach creating a data
structure (grid units in EffRows) where I have all the information
together.

> What would it mean for layout if large parts of the 'determineWinner()'
> functionality would actually shift somewhere to the Property subsystem and
> the FOTree (? where they are ultimately still accessible to Layout if
> needed?)

For layout, probably not much. It simply gets its information from a
different source. But going down this road I don't see how you solve
border resolution for break conditions.

Anyway, I designed the CollapsingBorderModel and subclass (one
additional subclass to be added later for collapse-with-precedence) so
it is completely isolated from the other layout code and can easily be
used for both non-break and break conditions.

> Anyway, the more I read the parts in the XSL-FO and CSS Rec about borders,
> the more I get the impression that the collapsing model works:
> - partly on the level of the bare properties/FOTree
>   the question: which borders are dominant in the general cases?

That's the question you came up with to optimize border resolution. As I
said, that might be helpful. My current implementation dumbly takes all
sources together, every time.

> - and partly on the level of layout
>   the question: what is the size of the space assigned
>                 to the applicable borders in this case?

This I don't get. Sorry.

> ATM it's only a thought. Just checking to see if, in anyone's opinion, it
> would be worth digging deeper into... if treating the above two questions
> separately helps to avoid (or even merely alleviate) certain headaches, all
> the better.

We should ask a pharma company for sponsoring. As a nice side-effect we
might get free headache pills for the team. :-)

Joke aside, if you see a way to make the current approach considerably
faster and less wasteful, then yes, it's worth digging deeper. But
before you do that, make sure you understand what the collapsing model
together with break conditions have as consequence for the table stepper.
On the paper (i.e. in our example on the Wiki) the problem is already
solved. What I had trouble with was not the border resolution, but the
step after that. I guess it should be functionality before speed,
especially when most of the issues are already solved in some way.

HTH

Jeremias Maerki

Reply | Threaded
Open this post in threaded view
|

RE: More collapsing borders... (RE: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr/table TableContentLayoutManager.java)

AttikDwella
> -----Original Message-----
> From: Jeremias Maerki [mailto:[hidden email]]
>

Hi,

(A very short one this time --only one clarification...)

> On 17.05.2005 23:24:55 Andreas L. Delmelle wrote:
>
> > Anyway, the more I read the parts in the XSL-FO and CSS Rec
> about borders,
> > the more I get the impression that the collapsing model works:
> > - partly on the level of the bare properties/FOTree
> >   the question: which borders are dominant in the general cases?
>
> That's the question you came up with to optimize border resolution. As I
> said, that might be helpful. My current implementation dumbly takes all
> sources together, every time.
>
> > - and partly on the level of layout
> >   the question: what is the size of the space assigned
> >                 to the applicable borders in this case?
>
> This I don't get. Sorry.

Funny... 'This you don't get', or 'this' you don't get? May have something
to do with my using 'this' where I meant to say 'the specific case', as in
'taking into account breaks/spans'.
The first question could be solved separately (earlier), and would always
yield the dominant borders in the general cases.
*After* that question has been answered, there are still the possibilities
of page/column-breaks or row/column-spanning cells (where the generally
dominant border we already have, _may_ turn out to be incorrect)

In a very straightforward table, taking up half a page or so, and with
exactly one table grid unit per cell (no spans), the second question becomes
irrelevant and never needs to be asked.



Cheers,

Andreas

Reply | Threaded
Open this post in threaded view
|

RE: More collapsing borders... (RE: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr/table TableContentLayoutManager.java)

Andreas L Delmelle
In reply to this post by Jeremias Maerki
> -----Original Message-----
> From: Jeremias Maerki [mailto:[hidden email]]
>

Hi,

(OK, apologies for my alias turning up... Completely unintended.
 here's the remainder of the response. Other comments, much longer than the
previous one...)

> On 17.05.2005 23:24:55 Andreas L. Delmelle wrote:
>
<snip />

> > The current fact that there's an explicit reference to the
> > border-*styles* in an essentially layout-related class...
> > Well, somehow it does not sit completely right with me --could
> > be a matter of taste, but IMO, layout needs to concern
> > itself only with sizes...
>
> That was a natural outcome since there are certain specialities in the
> "collapse" model. If you look closely there's a little special case
> where the collapsed border between two cells has the same style and
> width but potentially different colors. If you follow through the rules
> 1 to 5 you end up in rule 5 "solid 0.5pt red" specified in one cell,
> "solid 0.5pt green" in the neighbouring cell. See
> table-border-collapse1.xml in testcases for an example. There's no rule
> that says how to decide which of the two wins.

Very good point! Still, I would argue in that case that the layout engine
only needs to know that the border is going to be 0.5pt wide in case of no
breaks/spans. It is, strictly speaking, only when rendering the borders that
the implementation has to know for sure which colour to use.

If we are at liberty to determine that rule ourselves, I would obviously
feel much for an approach like 'before/start cell wins' or 'after/end cell
loses', and this as early in the process as possible. That would be very
convenient. Another idea is to use the color value to decide, or if you
really want a nasty solution that is bound to turn up in a bug report: XOR
the color values. "A bug?? Shame on you for having no sense of humour! Read
the CSS Spec and study your table-border source very closely. It's *all* in
there!" :-)

> Furthermore, looking at those rules again, you will see that the style
> is used to determine winners, styles having implicit precedence values.

Yes, but this could also be done when creating the FObjs --the styles map to
a numeric constant anyway. If we place them in the right precedence-order in
the Constants interface(*), we might not even need to assign precedence
values in a separate step. We'd directly use the integer value they map
to...?

> That's why you need them aside from having to pipe such information
> through to the addAreas() stage where the traits are set.

So, I would seem to disagree? Up until the 'aside from': the _latter_ part
is obviously necessary...

> If you're talking about the border resolution you can't avoid looking at
> the style, too. After that, in the table stepper, only the widths are
> relevant.

Getting hotter, I feel it! :-)

<snip />
> I still think you're not quite where I am. Take the following methods:
> - TableContentLayoutManager.resolveNormalBeforeAfterBordersForRowGroup()
> - TableRowIterator.resolveStartEndBorders()
>
> These are the two isolated places where the border resolution for
> non-break cases are done. After that, the table stepper simply fetches
> the table widths from the grid units to do the right calculations.

... and a large part of what happens in the above two methods --in the
background-- would already be dealt with in the FOTree, so the *only* parts
of the code remaining there would be dealing with the break-cases and the
span-cases. Very layout-specific stuff indeed!
Quite simply put: if a GridUnit corresponds with exactly one TableCell, in a
collapsing model, the *only* border-widths you'd really need to take into
account would be those of its TableCell. The rest I see as ruled out at that
point, *unless* in the following three cases
- after-border => still need to compare that to the Cell/GridUnit
immediately below
- row/column-spans => check the appropriate TableRow/TableColumn for start-
or end-borders
- page-breaks => check the TableBody/-Header/-Footer for the before- or
after-borders

>
> The part where I gave up for now was how exactly to handle the
> border resolution for break situations and how to bring the values into
> the overall calculation.

Resolution to be done by layout in the break-situations would come down to:

* First, compare after-border with GridUnit below.
  This step is always taken (see above) and yields
  the after-border in no-break situation.
  The dominant border is used in the following step.

* Then, only before- or after-borders matter here, so
    either the current GridUnits' TableCell
      (possibly corrected through the first step)
    or the TableBody/-Header/-Footer
  will yield the right border-width.

Resolution between TableCell and TableRow would already have been handled in
the FOTree. Same goes for the resolution between TableColumn and
TableBody/-Header/-Footer. If the order of fo:table-header, -footer
and -body is enforced by the Rec --so, header and footer *have* to appear
before body in the FO source--, then the same could be done for the
after/before-border pairs between header/footer and body.

Which would make the related layout code far less demanding for the
uninitiated :-)

For the real 'collapsing' mechanism, we'd have to point them to the FObjs
and the Properties.

> I get the impression that you're trying to solve a problem that is
> already solved, but if you find a better solution (i.e. one that makes
> the code faster and/or less memory consuming and/or clearer/easier) then
> I'm all for it. I've taken the straight-forward approach creating a data
> structure (grid units in EffRows) where I have all the information
> together.

Which flows naturally from the fact that your coding efforts these last
couple of months have been directed mainly towards the layout engine, I hear
you loud and clear!
But I got to thinking: part of the difficulty and main source of confusion
may exactly be that you're trying to handle *all* of the resolution in
layout... see above (?)
Maybe, approaching border-resolution *not* as a layout-problem can make
those parts of it that *do* need to be handled by layout a bit easier... and
who knows, maybe even more efficient. I'd have to gather more details about
the two approaches to say that for sure --ultimately depends on the
complexity of the table definition, of course....

<snip />
> Joke aside, if you see a way to make the current approach considerably
> faster and less wasteful, then yes, it's worth digging deeper. But
> before you do that, make sure you understand what the collapsing model
> together with break conditions have as consequence for the table stepper.

Yes, the stepper will be the very next thing to look at, but this was more
of a 'first things first' sanity check... Up to here, I get the impression
that what's currently in my head will turn out to be very beneficial to the
rest of the process.

> On the paper (i.e. in our example on the Wiki) the problem is already
> solved. What I had trouble with was not the border resolution, but the
> step after that. I guess it should be functionality before speed,
> especially when most of the issues are already solved in some way.
>

Functionality indeed, but most importantly, functionality where it belongs.
If all goes well, speed and efficiency should necessarily follow from that.


Cheers,

Andreas

[(*) = OT: instead of non-contextually sorting the integer values according
to the alphabetical name of the constants in question, which only _looks_
nice code-wise, but seems to serve no other purpose. What does it mean
'EN_DOTTED' is 'enum constant number/value 36'? When/where is that
information ever going to be relevant? Maybe this line of thinking can also
be applied to other constants...? Admitted, some constants can pop up in a
few different types of situations, so there, we'd need to very careful when
establishing relationships between them at a level so global as the
interface, but in this particular case the keywords are so specific to the
context of border-style, there would be no harm in reserving a set of ints
starting at N and define the constants as:

// Border style constants
EN_INSET = N;
EN_GROOVE = EN_INSET + 1;
...
EN_SOLID = EN_DASHED + 1;
EN_DOUBLE = EN_SOLID + 1;

and we can conveniently compare two border-style constants to see which one
is 'larger' --in any class that implements the interface, at whatever stage
of processing. There may be other scenarios where such functionality would
be useful... and may avoid having to locally re-establish this type of
relationship between constants in classes implementing Constants.]

Reply | Threaded
Open this post in threaded view
|

Re: More collapsing borders... (RE: cvs commit: xml-fop/src/java/org/apache/fop/layoutmgr/table TableContentLayoutManager.java)

gmazza
Andreas L. Delmelle wrote:

>[(*) = OT: instead of non-contextually sorting the integer values according
>to the alphabetical name of the constants in question, which only _looks_
>nice code-wise, but seems to serve no other purpose.
>

I come from an Oracle database background, so this is quite acceptable
(perhaps even ideal) for me.  These constants are like database "primary
keys", and hence their uniqueness and complete lack of business logic to
their individual values is usually a proper design.

If this were a database table and I were the DBA, rather than me
ordering the primary key values in such a way that the business logic
below could directly rely on them, I would create another table column,
specifically name in "border precedence" or something, and then hardcode
the 1-2-3-etc. values in that column.

>What does it mean
>'EN_DOTTED' is 'enum constant number/value 36'? When/where is that
>information ever going to be relevant?
>

Absolutely nothing and hopefully never, respectively, to
database-centric developers...    ;-)

>Maybe this line of thinking can also
>be applied to other constants...? Admitted, some constants can pop up in a
>few different types of situations, so there, we'd need to very careful when
>establishing relationships between them at a level so global as the
>interface, but in this particular case the keywords are so specific to the
>context of border-style, there would be no harm in reserving a set of ints
>starting at N and define the constants as:
>
>// Border style constants
>EN_INSET = N;
>EN_GROOVE = EN_INSET + 1;
>...
>EN_SOLID = EN_DASHED + 1;
>EN_DOUBLE = EN_SOLID + 1;
>
>and we can conveniently compare two border-style constants to see which one
>is 'larger' --in any class that implements the interface, at whatever stage
>of processing. There may be other scenarios where such functionality would
>be useful... and may avoid having to locally re-establish this type of
>relationship between constants in classes implementing Constants.]
>
>
>  
>

I can perhaps rephrase your suggestion in a way that might cause you to
rethink the actual elegance of this idea:  What you're suggesting is
that the table formatting business logic (here, the precedence of
various border styles) should be stored and maintained in the Constants
interface, rather than in the relevant layout classes.

But I see your suggestion actually as more of a performance benefit,
because there is no more computationally efficient way to determine the
"larger" value.  And since these values can be presumably heavily
queried for a table with many rows and columns, it may beneficial for us
to do this for performance reasons alone.  Although another option,
which would probably need to be done anyway if these constants find use
in multiple contexts, is to do a one-time initialization of a static
integer (sparse?) array as follows:

int[] precedence = new int[].....
precedence[EN_INSET] = 1;
precedence[EN_GROOVE] = 2; ... etc.

within the appropriate layout class--keeping this logic nicely
there--and do precedence[EN_XXX] > precedence[EN_YYY], etc. comparisons
instead of EN_XXX > EN_YYY.

But I'm just speculating here--I haven't looked at the code, or (truth
be told...) the relevance of this discussion to the problem at hand.

Glen