Re: [lmi] Group-quote PDF: whitespace changes, and enhancement

[Greg Chicares]

On 2018-03-10 01:14, Vadim Zeitlin wrote:
> On Sat, 10 Mar 2018 00:34:27 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> Do we have 126 en available? If so, then we just lay out this table with
> GC> one space between columns, and it has to fit.
> 
>  It does fit if we allow having no column margin at all (which we more or
> less have to do as there is just 1 pixel of it left for this table anyhow):
> 
> ---------------------------------- >8 --------------------------------------
> diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
> index 604351e3e..5b96d9059 100644
> --- a/wx_table_generator.cpp
> +++ b/wx_table_generator.cpp
> @@ -211,7 +211,7 @@ void 
> wx_table_generator::do_compute_column_widths_if_necessary()
>              // fit into the available width.
>              auto const overflow_per_column =
>                  (overflow + num_columns - 1)/num_columns;
> -            if(overflow_per_column <= column_margin_)
> +            if(overflow_per_column <= 2*column_margin_)
>                  {
>                  // We are going to reduce the total width by more than
>                  // necessary, in general, because of rounding up above, so
> ---------------------------------- >8 --------------------------------------

Thanks, this change fixes the problem at hand, so I've committed it.
But now please help me understand it: I think I just don't quite
understand exactly what the variable names mean.

I thought I understood the original version of the changed line: I'd
express it in natural language as:

As originally laid out, the table is too wide. Calculate the number
of pixels by which it overflows--for the whole table:
  auto const overflow = total_fixed - total_width_;
    where total_width_ is the width of the page, e.g., 210mm for A4
    and total_fixed is width of all fixed-width columns, as
      originally laid out
...and also for each column:
  // We need to round up in division here to be sure that all columns
  // fit into the available width.
  auto const overflow_per_column =
      (overflow + num_columns - 1)/num_columns;
Now determine whether reducing the margins will make the table fit.
If that works, then do it; else don't do it, and print a warning.
In the original version of that line:
  if(overflow_per_column <= column_margin_)
I'd guess that column_margin_ is the number of pixels between
columns, as the table was originally laid out--which, as we just
determined, was too generous, so we're going to try reducing it.
Then I'd read that conditional as comparing
  the number of pixels by which we must shrink each column, to
  the number of pixels of padding between columns
and it naturally makes sense: reducing the padding is a workable
strategy if the desired reduction is less than the padding. For
example, if each column has
   5 pixels of padding, plus
  10 pixels of formatted text
and we'd like to reduce that total of 15 to 12, then we need to
remove 3 pixels; and 3 < 5, so we can do it, resulting in:
   2 pixels of padding, plus
  10 pixels of formatted text

So I thought I understood the original, but now the modified line
seems to say that this strategy works if
  desired reduction is <= TWO TIMES the padding
Thus, in the example above, if we want to reduce the total from
15 to 9, we can achieve that reduction of 6 pixels, because
  6 < 2 * 5
resulting in
  -1 pixels of padding, plus
  10 pixels of formatted text
which would make a 10-pixel column fit in a 9-pixel space. But
obviously I've misunderstood something, because the patch works.
Does that mean I've misunderstood column_margin_?