Skip to content

Add ability to write pixel density into pngs#79

Merged
timholy merged 2 commits intoJuliaIO:masterfrom
jkrumbiegel:jk/dpi
Feb 20, 2025
Merged

Add ability to write pixel density into pngs#79
timholy merged 2 commits intoJuliaIO:masterfrom
jkrumbiegel:jk/dpi

Conversation

@jkrumbiegel
Copy link
Copy Markdown
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
Copy Markdown
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 thread src/io.jl
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Comment thread test/test_dpi.jl Outdated
Comment thread test/test_dpi.jl
Co-authored-by: Tim Holy <tim.holy@gmail.com>
@timholy timholy merged commit dd2dd83 into JuliaIO:master Feb 20, 2025
@timholy
Copy link
Copy Markdown
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