Padding interferes with YPos.TOP

[mjasperse]

Hi guys, another really minor issue I encountered in this very useful library related to YPos.TOP handling in multi_cell.

It seems to me that fpdf.py:3499 is duplicated at fpdf.py:3526 (lines on master) which leads to an accumulation of the padding amount if that feature is used.

Describe the bug

Error details Padding is accumulated in y-position despite using new_y=TOP

Minimal code Please include some minimal Python code reproducing your issue:

from fpdf import FPDF, YPos
pdf = FPDF()
pdf.add_page()
pdf.set_font('helvetica', '', 10)
pdf.set_fill_color(235)
for ch in 'ABCD':
    pdf.multi_cell(w=30, text=ch, align='CENTER', padding=5, fill=True, border=True, new_y=YPos.TOP)
pdf.output('test.pdf')

Expected behaviour is all cells would be in a single row. Commenting out the duplicated line recovers the expected behaviour. Happy to make a PR but looks like a one-liner to me.

[gmischler]

Thanks for reporting this bug, @mjasperse !

Although you consider it minor, it looks annoying. The code in multi_cell() relies on the internal method _render_styled_text_line() to handle the cursor position, but with a top margin, that obviously doesn't know about the correct height. You're very welcome to submit a PR fixing it. Even with a one-liner (or three), someone needs to do it... :wink:

[mjasperse]

I've been diving into testing this issue a bit further ahead of submitting a PR, and I noticed a number of related unexpected behaviours around padding. In particular, multiline cells will obscure each-other when fill=True, and Align.X moves them out of alignment within the group:

For comparison here is the same script with padding=0:

I've made some changes to the handling, and here's the result I achieved:

I'm now regression testing locally to make sure it hasn't broken anything else, since it could affect a number of use-cases. Hopefully I'll get a PR up this weekend!

[mjasperse]

Here´s the script for reproducing the above behaviour, which I'll convert to a unit test

def test_multi_cell_align_with_padding():
    pdf = FPDF()
    pdf.add_page()
    pdf.set_font("Helvetica", size=10)
    pdf.set_fill_color(222)

    # show alignment markers
    with pdf.local_context(draw_color=(0,255,0)):
        pdf.line(pdf.l_margin, 0, pdf.l_margin, pdf.h)
        pdf.line(pdf.w/2, 0, pdf.w/2, pdf.h)

    # create a set of multi_cells contaning multiple lines each
    def create_boxes(new_x, new_y, align, padding=2):
        w = (pdf.w - pdf.l_margin - pdf.r_margin)/6
        for ch in 'ABC':
            text = f'{ch}1\n{ch}{ch}2\n{ch}{ch}{ch}3'
            pdf.multi_cell(w=w, text=text, fill=True, border=True, padding=padding, new_x=new_x, new_y=new_y, align=align)

    create_boxes('LMARGIN', 'NEXT', 'JUSTIFY')
    create_boxes('RIGHT', 'TOP', 'CENTER')
    create_boxes('LEFT', 'NEXT', 'RIGHT')
    create_boxes('CENTER', 'NEXT', 'X')
    pdf.output('test.pdf')

[gmischler]

More interesting observations!

The reason is again in the interaction between multi_cell() and _render_styled_text_line(). Currently the drawing of background and borders is delegated to the latter, seperately for each line of text. This was fine, until padding was introduced.

Now it's probably time to have multi_cell() draw its own background and borders. Apart from fixing the issues you noticed, that will also be more efficient and result in smaller output files. Is that the approach you chose?

[mjasperse]

My approach was to adjust how the padding gets used within multi_cell() and _render_styled_text_line() to correct the observations, so my branch is more of a small adjustment to the existing method. The change passes the test suite, and I am adding some additional cases for verification.

I did consider refactoring the background/border drawing up into multi_cell(), which has the advantages you mentioned, as well as an additional improvement regarding rendering the result in a viewer. I noticed that in the existing implementation using fill=True causes some PDF viewers to render a small gap between the constituent line cells at some levels of zoom - presumably a rounding artifact in rendering. You can notice these small gaps in the screenshot I posted above, and drawing a single background rect will avoid this issue entirely.

There are some complications regarding page break handling, since multi_cell() would need to render multiple background rects in that scenario, but I would be happy to work on that further.

[Lucas-C]

@allcontributors please add @mjasperse for bug

Thank you for the report, and the PR to fix this!

[allcontributors[bot]]

@Lucas-C

I've put up a pull request to add @mjasperse! :tada:

I couldn't determine any contributions to add, did you specify any contributions? Please make sure to use valid contribution names.