Skip to content
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

LibGfx: Add a .pam loader #22935

Merged
merged 5 commits into from
Jan 26, 2024
Merged

LibGfx: Add a .pam loader #22935

merged 5 commits into from
Jan 26, 2024

Conversation

nico
Copy link
Contributor

@nico nico commented Jan 25, 2024

.pam is a "portrable arbitrarymap" as documented at
https://netpbm.sourceforge.net/doc/pam.html

It's very similar to .pbm, .pgm, and .ppm, so this uses the
PortableImageMapLoader framework. The header is slightly different,
so this has a custom header parsing function.

Also, .pam only exixts in binary form, so the ascii form support
becomes optional.


This doesn't yet implement the BLACKANDWHITE DEFINED TUPLE TYPE from https://netpbm.sourceforge.net/doc/pam.html since the spec isn't super clear on what that means.

It does add support for the CMYK tuple type which isn't documented on that page, but which mutool extract writes for CMYK images in PDFs. See #22922 (comment) for an example.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jan 25, 2024
@LucasChollet
Copy link
Member

I just realized, can we have a test? 😃

.pam is a "portrable arbitrarymap" as documented at
https://netpbm.sourceforge.net/doc/pam.html

It's very similar to .pbm, .pgm, and .ppm, so this uses the
PortableImageMapLoader framework. The header is slightly different,
so this has a custom header parsing function.

Also, .pam only exixts in binary form, so the ascii form support
becomes optional.
These are written by `mutool extract` for CMYK images.

They don't contain color profiles so they're not super convenient.
But `image` can convert them (*) to sRGB as long as you use it with
`--assign-color-profile` pointing to some CMYK icc profile of your
choice.

*: Once SerenityOS#22922 is merged.
Hand-written in a text editor.

I verified that `convert` converts it to a png that looks like the
test expectations.
Hand-written in a text editor.

I verified that `convert` converts it to a png that looks like the
rgb test expectations.
@nico
Copy link
Contributor Author

nico commented Jan 25, 2024

Sure, added tests. I couldn't find a prebuilt binary of https://netpbm.sourceforge.net/doc/pamtopng.html in apt-cache, so I hand-wrote the files and checked that they convert ok using imagemagick's convert.

Copy link
Member

@LucasChollet LucasChollet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

@LucasChollet LucasChollet added ✅ pr-community-approved PR has been approved by a community member and removed 👀 pr-needs-review PR needs review from a maintainer or community member labels Jan 26, 2024
@awesomekling awesomekling merged commit c694d43 into SerenityOS:master Jan 26, 2024
15 checks passed
@github-actions github-actions bot removed the ✅ pr-community-approved PR has been approved by a community member label Jan 26, 2024
@nico nico deleted the pam branch January 26, 2024 13:06
nico added a commit to nico/serenity that referenced this pull request Jan 26, 2024
This is already done at the caller decode() in
PortableImageLoaderCommon.h, as pointed out by @LucasChollet at
SerenityOS#22935 (comment)

No behavior change.
awesomekling pushed a commit that referenced this pull request Jan 26, 2024
This is already done at the caller decode() in
PortableImageLoaderCommon.h, as pointed out by @LucasChollet at
#22935 (comment)

No behavior change.
kleinesfilmroellchen pushed a commit to kleinesfilmroellchen/serenity that referenced this pull request Jan 27, 2024
This is already done at the caller decode() in
PortableImageLoaderCommon.h, as pointed out by @LucasChollet at
SerenityOS#22935 (comment)

No behavior change.
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.

3 participants