On 11.09.2008 22:59:31 Vincent Hennebert wrote: > Jeremias Maerki wrote: > > I knew full well when I committed this that there's a certain > > probability that you're not going to be happy with how I dit it. It's > > hard to please you these days. So, is this post of yours a veto? > > Depends. The thing is, this issue will most probably show up also with > the new layout approach, and it would be good to have it sorted out > before. > > > > At any rate, I don't think that generating FOs for missing table cells > > is the right approach. > > Why?
Elaborating on "additional overhead": You would generate additional FOs for non-existent table-cells. For each FO, a layout manager is created which will also need to be called. Table layout will need to run through more cells per row than necessary even though they're empty. More objects, more CPU cycles. Granted we're probably not talking about a lot of empty table-cells but still. RowPainter is area-generation stage. My solution is just making sure areas are generated with the information that is available anyway. It's not doing anything in a place where it shouldn't do it. The change is confined to a narrow space. Your solution spans FO tree, layout and area generation. So I'm not convinced that my solution is bad or that your solution is better. > > As can be seen in my commit, it is perfectly > > possible to do without the additional overhead involved with your > > solution. > > But not without additional code duplication. > > > > On 11.09.2008 14:11:31 Vincent Hennebert wrote: > >> Hi, > >> > >>> Author: jeremias > >>> Date: Fri Aug 29 09:36:17 2008 > >>> New Revision: 690319 > >>> > >>> URL: http://svn.apache.org/viewvc?rev=690319&view=rev > >>> Log: > >>> Added missing generation of areas for empty grid units in tables with > >>> collapsing border model. > >> <snip/> > >> > >> I’m not happy with that change. Basically it’s code copy-pasted from > >> several places and slightly modified to handle missing cells. I didn’t > >> spend months trying to clean up the table layout code to see that kind > >> of change introduced at the first opportunity... > >> > >> A bit of background first: distinction needs to be made between /empty/ > >> cells (existing table-cell elements with no content), and /missing/ > >> cells (missing table-cell element; e.g., only three cells on a row in > >> a four-column table). The EmptyGridUnit class is meant to handle that > >> latter case only (yes, it’s misnamed). I didn’t have the opportunity of > >> addressing that issue when I worked on tables, so I left it as is. > >> > >> That EmptyGridUnit class doesn’t fit well in the big picture. It > >> constantly requires special care just because you can’t get a non-null > >> TableCell object from it. The only way I could think of to handle that > >> properly is to ‘fix’ the FO tree, completing it by adding empty > >> table-cell elements. Basically in FixedColRowGroupBuilder.endRow(), > >> instead of filling the gaps with EmptyGridUnit objects, proper TableCell > >> objects would be created in some way. > >> > >> To achieve that we need to be able to create FONode objects outside of > >> the FOTreeBuilder machinery. Or to trigger the proper events on > >> FOTreeBuilder so that new TableCell objects are ‘automatically’ created. > >> I haven’t had the opportunity to look into it into more details, but I’d > >> rather explore this possibility, instead of dealing with EmptyGridUnit. > >> If FO tree specialists have any comments or suggestions... > >> > >> > >> Thanks, > >> Vincent > > > > > > > > > > Jeremias Maerki > > Vincent Jeremias Maerki
