py-pdf/fpdf2

Images dont respect margins?

Lucas-C opened this issue ยท 9 comments

Discussed in #1155

Originally posted by joren-dev May 4, 2024
So I am using the self.image on a page, and while doing so I ran into the following problem
image

It clearly does not respect the bottom margin that I've said like so in the ctor:

margin_footer: int = 35
self.set_auto_page_break(True, margin_footer)

now the images are rendered as follows:

def print_images(self, images, fixed_width, max_per_row=0, vertical_spacing=5):
    
    image_info = None
    images_in_row = 0
    
    # Iterate through images
    for image_path in images:
        # If maximum images per row is set and reached, move to the next row
        if max_per_row != 0 and images_in_row == max_per_row:
            self.set_x(self.l_margin)  # Reset x position
            self.set_y(self.get_y() + image_info.rendered_height + vertical_spacing)  # Move to next row
            images_in_row = 0  # Reset images in row counter

        # If adding this image would exceed the page width, move to the next row
        if self.get_x() + fixed_width > self.epw:
            self.set_x(self.l_margin)  # Reset x position
            self.set_y(self.get_y() + image_info.rendered_height + vertical_spacing)  # Move to next row
            images_in_row = 0  # Reset images in row counter

        # Display image
        image_info = self.image(image_path, x=self.get_x(), y=self.get_y(), w=fixed_width)

        # Update current_x and images_in_row for next iteration
        self.set_x(self.get_x() + fixed_width + 10)  # Add 10 units spacing between images
        images_in_row += 1

    # Set y to below images, reset x to left margin
    self.set_xy(
        self.l_margin,
        self.get_y() + (image_info.rendered_height if image_info else 0) + 5,
    )

Now for both the width and the height I make sure that itll fit within the margins, is this intended? I would think that for the bottom margin it would auto break like with self.cell, self.tables, etc.

I solved the issue myself by adding:

# TODO: Figure out why images dont respect bottom margins
computed_image_height = calculate_image_height(image_path, fixed_width)

# Check if the image exceeds the page height
if self.get_y() + computed_image_height > self.page_break_trigger:
    self.add_page()  # Trigger page break
    self.set_xy(self.l_margin, self.t_margin)  # Reset x and y position

Prior to calling image_info = self.image(image_path, x=current_x, y=current_y, w=fixed_width) which solved the issue.

image

However having to do something alike for each image would be pretty inefficient and I have a feeling I am doing something wrong which causes my behavior, could someone help me solve this mystery?


I've tried to make a minimal working example:

from fpdf import FPDF

class PDF(FPDF):
    def __init__(self, orientation="P", unit="mm", format="A4"):
        super().__init__(orientation=orientation, unit=unit, format=format)
        self.set_auto_page_break(auto=True, margin=15)
        

# Create a new PDF object
pdf = PDF()

# Set up the document
pdf.set_title('My PDF Document')
pdf.set_author('John Doe')

# Add a chapter
pdf.add_page()
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)
pdf.ln(2)
pdf.image('settings/meter_reading_example.png', w=100)


# Output the PDF
pdf.output('test.pdf')

But it seems to respect the margins just fine:
image

So I am doing something wrong, just not sure what.

Hi, I successfully created a minimal reproduction of the error

class PDF(FPDF):
    def __init__(self, orientation="P", unit="mm", format="A4"):
        super().__init__(orientation=orientation, unit=unit, format=format)
        self.set_auto_page_break(auto=True, margin=15)
        
    def print_images(self, images, fixed_width):
        image_info = None
        
        # Iterate through images
        for image_path in images: 
            # Display image
            image_info = self.image(image_path, x=self.get_x(), y=self.get_y(), w=fixed_width)
            
            # Update y position to below the image + margin
            self.set_y(self.get_y() + image_info.rendered_height + 5)

# Create a new PDF object
pdf = PDF()

# Set up the document
pdf.set_title('My PDF Document')
pdf.set_author('John Doe')
pdf.add_page()

image_path = 'settings/meter_reading_example.png'
images_list = [image_path] * 30

pdf.print_images(images_list, fixed_width=100)

# Output the PDF
pdf.output('test.pdf')

image

Seems like I've pinpointed the issue a bit further down:

    def print_images(self, images, fixed_width):
        image_info = None

        # Iterate through images
        for image_path in images:
            # Display image
            self.set_x(self.l_margin)
            self.image(
                image_path, w=fixed_width
            )
            self.ln(5)

Works just fine
image

Seems like providing custom x and y to self.image creates the issue: x=self.get_x(), y=self.get_y()

in _raster_image on line: 4023

        # Flowing mode
        if y is None:
            self._perform_page_break_if_need_be(h)
            y = self.y
            self.y += h
        if x is None:
            x = self.x

It seems like when y is passed, it will not check for _perform_page_break_if_need_be and that its intended that way. However in my case I have to pass y otherwise I cannot have my images in rows horizontally.

for image_path in images:
        if (max_per_row != 0 and images_in_row == max_per_row) or (
            self.get_x() + fixed_width > self.epw
        ):
            self.set_x(self.l_margin)  # Reset x position
            self.ln(vertical_spacing)
            images_in_row = 0  # Reset images in row counter

        # Display image
        image_info = self.image(
            image_path, w=fixed_width
        )

        # Update current_x and images_in_row for next iteration
        self.set_x(
            self.get_x() + fixed_width + 10
        )  # Add 10 units spacing between images
        images_in_row += 1

Respects the margin, but without specifying the y= in the image call I cannot get them horizontally aligned.

image

But if I were to pass the y= and get them horizontally aligned, it will stop respecting the border. I would say that it should always check if it should auto_break, or have another member passed to image like: force_auto_break or smth alike to make it possible to pass a custom y and still auto_break if the image is not possible within a margin.

Well done in finding the root cause of your problem!

But if I were to pass the y= and get them horizontally aligned, it will stop respecting the border. I would say that it should always check if it should auto_break, or have another member passed to image like: force_auto_break or smth alike to make it possible to pass a custom y and still auto_break if the image is not possible within a margin.

I see...
Have you tried setting pdf.y BEFORE calling pdf.image(), without passing y= to .image()?

from fpdf import FPDF

pdf = FPDF()
pdf.add_page()
pdf.set_y(pdf.h - 50)
pdf.image('test/image/image_types/insert_images_insert_jpg.jpg')
pdf.output('issue_1156.pdf')

Well done in finding the root cause of your problem!

But if I were to pass the y= and get them horizontally aligned, it will stop respecting the border. I would say that it should always check if it should auto_break, or have another member passed to image like: force_auto_break or smth alike to make it possible to pass a custom y and still auto_break if the image is not possible within a margin.

I see... Have you tried setting pdf.y BEFORE calling pdf.image(), without passing y= to .image()?

from fpdf import FPDF

pdf = FPDF()
pdf.add_page()
pdf.set_y(pdf.h - 50)
pdf.image('test/image/image_types/insert_images_insert_jpg.jpg')
pdf.output('issue_1156.pdf')

So this is what I had before, with my print_images(), which gave:
image

Now I did what you suggested, with some small tweaks to work with my "max_per_row" system, here
The result is:
image

I did some debugging, now look at this:
image

This is the first iteration, the y is at the top margin as it should, seems fine and itll do line 27 so its setting y to 10.

image

Then second iteration, the y is 47.5, which is basically 10 + image height, okay so it moved the y down with the rendered height of my image, seems fine. However I correct this by doing the: self.set_y(self.get_y() - image_info.rendered_height). This line yields: 10 again, okay so far so good, except the image is printed on top of the other now.

The set_x() that adjusts the x value has not been changed, and if I were to pass x and y as done in here, it does put the image right next to it just fine.

Which seems to be caused by the following line: self.set_y(self.get_y() - image_info.rendered_height)

# self.get_x() gives correct value: 70.0
if images_in_row > 0:
    self.set_y(self.get_y() - image_info.rendered_height)  # Adjust y-coordinate
    # self.get_x() gives incorrect value after this call above: back to 10.0
else:
    self.set_y(self.get_y())  # Set y-coordinate as it is for the first image

image

Apparently setting y resets the x to l_margin, somehow I didnt know that.

So preserving the x prior to set_y solves the issue:
image

This is the final working code: https://pastebin.com/mHfaBD36

So in short:

  • x resets to l_margin when adjusting y, that seems, incorrect but might be intended that way.
  • When passing x and y to image the bottom margins are not respected and _perform_page_break_if_need_be (auto page break) will not trigger
  • passing x to image has no effect to top/bottom margins, it wont respect r_margin but that seems logical

In order to have custom x/y one should set y and x prior to image call

correct_x = self.get_x()
if images_in_row > 0:
    self.set_xy(correct_x, self.get_y() - image_info.rendered_height)

correct_x ensures the x isnt reset to l_margin after adjusting y

I am fairly sure we would image to always respect the bottom margin, despite passing a custom y, so that should probably be fixed.

Other options that you have:

  • use FPDF.set_xy() to set both X & Y positions at the same time
  • directly set pdf.x = and pdf.y =

x resets to l_margin when adjusting y, that seems, incorrect but might be intended that way.

I agree that it can be unexpected, but this is the behaviour since fpdf2 exists I think.
That doesn't mean that this is good design, just that it would be delicate to change without breaking existing user code that uses this method.

Which seems to be caused by the following line: self.set_y(self.get_y() - image_info.rendered_height)

Thank you for the detailed analysis, but most of the debugging analysis you shared is about your own code (the print_image() method), and that does not really has much to do with the fpdf2 library itself... ๐Ÿ˜…

I am fairly sure we would image to always respect the bottom margin, despite passing a custom y, so that should probably be fixed.

I am not so sure.
Consider for example image insertion performed inside FPDF.footer(): in that case we may want to avoid page breaks in order to insert images in margins.

However, I agree that this behaviour should be better documented.
Maybe the docstring of FPDF.image() could include a sentence like:
"If y is provided, this method will not trigger any page break; otherwise, auto page break detection will be performed"

What do you think about this @joren-dev?
Would you like to contribute a PR regarding this?
Else I can handle this myself, no problem.

I made the minor docstring improvement in PR #1179

Unless you have other comments or questions @joren-dev, we will probably close this issue.

I am fairly sure we would image to always respect the bottom margin, despite passing a custom y, so that should probably be fixed.

I don't think this is quite so clear cut.

fpdf2 is a general purpose PDF generator. There is some vaguely defined focus on flowing text across pages, and you can include images in that flow. But when you explicitly specify a vertical position for an image, then it doesn't really make sense to have it continue to move with the text flow. In particular, would you expect the image to actually be placed at that height on the next page, or or is it supposed to silently ignore the explicit placement in that case? There's a perfectly valid use case of placing an image outside of the text margins, and you don't want to have to globally disable auto-page-break every time you do so.

If you need your images to always flow with the text no matter what, then I recommend to take a look at text_columns(), which will unconditionally garantee that. Note that in this case, the image will not accept explicit x or y values. But you can align it horizontally within the column, and give it a top and/or bottom margin, which should have the same effect in most cases.

Thank you @gmischler for this extra answer & suggestion ๐Ÿ™‚

Closing this now.