python-pptx icon indicating copy to clipboard operation
python-pptx copied to clipboard

Generating corrupted PPT when using connectors

Open srivkrnt opened this issue 9 months ago • 3 comments

I have observed an issue recently with the library where the generated PPT doesn't open in Google Slides and Microsoft Powerpoint

You can have a look at the below example

for i in range(len(positions) - 1):
    keys = list(positions.keys())
    start_shape = shapes[keys[i]]
    end_shape = shapes[keys[i + 1]]
    
    # Calculate the absolute positions for start and end points
    start_left = start_shape.left + start_shape.width
    start_top = start_shape.top + start_shape.height / 2
    end_left = end_shape.left
    end_top = end_shape.top + end_shape.height / 2

    connector = slide.shapes.add_connector(
        MSO_CONNECTOR.CURVE,
        start_left,
        start_top,
        end_left,
        end_top
    )

I have observed that when the start_top and end_top are not integers because of the division. The PPT generated doesn't open in Google Slides and Microsoft PPT.

start_top = start_shape.top + int(start_shape.height / 2)
end_top = end_shape.top + int(end_shape.height / 2)

and this fixes the issue. I was thinking of moving this inside the library itself, so that even if the client sends a non-int value, it can be cast to a int value and the PPT generated will be valid.

Let me know if I can raise of PR for this @scanny

srivkrnt avatar Apr 29 '25 05:04 srivkrnt

I would vote against this going into python-pptx itself: I wouldn't want to throw away accuracy in calculations. (In my project - that leans heavily on python-pptx - I aim to do the conversion to integer after positioning the elements.)

MartinPacker avatar Apr 29 '25 05:04 MartinPacker

I think this is fair enough, I think replacing x with int(x) and the same for y, cx, and cy in these couple lines would do the trick: https://github.com/scanny/python-pptx/blob/master/src/pptx/oxml/shapes/connector.py#L49

@MartinPacker there won't be any loss of accuracy here because length values are integer EMU values, 914400 to the inch, so truncating a fractional value would change that value by at most 1 millionth of an inch in round numbers. Also there is no way to preserve a fractional EMU value so the only choice is "next bigger" or "next smaller".

@srivkrnt note the type of these arguments on .add_connector() are declared, so if you're ignoring type errors and passing in something else then you can expect odd behaviors.

Another approach that strikes me as a little bit interesting would be to define __truediv__() -> Self on Length such that the result of dividing a length is also Length (an integer value).

Anyway, I'll add this to the shortlist for the next time I'm in there. In the meantime I recommend you pay attention to type errors which would point this case up I expect :)

scanny avatar May 01 '25 18:05 scanny

Thanks @scanny makes sense.

srivkrnt avatar May 02 '25 09:05 srivkrnt