Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rendering goes crazy if non-integer Row height is specified or no columns in row. #414

Open
xaurx opened this issue Apr 26, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@xaurx
Copy link

xaurx commented Apr 26, 2024

Describe the bug
I generate a large multi row table by calling m.AddRow(4, text.NewCol(...)). It worked perfect until I added some empty lines as padding like this: m.AddRow(1)
and after that everything goes crazy - line backgrounds and content are shown separately at some distance from each other... Luckily, I found a workaround as m.AddRow(1, text.NewCol(12, ""))

Similar issue if I use fractional row height with lines like m.AddRows(line.NewRow(0.1, props.Line{Thickness: 0.1, SizePercent: 100})).

Screenshot 2024-04-25 at 23 02 16
@Fernando-hub527
Copy link
Collaborator

hello everything is fine?
Regarding the first point you mentioned, I tried to reproduce the error, but I couldn't, the code seems to be working as expected:

  func main() {
	  m := maroto.New()
  
	  m.AddRow(15, text.NewCol(4, "1")).WithStyle(&props.Cell{BackgroundColor: &props.GreenColor})
	  m.AddRow(15, text.NewCol(4, "2")).WithStyle(&props.Cell{BackgroundColor: &props.BlueColor})
	  m.AddRow(15, text.NewCol(4, "3")).WithStyle(&props.Cell{BackgroundColor: &props.GreenColor})
	  m.AddRow(15, text.NewCol(4, "4")).WithStyle(&props.Cell{BackgroundColor: &props.BlueColor})
	  m.AddRow(15, text.NewCol(4, "5")).WithStyle(&props.Cell{BackgroundColor: &props.GreenColor})
	  m.AddRow(15, text.NewCol(4, "6")).WithStyle(&props.Cell{BackgroundColor: &props.BlueColor})
	  m.AddRow(15, text.NewCol(4, "7")).WithStyle(&props.Cell{BackgroundColor: &props.GreenColor})
	  m.AddRow(15, text.NewCol(4, "8")).WithStyle(&props.Cell{BackgroundColor: &props.BlueColor})
	  m.AddRow(15).WithStyle(&props.Cell{BackgroundColor: &props.GreenColor})
	  m.AddRow(15).WithStyle(&props.Cell{BackgroundColor: &props.BlueColor})
	  m.AddRow(15).WithStyle(&props.Cell{BackgroundColor: &props.GreenColor})
  
	  document, err := m.Generate()
	  if err != nil {
		  log.Fatal(err.Error())
	  }
  
	  err = document.Save("./pdf.pdf")
	  if err != nil {
		  log.Fatal(err.Error())
	  }
  }

This was the result, is it what you need ?
Captura de tela de 2024-04-27 22-36-23

Can you post an example code? This way I can check what I'm doing differently and try to help.

@xaurx
Copy link
Author

xaurx commented Apr 29, 2024

sure. lets start with the simple one:

func main() {
        cfg := config.NewBuilder().
                WithPageSize(pagesize.Letter).
                WithOrientation(orientation.Horizontal).
                WithMargins(15, 15, 15)

        m := maroto.New(cfg.Build())

        textProps := props.Text{
                Size: 8,
                Align: align.Left,
                Left:  2,
                Right: 2,
        }

        for i := 0; i < 1000; i++ {
                m.AddRow(4)
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xE0, Blue: 0xE0, Green: 0xE0}})
        }

        document, err := m.Generate()
        if err != nil {
                log.Fatal(err.Error())
        }

        err = document.Save("./pdf.pdf")
        if err != nil {
                log.Fatal(err.Error())
        }
}

one would expect an empty line, than line with a text "row N".
page 1 and 2 look ok, page 3 slightly incorrect and page 4 is a disaster.

Screenshot 2024-04-28 at 21 13 56

if main loops looks like:

        for i := 0; i < 1000; i++ {
                m.AddRow(4, text.NewCol(12, ""))
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xE0, Blue: 0xE0, Green: 0xE0}})
        }

than PDF is rendered correctly.

@xaurx
Copy link
Author

xaurx commented Apr 29, 2024

another scenario where I want to separate groups of rows by line:

        for i := 0; i < 1000; i++ {
                m.AddRows(line.NewRow(0.4, props.Line{Thickness: 0.4, SizePercent: 100}))
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xE0, Blue: 0xE0, Green: 0xE0}})
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("2nd row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xC0, Blue: 0xC0, Green: 0xC0}})
        }

the issue becomes visible starting from page 4-5. e.g.:

  • "2nd row 85" is absent,
  • "row 107" is a the bottom of page 6, while should be at the top
  • and closer to the end of PDF at pages 46-47 you can see half of the page is missing!
Screenshot 2024-04-28 at 21 28 24

@Robur333
Copy link

I have been getting the same issue with the breaking layout when I was adding a lot of generated tables. I was trying to make the tables the way so that when they do not have enough space on a page they would appear on the second one. I didn't want them to break in the middle. Not sure if the issue is connected with yours but I think it was happening because something was going wrong with automatic page breaking. I think it has some issues when there is not enough space for a row to appear. I assumed it was making some weird stuff to the layout.

The solution I came up with was to check whether the row would have enough space on the page. If not it would fill in the page with an empty row with a height of space that was left on a page(float64). It helped but not really sure what was causing the error

@johnfercher
Copy link
Owner

johnfercher commented Apr 29, 2024

We only have to add an if to check of cols are empty, if is we should add an empty column to it.

maroto/maroto.go

Lines 103 to 107 in 4f5e446

func (m *Maroto) AddRow(rowHeight float64, cols ...core.Col) core.Row {
r := row.New(rowHeight).Add(cols...)
m.addRow(r)
return r
}

@johnfercher
Copy link
Owner

I think that the same should be done here:

maroto/maroto.go

Lines 94 to 96 in 4f5e446

func (m *Maroto) AddRows(rows ...core.Row) {
m.addRows(rows...)
}

@xaurx
Copy link
Author

xaurx commented Apr 29, 2024

We only have to add an if to check of cols are empty, if is we should add an empty column to it.

maroto/maroto.go

Lines 103 to 107 in 4f5e446

func (m *Maroto) AddRow(rowHeight float64, cols ...core.Col) core.Row {
r := row.New(rowHeight).Add(cols...)
m.addRow(r)
return r
}

can't I add columns AFTER m.AddRow() call? especially taking into account that it returns *Row one is tempted to do smth like m.AddRow().AddCol(...)

@johnfercher
Copy link
Owner

johnfercher commented Apr 29, 2024

Bro, the library is not to use like this...See the documentation there is none example using like this... There are a bunch of examples here...

@Fernando-hub527
Copy link
Collaborator

Fernando-hub527 commented Apr 30, 2024

We only have to add an if to check of cols are empty, if is we should add an empty column to it.

maroto/maroto.go

Lines 103 to 107 in 4f5e446

func (m *Maroto) AddRow(rowHeight float64, cols ...core.Col) core.Row {
r := row.New(rowHeight).Add(cols...)
m.addRow(r)
return r
}

It seems to be easy to implement, but I was thinking: as it is not possible to use a row without columns, wouldn't it be better to do this validation when the row is created: row.New(rowHeight).Add(cols...) , this way when the line is used in other places it will already be in the correct configuration. Does it make sense to do it this way?
To be honest, I'm not sure if it would be viable to do it this way because it seems to me that the columns would need to be passed as a parameter in row.New(), but maybe there's another way

@Fernando-hub527
Copy link
Collaborator

another scenario where I want to separate groups of rows by line:

        for i := 0; i < 1000; i++ {
                m.AddRows(line.NewRow(0.4, props.Line{Thickness: 0.4, SizePercent: 100}))
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xE0, Blue: 0xE0, Green: 0xE0}})
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("2nd row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xC0, Blue: 0xC0, Green: 0xC0}})
        }

the issue becomes visible starting from page 4-5. e.g.:

  • "2nd row 85" is absent,
  • "row 107" is a the bottom of page 6, while should be at the top
  • and closer to the end of PDF at pages 46-47 you can see half of the page is missing!
Screenshot 2024-04-28 at 21 28 24

Hello !
I did some tests and noticed that the error only occurs when the line size is less than 0.5, did you notice something similar?

@Fernando-hub527
Copy link
Collaborator

another scenario where I want to separate groups of rows by line:

        for i := 0; i < 1000; i++ {
                m.AddRows(line.NewRow(0.4, props.Line{Thickness: 0.4, SizePercent: 100}))
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xE0, Blue: 0xE0, Green: 0xE0}})
                m.AddRow(4, text.NewCol(12, fmt.Sprintf("2nd row %d", i), textProps)).WithStyle(&props.Cell{BackgroundColor: &props.Color{Red: 0xC0, Blue: 0xC0, Green: 0xC0}})
        }

the issue becomes visible starting from page 4-5. e.g.:

  • "2nd row 85" is absent,
  • "row 107" is a the bottom of page 6, while should be at the top
  • and closer to the end of PDF at pages 46-47 you can see half of the page is missing!
Screenshot 2024-04-28 at 21 28 24

I'm working on this problem, but it seems to be more complex than I thought. The error does not appear for all lines with fractional size, but only for some values ​​and these values ​​change according to the margins of the maroto, for example:

  • When setting the margins with the value WithMargins(15, 15, 15), lines with sizes 0.1, 0.3, 0.4, 0.6, 0.8 and 0.9 generate the error.
  • When setting the margins with the value WithMargins(15, 14, 15), lines with sizes 0.1, 0.3, 0.4, 0.7, 0.8 and 0.9 generate the error.

However, when setting the margins with the value WithMargins(15, 0, 15) the error does not occur.

Suggestions:

Captura de tela de 2024-05-08 23-16-09

Looking at the image you can see that the first page is configured correctly, but from the second page onwards the content of the lines moves upwards, in addition when line 231 of the maroto.go file: m.rows = append(m. rows, spaceRow) is commented out, the error does not appear to occur. This makes me believe that the problem is in this process of creating a new page.

I'll keep trying to solve it, but if anyone has any suggestions...

@johnfercher
Copy link
Owner

Fixed error when using empty columns in this release: https://github.com/johnfercher/maroto/releases/tag/v2.0.0-beta.19

@johnfercher johnfercher added the bug Something isn't working label May 16, 2024
@maikerlab
Copy link

In my project I just created simple rows with 2 columns.
Because maroto doesn't support adaptive row heights (yet) I was calculating the row height dynamically based on the length of the text. The final PDF that was created was really messed up, some lines were randomly placed onto the next page, text got overlapped etc.

I didn't read through all the comments here, but I just wanted to tell you about the problems I had, which seem to be related to this issue. I fixed the problem in my project by just rounding up to the nearest integer, so maybe my simple solution could also be implemented directly in maroto?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants