Team,

To simplify the Area Tree<-->Renderer interaction somewhat, making this section of the code easier to follow, I'd like to make two changes to the code:

1.) Remove the serveVisitor() methods in AbstractRenderer.java [1], and return to what we were using last year, that of having the InlineArea call the render() methods. This will allow us to drop the area.inline.InlineAreaVisitor interface[2] as well as the sundry "acceptVisitor" methods in the inlineArea subclasses[3, for one example of the 8-9 places this occurs].

Currently developing with them around IMO makes comprehension, coding and debugging more difficult. They also add an additional level of indirection in the code where there is enough complexity already. If there's a need for these patterns in the future, e.g., for more Area Tree pluggability, we can bring them back, but upon actual demand and after things are finished in our coding.

Anyway, here's my +1 for this change.

[1] http://cvs.apache.org/viewcvs.cgi/xml-fop/src/java/org/apache/fop/render/AbstractRenderer.java?r1=1.12&r2=1.13&diff_format=h
[2] http://cvs.apache.org/viewcvs.cgi/xml-fop/src/java/org/apache/fop/area/inline/InlineAreaVisitor.java?diff_format=h&rev=1.2&view=auto
[3] http://cvs.apache.org/viewcvs.cgi/xml-fop/src/java/org/apache/fop/area/inline/Character.java?r1=1.1&r2=1.2


---------------------------------------------------------------------

2.) (This I'm less sure on) After reverting, I'd like to remove the "render" functions within the InlineArea objects in favor of direct function calls within AbstractRenderer:

i.e., move from here in render.AbstractRenderer:

protected void renderLineArea(LineArea line) {
       List children = line.getInlineAreas();

       for (int count = 0; count < children.size(); count++) {
           InlineArea inline = (InlineArea) children.get(count);
           inline.render(this);
       }
   }

to here:

protected void renderLineArea(LineArea line) {
       List children = line.getInlineAreas();

for (int count = 0; count < children.size(); count++) {
InlineArea inline = (InlineArea) children.get(count);
renderInlineArea(inline); // may need expansion, with instanceof() operators.
}
}



This will result in allowing us to remove several render() functions from the InlineArea objects, and remove the bounce-back between Renderers and Area objects, further simplifying the coding. The drawback to this method is that we may need some instanceof() operators in order to choose the proper render method. (Different InlineAreas currently have different rendering functions.)


I'm almost +1 on this, but am awaiting more comments on the performance issues involved. Performance is the my only concern with this change (this section of the code is very highly called)--I otherwise like the simplification that this change will offer.

Thanks,
Glen




Reply via email to