jung-kurt / gofpdf

A PDF document generator with high level support for text, drawing and images
http://godoc.org/github.com/jung-kurt/gofpdf
MIT License
4.34k stars 787 forks source link

Wrong page split behavior with multiple MultiCells across same page break #238

Open snargleplax opened 5 years ago

snargleplax commented 5 years ago

Synopsis

The automatic page-breaking behavior for MultiCell behaves incorrectly when multiple calls to MultiCell cause their generated cells to span the same page break. This arises e.g. when trying to render a multi-column table with a row that spills over a page break. The first cell breaks as expected (in my repro, between pages 1 and 2), but the second appears to incorrectly trigger the "add a new page" logic again, and winds up splitting its contents across non-adjacent pages (in my repro, between pages 1 and 3).

Repro

package main

import (
    "io/ioutil"
    "log"

    "github.com/jung-kurt/gofpdf"
)

func main() {
    f := gofpdf.New("P", "in", "Letter", "")
    f.SetMargins(margin, margin, -margin)
    f.AddPage()

    f.SetFont("Arial", "", 12)
    _, h := f.GetFontSize()

    content := `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn. Hai n'gha chtenff, kadishtu y'hah Dagon gof'nn ph'ya uln ebunmanyth 'bthnkog nog, kadishtuog lw'nafh f'Shub-Niggurath sgn'wahl uln goka ah. Uaaah h'hai ch' uaaah h'gof'nn h'fhtagn kadishtu hlirgh ya, ep gotha nglui goka kadishtu goka Shub-Niggurath nashtunggli, cmnahn' Azathoth f'phlegeth ehye nagnaiih vulgtm naCthulhu. Li'hee gnaiih goka ahnyth ron geb shtunggli Chaugnar Faugn kn'a, h'Yoggoth grah'n shogg Yoggothnyth f'bug nglui h'mg, shugg s'uhn 'bthnk gnaiih y-'bthnk kn'a shagg. Kn'a f'mg naooboshu nglui k'yarnak shugg shagg kn'a ep Azathoth grah'n athg Azathoth, h'ya goka ngn'ghft ph'ehye hai lloig f'ya nnnfm'latgh ooboshu hrii y-ep.`

    p := f.PageNo()
    yMax := 10.0
    f.SetXY(margin, yMax-0.5)
    f.MultiCell(usableWidth/2, h, content, "1", "LM", false)

    f.SetPage(p)
    f.SetXY(margin+usableWidth/2, yMax-0.5)
    f.MultiCell(usableWidth/2, h, content, "1", "LM", false)

    tf, err := ioutil.TempFile("", "gofpdf-repro")
    if err != nil {
        log.Fatalln("open temp file:", err)
    }

    if err := f.Output(tf); err != nil {
        log.Fatalln("write file output:", err)
    }

    log.Println("wrote ", tf.Name())
}

const (
    margin          = 1
    letterWidth     = 8.5
    usableWidth     = letterWidth - (margin * 2)
)

Expected results

A two-page document, with each of the two cells split across the boundary between pages 1 and 2.

Actual results

A three-page document, with the left cell split across the boundary between pages 1 and 2, and the right cell split across the boundary between pages 1 and 3.

jung-kurt commented 5 years ago

Nice writeup, @snargleplax. Thanks for coming up with the code to reproduce the bug. I'll look into this.

lkoller commented 5 years ago

I am also seeing this same bug in a project I'm working on -- any good leads on a fix?

jung-kurt commented 5 years ago

I think the problem is here. When an automatic page break occurs, a new page is added unconditionally. One solution would be to check to see if the current page is not the last page, as in your example above. I will have to ponder the consequences of this a bit more.

jung-kurt commented 5 years ago

You can obtain your desired results by manually controlling page breaks. Change

p := f.PageNo()
yMax := 10.0
f.SetXY(margin, yMax-0.5)
f.MultiCell(usableWidth/2, h, content, "1", "LM", false)

f.SetPage(p)
f.SetXY(margin+usableWidth/2, yMax-0.5)
f.MultiCell(usableWidth/2, h, content, "1", "LM", false)

to the following

p := f.PageNo()
col := 0
yMax := 10.0
f.SetAcceptPageBreakFunc(func() bool {
    if col == 0 {
        return true
    }
    f.SetPage(f.PageNo() + 1)
    f.SetXY(margin+usableWidth/2, margin)
    return false
})

f.SetXY(margin, yMax-0.5)
f.MultiCell(usableWidth/2, h, content, "1", "LM", false)

col = 1
f.SetPage(p)
f.SetXY(margin+usableWidth/2, yMax-0.5)
f.MultiCell(usableWidth/2, h, content, "1", "LM", false)

This assumes that the content in the leftmost column will be the longest. Automatic page breaks occur normally for this column. For subsequent columns, automatic page breaks are suppressed and, instead, the page is set to the following page and the X and Y coordinates are initialized appropriately.

I will add a PageCount() method so that this scheme will be more robust. When the page count is known, you will allow a new page to be added only when the break occurs on what is currently the last page.

lkoller commented 5 years ago

Thanks for the response -- here's how I solved for my situation incase anyone else is looking for an idea :)

  pdf.SetAcceptPageBreakFunc(func() bool {                                           
    lastPage = lastPage + 1                                                          
    if lastPage != pdf.PageNo(){                                                     
      pdf.SetPage(lastPage)                                                          
      pdf.SetY(headerHeight + 3)                                                     
      return false                                                                   
    }                                                                                

    return true                                                                      
  })
jung-kurt commented 5 years ago

Nice solution, @lkoller! It would be nice to fully automate something like this, but positioning the cursor when returning to an existing page will always be application dependent, so this seems like the best way to solve this problem.