-
Notifications
You must be signed in to change notification settings - Fork 462
🐛(pdf) preserve image aspect ratio in PDF export #1622
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
Conversation
187657b to
40a5394
Compare
|
Size Change: +58 B (0%) Total Size: 4.07 MB
|
AntoLC
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitively a great improvement !
|
|
||
| const PIXELS_PER_POINT = 0.75; | ||
| const FONT_SIZE = 16; | ||
| const MAX_WIDTH = 600; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not duplicate with FALLBACK_SIZE ?
docs/src/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/imagePDF.tsx
Line 24 in 40a5394
| const FALLBACK_SIZE = 536; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not duplicate with
FALLBACK_SIZE?docs/src/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/imagePDF.tsx
Line 24 in 40a5394
const FALLBACK_SIZE = 536;
They are up to different things:
MAX_WIDTH = 600: Maximum width in PDF to prevent images from overflowing the page
| pngConverted = await convertSvgToPng(svgText, previewWidth); | ||
|
|
||
| // Get actual dimensions from the converted PNG | ||
| if (typeof window !== 'undefined') { | ||
| const img = document.createElement('img'); | ||
| img.src = pngConverted; | ||
| await new Promise((resolve) => { | ||
| img.onload = () => { | ||
| dimensions = { width: img.width, height: img.height }; | ||
| resolve(null); | ||
| }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see we have some informations in convertSvgToPng, could we do something like that maybe ?
const {
png,
width: pngWidth,
height: pngHeight,
} = await convertSvgToPng(svgText, previewWidth);
pngConverted = png;
dimensions = { width: pngWidth, height: pngHeight };There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yes and I have to update imageODT and imageDOCS too !
| async function getImageDimensions(blob: Blob) { | ||
| if (typeof window !== 'undefined') { | ||
| const bmp = await createImageBitmap(blob); | ||
| const { width, height } = bmp; | ||
| bmp.close(); | ||
| return { width, height }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit concerned about potential performance implications when lot of images and heavy images, is there maybe a less greedy way to get the size of an images?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, after research wdyt by switched to document.createElement('img') which should be lighter than createImageBitmap(). This matches the approach already used in imageDocx.tsx
I put it in a new fixup
315a885 to
1c38592
Compare
1c38592 to
ce469ac
Compare
images were distorted in PDF exports; height is now computed to fix that Signed-off-by: Cyril <c.gromoff@gmail.com>
ce469ac to
2e64298
Compare
Purpose
Ensure images maintain their original aspect ratio in PDF exports, aligning behavior with DOCX/ODT exports
Proposal