Bounds don't get reset after rendering multi-page, centered table within indent #722

Closed
TylerRick opened this Issue May 12, 2014 · 7 comments

Comments

Projects
None yet
3 participants

You can observe the problem with this example (gist):

Prawn::Document.generate("#{__FILE__}.pdf") do |pdf|
  pdf.start_new_page margin: [100, 100, 100, 100]
  pdf.stroke_axis
  orig_bounds_width = pdf.bounds.width
  pdf.formatted_text_box [ { :text => "width: #{pdf.bounds.width}" } ], :at => [0, 20]

  pdf.indent(100) do
    pdf.stroke_axis
    pdf.formatted_text_box [ { :text => "width: #{pdf.bounds.width}" } ], :at => [0, 20]

    # Problem only occurs when table too big to fit on one page, and position: :center is used
    pdf.table([ ['Test'] ]*30, {position: :center})
  end

  pdf.start_new_page margin: [100, 100, 100, 100]
  pdf.stroke_axis
  # Observe: Width is 312.0, but should have been reset to 412.0
  pdf.formatted_text_box [ { :text => "width: #{pdf.bounds.width} (should still be #{orig_bounds_width})" } ], :at => [0, 20]
end

Basically, the indent from the indent {} block doesn't seem to get reset after returning from the block. This only seems to be a problem for centered tables. Any ideas?

Tested against latest from master branch (4d1afa9).

Owner

practicingruby commented May 12, 2014

@hbrandl: Please investigate

hbrandl self-assigned this May 13, 2014

Contributor

hbrandl commented May 13, 2014

Assigned to myself. I'll look into this issue.
I'll be a lot faster if anyone can provide a failing test case for it.

Contributor

hbrandl commented May 14, 2014

I can confirm the issue and that it is related to multi page tables.
Here is a test case for this issue

  describe "indent blocks should be reset on a new page" do
    it "should reset indent blocks even when a centered table spans accross multiple pages", :unresolved, :issue => 722 do
      pdf = Prawn::Document.new
      pdf.start_new_page margin: [100, 100, 100, 100]
      pdf.stroke_axis
      orig_bounds_width = pdf.bounds.width
      pdf.formatted_text_box [ { :text => "width: #{pdf.bounds.width}" } ], :at => [0, 20]

      pdf.indent(100) do
        pdf.stroke_axis
        pdf.formatted_text_box [ { :text => "width: #{pdf.bounds.width}" } ], :at => [0, 20]

        # Problem only occurs when table too big to fit on one page, and position: :center is used
        pdf.table([ ['Test'] ]*30, {position: :center})
      end

      pdf.start_new_page margin: [100, 100, 100, 100]
      pdf.stroke_axis
      # Observe: Width is 312.0, but should have been reset to 412.0
      pdf.bounds.width.should == orig_bounds_width
    end
  end

I'll keep you updated.

Contributor

hbrandl commented May 14, 2014

I've further simplified the testcase

  describe "indent blocks should be reset on a new page" do
    it "should reset indent blocks even when a centered table spans accross multiple pages", :unresolved, :issue => 722 do
      pdf = Prawn::Document.new
      pdf.start_new_page margin: [100, 100, 100, 100]
      orig_bounds_width = pdf.bounds.width

      pdf.indent(100) do
        #bounds should be reduced by a 100
        pdf.bounds.width.should == orig_bounds_width - 100

        # Problem only occurs when table too big to fit on one page, and position: :center is used
        pdf.table([ ['Test'] ]*30, {position: :center})
      end

      pdf.start_new_page margin: [100, 100, 100, 100]

      # Observe: Width is 312.0, but should have been reset to 412.0
      pdf.bounds.width.should == orig_bounds_width
    end
  end
Contributor

hbrandl commented May 15, 2014

I'm unsure if the bug is in the table code or somewhere else. I couldn't figure out why a centered or left table lead to different results. @sandal or someone else, could you check out this bug or give me some pointers?

For some reason that I couldn't figure out yet, the following code behaves differently when the table in the test case is passed a position of :center or :left.

generate_margin_box (document.rb)

# This check maintains indentation settings across page breaks
      if old_margin_box
        @margin_box.add_left_padding(old_margin_box.total_left_padding)
        @margin_box.add_right_padding(old_margin_box.total_right_padding)
      end

For some reason this code thinks that the indentation block is not finished yet when the table was centered, but behaves as expected when the table was positioned to the left.

Contributor

hbrandl commented Jun 24, 2014

I've looked into this issue again and resolved it a little further.

After investigation further my current guess is that the problematic code is located here:

    # Sets up a bounding box to position the table according to the specified
    # :position option, and yields.
    #
    def with_position
      x = case defined?(@position) && @position || :left
          when :left   then return yield
          when :center then (@pdf.bounds.width - width) / 2.0
          when :right  then  @pdf.bounds.width - width
          when Numeric then  @position
          else raise ArgumentError, "unknown position #{@position.inspect}"
          end
      dy = @pdf.bounds.absolute_top - @pdf.y
      final_y = nil

      @pdf.bounding_box([x, @pdf.bounds.top], :width => width) do
        @pdf.move_down dy
        yield
        final_y = @pdf.y
      end

      @pdf.y = final_y
    end

This code isn't very readable. So I'm a little lost. (That's why a lot of the table code should be refactored once I find time for it.)

What I can provide you with is a workaround. Adding one line after the indent block solves the problem for me. Can you confirm this?

pdf.margin_box.subtract_left_padding(100)

The whole block

pdf.indent(100) d
  # Problem only occurs when table too big to fit on one page, and position: :center is used
  pdf.table([ ['Test'] ]*30, {position: :center})
end
pdf.margin_box.subtract_left_padding(100)
Owner

practicingruby commented Jul 18, 2014

I am closing all issues for Prawn::Table, because it is being extracted into its own gem with its own repository (https://github.com/prawnpdf/prawn-table).

As I close tickets, I'm marking them with a "table" tag, so that @hbrandl can easily find them and optionally move them over to the prawn-table issue tracker. Anything that has the table tag can be assumed to have been unresolved at the time I closed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment