Merge pull request #17 from GetSilverfin/nikitam-port-colspan-fixes

Backport code prawn to correctly render colspan table cells

lib/prawn.rb

@@ -6,6 +6,7 @@
 #
 module Prawn #:nodoc:
   VERSION = "1.0.0.rc2"
+  FLOAT_PRECISION = 1.0e-9
 end
 
 require "prawn/utilities"

lib/prawn/table.rb

@@ -6,6 +6,7 @@
 #
 # This is free software. Please see the LICENSE and COPYING files for details.
 
+require 'prawn/table/column_width_calculator'
 require 'prawn/table/cells'
 require 'prawn/table/cell'
 require 'prawn/table/cell/in_table'
@@ -567,21 +568,7 @@ def assert_proper_table_data(data)
     # Returns an array of each column's natural (unconstrained) width.
     #
     def natural_column_widths
-      @natural_column_widths ||=
-        begin
-          widths_by_column = Hash.new(0)
-          cells.each do |cell|
-            next if cell.is_a?(Cell::SpanDummy)
-
-            # Split the width of colspanned cells evenly by columns
-            width_per_column = cell.width.to_f / cell.colspan
-            cell.colspan.times do |i|
-              widths_by_column[cell.column + i] =
-                [widths_by_column[cell.column + i], width_per_column].max
-            end
-          end
-          widths_by_column.sort_by { |col, _| col }.map { |_, w| w }
-        end
+      @natural_column_widths ||= ColumnWidthCalculator.new(cells).natural_widths
     end
 
     # Returns the "natural" (unconstrained) width of the table. This may be

lib/prawn/table/cells.rb

@@ -152,16 +152,7 @@ def style(options={}, &block)
       # Returns the total width of all columns in the selected set.
       #
       def width
-        widths = {}
-        each do |cell|
-          index = cell.column
-          per_cell_width = cell.width_ignoring_span.to_f / cell.colspan
-          cell.colspan.times do |n|
-            widths[cell.column+n] = [widths[cell.column+n], per_cell_width].
-              compact.max
-          end
-        end
-        widths.values.inject(0, &:+)
+        ColumnWidthCalculator.new(self).natural_widths.inject(0, &:+)
       end
 
       # Returns minimum width required to contain cells in the set.
@@ -228,48 +219,7 @@ def index_cells
       # each cell, grouped by +row_or_column+.
       #
       def aggregate_cell_values(row_or_column, meth, aggregate)
-        values = {}
-
-        #calculate values for all cells that do not span accross multiple cells
-        #this ensures that we don't have a problem if the first line includes
-        #a cell that spans across multiple cells
-        each do |cell|
-          #don't take spanned cells
-          if cell.colspan == 1 and cell.class != Prawn::Table::Cell::SpanDummy
-            index = cell.send(row_or_column)
-            values[index] = [values[index], cell.send(meth)].compact.send(aggregate)
-          end
-        end
-
-        each do |cell|
-          index = cell.send(row_or_column)          
-          if cell.colspan > 1
-            #calculate current (old) return value before we do anything
-            old_sum = 0
-            cell.colspan.times { |i|
-              old_sum += values[index+i] unless values[index+i].nil?
-            }
-
-            #calculate future return value 
-            new_sum = cell.send(meth) * cell.colspan
-
-            if new_sum >= old_sum
-              #not entirely sure why we need this line, but with it the tests pass
-              values[index] = [values[index], cell.send(meth)].compact.send(aggregate) 
-              #overwrite the old values with the new ones, but only if all entries existed
-              entries_exist = true
-              cell.colspan.times { |i| entries_exist = false if values[index+i].nil? }
-              cell.colspan.times { |i|
-                values[index+i] = cell.send(meth) if entries_exist
-              }
-            end
-          else
-            if cell.class == Prawn::Table::Cell::SpanDummy
-              values[index] = [values[index], cell.send(meth)].compact.send(aggregate) 
-            end
-          end
-        end
-        values.values.inject(0, &:+)
+        ColumnWidthCalculator.new(self).aggregate_cell_values(row_or_column, meth, aggregate)
       end
 
       # Transforms +spec+, a column / row specification, into an object that

lib/prawn/table/column_width_calculator.rb

@@ -0,0 +1,160 @@
+module Prawn
+  class Table
+    class ColumnWidthCalculator
+      def initialize(cells)
+        @cells = cells
+
+        @widths_by_column        = Hash.new(0)
+        @rows_with_a_span_dummy  = Hash.new(false)
+
+        #calculate for each row if it includes a Cell:SpanDummy
+        @cells.each do |cell|
+          @rows_with_a_span_dummy[cell.row] = true if cell.is_a?(Cell::SpanDummy)
+        end
+      end
+
+      # does this row include a Cell:SpanDummy?
+      #
+      # @param row - the row that should be checked for Cell:SpanDummy elements
+      #
+      def has_a_span_dummy?(row)
+        @rows_with_a_span_dummy[row]
+      end
+
+      def fill_values_if_needed(values, cell, index, meth)
+        #have all spanned indices been filled with a value?
+        #e.g. values[0], values[1] and values[2] don't return nil given a index of 0 and a colspan of 3
+        number_of_nil_values = 0
+        cell.colspan.times do |i|
+          number_of_nil_values += 1 if values[index+i].nil?
+        end
+
+        #nothing to do? because
+        #a) all values are filled
+        return values if number_of_nil_values == 0
+        #b) no values are filled
+        return values if number_of_nil_values == cell.colspan
+        #c) I am not sure why this line is needed FIXXME
+        #some test cases manage to this line even though there is no dummy cell in the row
+        #I'm not sure if this is a sign for a further underlying bug.
+        return values unless has_a_span_dummy?(cell.row)
+        #fill up the values array
+
+        #calculate the new sum
+        new_sum = cell.send(meth) * cell.colspan
+        #substract any calculated values
+        cell.colspan.times do |i|
+          new_sum -= values[index+i] unless values[index+i].nil?
+        end
+
+        #calculate value for the remaining - not yet filled - cells.
+        new_value = new_sum.to_f / number_of_nil_values
+        #fill the not yet filled cells
+        cell.colspan.times do |i|
+          values[index+i] = new_value if values[index+i].nil?
+        end
+        return values
+      end
+
+      def natural_widths
+        #calculate natural column width for all rows that do not include a span dummy
+        @cells.each do |cell|
+          unless has_a_span_dummy?(cell.row)
+            @widths_by_column[cell.column] =
+              [@widths_by_column[cell.column], cell.width.to_f].max
+          end
+        end
+
+        #integrate natural column widths for all rows that do include a span dummy
+        @cells.each do |cell|
+          next unless has_a_span_dummy?(cell.row)
+          #the width of a SpanDummy cell will be calculated by the "mother" cell
+          next if cell.is_a?(Cell::SpanDummy)
+
+          if cell.colspan == 1
+            @widths_by_column[cell.column] =
+              [@widths_by_column[cell.column], cell.width.to_f].max
+          else
+            #calculate the current with of all cells that will be spanned by the current cell
+            current_width_of_spanned_cells =
+              @widths_by_column.to_a[cell.column..(cell.column + cell.colspan - 1)]
+                               .collect{|key, value| value}.inject(0, :+)
+
+            #update the Hash only if the new with is at least equal to the old one
+            #due to arithmetic errors we need to ignore a small difference in the new and the old sum
+            #the same had to be done in the column_widht_calculator#natural_width
+            update_hash = ((cell.width.to_f - current_width_of_spanned_cells) >
+                           Prawn::FLOAT_PRECISION)
+
+            if update_hash
+              # Split the width of colspanned cells evenly by columns
+              width_per_column = cell.width.to_f / cell.colspan
+              # Update the Hash
+              cell.colspan.times do |i|
+                @widths_by_column[cell.column + i] = width_per_column
+              end
+            end
+          end
+        end
+
+        @widths_by_column.sort_by { |col, _| col }.map { |_, w| w }
+      end
+      def aggregate_cell_values(row_or_column, meth, aggregate)
+        values = {}
+
+        #calculate values for all cells that do not span accross multiple cells
+        #this ensures that we don't have a problem if the first line includes
+        #a cell that spans across multiple cells
+        @cells.each do |cell|
+          #don't take spanned cells
+          if cell.colspan == 1 and cell.class != Prawn::Table::Cell::SpanDummy
+            index = cell.send(row_or_column)
+            values[index] = [values[index], cell.send(meth)].compact.send(aggregate)
+          end
+        end
+
+        # if there are only colspanned or rowspanned cells in a table
+        spanned_width_needs_fixing = true
+
+        @cells.each do |cell|
+          index = cell.send(row_or_column)
+          if cell.colspan > 1
+            #special treatment if some but not all spanned indices in the values array have been calculated
+            #only applies to rows
+            values = fill_values_if_needed(values, cell, index, meth) if row_or_column == :column
+            #calculate current (old) return value before we do anything
+            old_sum = 0
+            cell.colspan.times { |i|
+              old_sum += values[index+i] unless values[index+i].nil?
+            }
+
+            #calculate future return value
+            new_sum = cell.send(meth) * cell.colspan
+
+            #due to float rounding errors we need to ignore a small difference in the new
+            #and the old sum the same had to be done in
+            #the column_width_calculator#natural_width
+            spanned_width_needs_fixing = ((new_sum - old_sum) > Prawn::FLOAT_PRECISION)
+
+            if spanned_width_needs_fixing
+              #not entirely sure why we need this line, but with it the tests pass
+              values[index] = [values[index], cell.send(meth)].compact.send(aggregate)
+              #overwrite the old values with the new ones, but only if all entries existed
+              entries_exist = true
+              cell.colspan.times { |i| entries_exist = false if values[index+i].nil? }
+              cell.colspan.times { |i|
+                values[index+i] = cell.send(meth) if entries_exist
+              }
+            end
+          else
+            if spanned_width_needs_fixing && cell.class == Prawn::Table::Cell::SpanDummy
+              values[index] = [values[index], cell.send(meth)].compact.send(aggregate)
+            end
+          end
+        end
+
+        return values.values.inject(0, &:+)
+      end
+    end
+  end
+end

spec/table_spec.rb

@@ -80,6 +80,34 @@
 
     end
 
+    it "illustrates issue #710", :issue => 710 do
+      partial_width = 40
+      pdf = Prawn::Document.new({page_size: "LETTER", page_layout: :portrait})
+      col_widths = [
+        50,
+        partial_width, partial_width, partial_width, partial_width
+      ]
+
+      day_header = [{
+          content: "Monday, August 5th, A.S. XLIX",
+          colspan: 5,
+      }]
+
+      times = [{
+        content: "Loc",
+        colspan: 1,
+      }, {
+        content: "8:00",
+        colspan: 4,
+      }]
+
+      data = [ day_header ] + [ times ]
+
+      #raised a Prawn::Errors::CannotFit:
+      #Table's width was set larger than its contents' maximum width (max width 210, requested 218.0)
+      table = Prawn::Table.new data, pdf, :column_widths => col_widths
+    end
+
     it "illustrate issue #533" do
       data = [['', '', '', '', '',''],
               ['',{:content => '', :colspan => 5}]]
@@ -91,15 +119,36 @@
       pdf = Prawn::Document.new
       first = {:content=>"Foooo fo foooooo",:width=>50,:align=>:center}
       second = {:content=>"Foooo",:colspan=>2,:width=>70,:align=>:center}
-      third = {:content=>"fooooooooooo, fooooooooooooo, fooo, foooooo fooooo",:width=>55,:align=>:center}
-      fourth = {:content=>"Bar",:width=>15,:align=>:center}
+      third = {:content=>"fooooooooooo, fooooooooooooo, fooo, foooooo fooooo",:width=>50,:align=>:center}
+      fourth = {:content=>"Bar",:width=>20,:align=>:center}
       table_content = [[
       first,
       [[second],[third,fourth]]
       ]]
       pdf.move_down(20)
+      table = Prawn::Table.new table_content, pdf
       pdf.table(table_content)
     end
+
+    #https://github.com/prawnpdf/prawn/issues/407#issuecomment-28556698
+    it "correctly computes column widths with empty cells + colspan" do
+      data = [['', ''],
+              [{:content => '', :colspan => 2}]
+              ]
+      pdf = Prawn::Document.new
+
+      table = Prawn::Table.new data, pdf, :column_widths => [50, 200]
+      table.column_widths.should == [50.0, 200.0]
+    end
+
+    it "illustrates a variant of problem in issue #407 - comment 28556698" do
+      pdf = Prawn::Document.new
+      table_data = [["a", "b", "c"], [{:content=>"d", :colspan=>3}]]
+      column_widths = [50, 60, 400]
+
+      # Before we fixed #407, this line incorrectly raise a CannotFit error
+      pdf.table(table_data, :column_widths => column_widths)
+    end
   end
 
   describe "#initialize" do