Skip to content

Add ability to write pixel density into pngs #79

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

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

jkrumbiegel
Copy link
Contributor

I would like to make Makie write dpi information into the pngs it produces. This PR adds the necessary ability to PNGFiles.

@jkrumbiegel
Copy link
Contributor Author

The failing run on mac aarch64 seems to be the same in all recent CI runs, so not related to this code change. Otherwise this is good to go from my side.

Comment on lines +343 to +345
- `dpi`: stores the pixel density given in dots per inch into the `pHYs` chunk as pixels per meter.
The density is given as `dpi` for ease of use as pixels per meter is an uncommon format. If set to `nothing`,
no pixel density is written. If set to a 2-element tuple, a different density is written for x and y, respectively.
Copy link
Member

Choose a reason for hiding this comment

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

Pixels per meter seems to be native to PNG (cf. https://www.w3.org/TR/PNG-Chunks.html), and it is using an SI unit, so even though DPI may be more common, why not at least support both a pixels_per_meter and a dots_per_inch keyword?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would also be possible, I just don't think anybody will really use that :)

Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

Other than possible test issues, LGTM

Co-authored-by: Tim Holy <tim.holy@gmail.com>
@timholy timholy merged commit dd2dd83 into JuliaIO:master Feb 20, 2025
5 of 6 checks passed
@timholy
Copy link
Member

timholy commented Feb 20, 2025

Thanks!

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