Skip to content

Conversation

@barucden
Copy link
Contributor

@barucden barucden commented May 9, 2024

Description

This PR allows users to restrict the version of an output PDF like this:

fig = Figure()
ax = Axis(fig[1, 1])
x = range(0, 10, length=100)
y = sin.(x)
save("version_1-4.pdf", fig, pdf_version="1.4")
save("version_1-7.pdf", fig, pdf_version="1.7")

shell> file version_1-4.pdf
version_1-4.pdf: PDF document, version 1.4, 1 page(s)

shell> file version_1-7.pdf
version_1-7.pdf: PDF document, version 1.7

This feature was discussed on Discourse (and also earlier)

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in CHANGELOG.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.

I need guidance regarding documentation (where to best put this) and testing (I am not sure how to check a PDF's version in Julia).

Also I was thinking that the CAIRO_PDF_VERSION_* constants together with restrict_pdf_version! should perhaps go to Cairo.jl. What do you think?

@barucden barucden force-pushed the restrict-pdf-version branch 2 times, most recently from 21ec306 to 7a71fc5 Compare May 9, 2024 12:42
@barucden barucden force-pushed the restrict-pdf-version branch from 7a71fc5 to 713efce Compare May 9, 2024 14:16
@t-bltg t-bltg added enhancement Feature requests and enhancements CairoMakie This relates to CairoMakie, the vector backend for Makie based on Cairo. labels May 11, 2024
@barucden barucden force-pushed the restrict-pdf-version branch 2 times, most recently from 08fee5d to e1f5a49 Compare May 13, 2024 09:33
@barucden
Copy link
Contributor Author

I have added an entry to changelog and documented the new config option.

Copy link
Member

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@barucden barucden force-pushed the restrict-pdf-version branch from e1f5a49 to 8940126 Compare May 15, 2024 07:17
@barucden
Copy link
Contributor Author

Rebased to resolve new conflicts

@barucden
Copy link
Contributor Author

The error is not related imho:

GLMakie/Normals of a Cat.png: Test Failed at /home/runner/work/Makie.jl/Makie.jl/ReferenceTests/src/runtests.jl:117
  Expression: score <= threshold
   Evaluated: 0.5715945363044739 <= 0.05

@barucden
Copy link
Contributor Author

CI's green ✅ Thank you, Simon.

@barucden barucden force-pushed the restrict-pdf-version branch from 1b29111 to fc572a2 Compare May 22, 2024 07:30
@barucden
Copy link
Contributor Author

Rebased to fix conflicts.

@barucden barucden force-pushed the restrict-pdf-version branch from fc572a2 to aa057ee Compare May 22, 2024 13:16
@barucden barucden force-pushed the restrict-pdf-version branch from aa057ee to 74291b0 Compare May 22, 2024 13:57
return IMAGE # By default assume that the render type is IMAGE
end

function restrict_pdf_version!(surface::Cairo.CairoSurface, v::Integer)
Copy link
Member

Choose a reason for hiding this comment

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

would be simpler to type the v with the enum here, then you don't need yet another check. I'd guess you don't even have to manually convert it to Int32 in the ccall

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to move restrict_pdf_version! to Cairo.jl eventually, where they stick to numerical constants instead of enums, so I'd prefer to keep the argument as Integer.

You are right that the manual conversion in the ccall is not needed. I removed it.

@barucden barucden force-pushed the restrict-pdf-version branch from 74291b0 to 529ca53 Compare May 22, 2024 14:13
@barucden
Copy link
Contributor Author

This PR was taken over by #3885.

@barucden barucden closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CairoMakie This relates to CairoMakie, the vector backend for Makie based on Cairo. enhancement Feature requests and enhancements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants