Re: [lmi] Pagination anomaly

[Vadim Zeitlin]

On Mon, 5 Feb 2018 20:24:17 +0100 I wrote:

Me> but I still have a few more problems/questions with/about this code.
Me> 
Me>  First one is that it's painfully obvious that I can't avoid bugs in it, so
Me> it would be great to have tests verifying that this works correctly.

 While still thinking about a better way to do it, I've at least written
and run the following program:

---------------------------------- >8 --------------------------------------
#include <catch.hpp>

constexpr int rows_per_group = 5;
constexpr int row_height = 12;
constexpr int get_footer_top() { return 500; }

int get_extra_pages_needed(int pos_y_top, int total_years)
{
   ... minimally adapted code from ledger_pdf_generator_wx.cpp ...
}

int render(int pos_y_top, int year_max)
{
   ... same as above ...
}

TEST_CASE()
{
    for(int years = 1; years <= 200; ++years)
        {
        for(int pos_y_top = 0; pos_y_top < 200; ++pos_y_top)
            {
            INFO("years = " << years << ", pos = " << pos_y_top);
            REQUIRE( get_extra_pages_needed(pos_y_top, years) == 
render(pos_y_top, years) );
            }
        }
}
---------------------------------- >8 --------------------------------------

and swiftly found another off-by-one problem which requires this fix:

---------------------------------- >8 --------------------------------------
diff --git a/ledger_pdf_generator_wx.cpp b/ledger_pdf_generator_wx.cpp
index 111eb0d..117ca74 100644
--- a/ledger_pdf_generator_wx.cpp
+++ b/ledger_pdf_generator_wx.cpp
@@ -1651,7 +1651,7 @@ class page_with_tabular_report
                     // And possibly a page break, which will be necessary if 
we don't
                     // have enough space for another full group because we 
don't want
                     // to have page breaks in the middle of a group.
-                    if(pos_y >= page_bottom - rows_per_group*row_height)
+                    if(pos_y > page_bottom - rows_per_group*row_height)
                         {
                         next_page(writer);
                         numbered_page::render(ledger, writer, 
interpolate_html);
---------------------------------- >8 --------------------------------------

which I've added as the third commit to https://github.com/vadz/lmi/pull/63


 The good news is that with this fix the test above passes, i.e. all 40000
combinations of the number of years and the space available for them
results in both functions returning the same values. This doesn't mean that
these values are correct, of course, e.g. the third problem mentioned in my
previous reply still exists, but it gives me at least some measure of
confidence in this code (better late than never).

 Regards,
VZ