search

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

Table widget refactors #1776

Open grahamsmith opened 1 week ago

grahamsmith commented 1 week ago

Started looking to see if the Multipage and Table performance can be enhanced with larger datasets.

Thought a small PR to start with would be easier.

Refactored to eliminate nullable doubles and reduce force unboxing.

grahamsmith commented 1 week ago

@DavBfr I think my formatter is different to yours, are you at 80 chars?

DavBfr commented 1 week ago

@DavBfr I think my formatter is different to yours, are you at 80 chars?

Yes I am using the default remanded dart formatting.

grahamsmith commented 1 week ago

I'll update with 80 chars.

It'll be the same code.

grahamsmith commented 1 week ago

@DavBfr - sorted - line lengths are 80 so the diff is simpler now

Unsure about the CI failures from the last run

grahamsmith commented 1 week ago

@DavBfr - in my next PR I have extracted the width calculation to its own function, and added it what I hope would be a decent performance enhancement to reuse the calculations if all the column widths are fixed. It would mean we wouldn't have to calculate for every multipage page the width again.

grahamsmith commented 1 week ago

@DavBfr - sorry not relevant to this PR but do not know where else to post as too excited

before my changes for a demo pdf with 10,000 items into a table it took 0:00:57.012834 my new fork with changes 0:00:11.770033

Nothing too major either!

Much excite

DavBfr commented 1 week ago

Congrats!

grahamsmith commented 1 week ago

I noticed the checks have passed, do I need to do anything else @DavBfr ?

DavBfr commented 1 week ago

Just need some time to review.

Gustl22 commented 1 week ago

@grahamsmith this will potentially conflicting with #1736, where some changes to the algorithm were made. Hope we find a good solution to make it perform good also with cell spans enabled (haven't tested the performance yet, though).

grahamsmith commented 1 week ago

I imagine it will be terrible, although a potential useful feature.

I have more changes in the background that greatly enhance the performance of the layout as its quite slow as other's have mentioned.

Wonder if the spanning could be in a specific set of widgets. In a perfect world would really love to know how many users need cell spanning (clearly you did).

grahamsmith commented 1 week ago

@Gustl22 - do you know when your changes will land? I am not particularly precious about this PR. One of the biggest gains is in MultiPage so I can do a seperate PR for that and hopefully it will not clash.

Gustl22 commented 1 week ago

do you know when your changes will land?

No I don't, open source takes time, just wanted to make aware of this conflict. I am also lacking the time to review my stuff in my repos.

Wonder if the spanning could be in a specific set of widgets. In a perfect world would really love to know how many users need cell spanning (clearly you did).

At least 14 people created or upvoted an issue regarding that. And I think the most people are working around this by creating nested tables or column and row layouts, but that's not the clean way. I don't mind which one is merged first, I also have to create performance tests.

There surely can made also optimizations, especially if no cell span is present.