Skip to content

typst-pdf: Attach known image types without compression#7876

Closed
krkk wants to merge 1 commit intotypst:mainfrom
krkk:attach-image-without-compression
Closed

typst-pdf: Attach known image types without compression#7876
krkk wants to merge 1 commit intotypst:mainfrom
krkk:attach-image-without-compression

Conversation

@krkk
Copy link
Copy Markdown

@krkk krkk commented Feb 16, 2026

Most of the image files already have the data compressed, so sprinkling deflate on top of it won't do much besides making it take longer to compile the document.

Initially, #6256 had it right, but then it got flipped during refactoring. I believe it wasn't intentional, hence my PR. :^)
(interesting that clippy doesn't catch equal match cases)

Most of the image files already have the data compressed, so sprinkling deflate on top of it won't do much besides making it take longer to compile the document.
@krkk
Copy link
Copy Markdown
Author

krkk commented Feb 16, 2026

waaaaait this is only for #pdf.attach(), #image() has its own code path. huhhhh, so i guess its almost useless now.
oh well ¯\(ツ)

@laurmaedje laurmaedje requested a review from saecki February 16, 2026 13:26
@laurmaedje laurmaedje added the waiting-on-review This PR is waiting to be reviewed. label Feb 16, 2026
@saecki
Copy link
Copy Markdown
Member

saecki commented Feb 16, 2026

Most of the image files already have the data compressed, so sprinkling deflate on top of it won't do much besides making it take longer to compile the document.

Initially, #6256 had it right, but then it got flipped during refactoring. I believe it wasn't intentional, hence my PR. :^) (interesting that clippy doesn't catch equal match cases)

Hmm, have you noticed or measured the performance impact of the deflate compression? Because in my testing I've had some cases where images weren't very efficiently encoded and this made quite the difference in size. Note that None in this case means: use automatic compression, and if the threshold isn't met the image is written as is.

@laurmaedje
Copy link
Copy Markdown
Member

Note that None in this case means: use automatic compression, and if the threshold isn't met the image is written as is.

That might be worth a comment, because I also thought it's a typo 😅

@laurmaedje laurmaedje added waiting-on-author Pull request waits on author and removed waiting-on-review This PR is waiting to be reviewed. labels Feb 16, 2026
@saecki
Copy link
Copy Markdown
Member

saecki commented Feb 16, 2026

Yeah, maybe we can also return a Smart<bool> from should_compress 🤔

@Enivex
Copy link
Copy Markdown
Collaborator

Enivex commented Feb 16, 2026

waaaaait this is only for #pdf.attach(), #image() has its own code path. huhhhh, so i guess its almost useless now.
oh well ¯\(ツ)

Relevant #7668

@laurmaedje
Copy link
Copy Markdown
Member

Since this is intentional, let's close this. We could switch to Smart in a separate PR.

Thanks for the PR anyway!

@laurmaedje laurmaedje closed this Feb 18, 2026
@laurmaedje laurmaedje removed the waiting-on-author Pull request waits on author label Feb 18, 2026
@krkk krkk deleted the attach-image-without-compression branch February 18, 2026 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants