DavBfr / dart_pdf

Pdf creation module for dart/flutter
https://pub.dev/packages/pdf
Apache License 2.0
1.42k stars 632 forks source link

feat: Table cell spans #1736

Open Gustl22 opened 2 months ago

Gustl22 commented 2 months ago

Add's the col and rowspan feature for tables.

Closes #989, #983, #896, #645, #313, #256, #167

Gustl22 commented 2 months ago

IntrinsicColumnWidth is not yet tested. Also I didn't get the whole concept, so there might be some edge cases, which I haven't considered (especially in the paint method, where _widths are consumed).

DavBfr commented 2 months ago

Can you add a test?

Gustl22 commented 2 months ago

Sure, I just have limited resources and vacation soon, so if someone wants to finish up, I don't mind. I'll get back to it in a few weeks.

Gustl22 commented 2 months ago

I added a test, but it does not precisely check e.g. the different box properties. If this should be also done, let me know and how to best approach it.

Gustl22 commented 2 months ago

I have a small problem: the tests now complain, that there's a difference (which is indeed in the renders). But if I inspect this in a pdf viewer / firefox / chrome etc. There isn't any difference visisble:

diff-3 image

I suspect the test renderer interprets this as small gap and therefore outputs the mean as grey? widgets-table.pdf

Should we ignore this or solve it another way?

DavBfr commented 2 months ago

Yes, that's possible. Seems to be at the intersection of each cell.

Gustl22 commented 2 months ago

To allow col spans the border must be calculated for each cell and not for the whole width / height of the table any more. So there now exist infinite small fake gaps, which in theory should not be visible, and also aren't in my inspection in the pdf readers, just the pdftocairo does this I think...

DavBfr commented 2 months ago

That's fine then.

Gustl22 commented 2 months ago

I now wonder, if it would make sense to introduce a new widget, like TableCell where one can define the col and row span. Otherwise I don't see a good way to support row spans in the future. Also this would remove potential errors in defining the col spans on the Row level.

See also: https://github.com/flutter/packages/pull/5917 https://www.w3.org/TR/css-tables-3/#height-distribution

Gustl22 commented 1 month ago

Row span is now also supported via TableCell:

image

I updated the goldens and saved the diffs here: diff.zip

Gustl22 commented 1 month ago

I now used an improved algorithm for rowspanning. There's a special case where the remaining space it not quite optimized, when multiple cells with rowpans overlap and in one row and no cell (without rowspan) with height is given to that row. This is the case, as there's no way to know beforehand how much space a second cell with rowspan would occupy in the current row, as it could use its space on the next row also. So we would need a third round of calculation who minimizes these gaps, or an optimization function in general. I found this quite hard to solve, but it occurs in only a very small amount of cases (I think). So I would leave it like that for now and if this needs improving, do it afterwards (?).