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

Implement Typst backend #48

Merged
merged 7 commits into from
Jun 9, 2024
Merged

Implement Typst backend #48

merged 7 commits into from
Jun 9, 2024

Conversation

jakobjpeters
Copy link
Contributor

@jakobjpeters jakobjpeters commented Jun 8, 2024

As described in the documentation and this comment, this pull request implements a Typst backend using Typstry.jl.

This initial draft contains a minimal working implementation:

julia> using MakieTeX, CairoMakie

julia> teximg(convert(MakieTeX.PDFDocument, MakieTeX.CachedTypst(typst"$ 1 / x $")))

TODO

  • Test
    • Perform manual testing
    • Write tests
  • Write additional documentation
  • Factor out duplicated code
  • Check Typst details
    • Choose a preamble

src/recipe.jl Outdated Show resolved Hide resolved
src/rendering/typst.jl Outdated Show resolved Hide resolved
src/types.jl Outdated Show resolved Hide resolved
# instead of auto-decomposing into its component scatter plot.
CairoMakie.is_cairomakie_atomic_plot(plot::TeXImg) = true
CairoMakie.is_cairomakie_atomic_plot(plot::Union{TeXImg, TypstImg}) = true
Copy link
Member

Choose a reason for hiding this comment

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

In this case I think we should let TeXImg be the main/only plot type, similarly for LTeX - they would just accept TypstDocuments as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify, I meant that we should remove the TypstImg and LTypst entirely, since they don't have any differentiating factors from LTeX and the two would get a bit confusing IMO. If there is some additional value to be gained by having separate recipes then we can look into this again, but for now it seems easier to just have one canonical recipe to plot things with...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, thank you for elaborating. I didn't actually know what it did and tried to revert it.

@asinghvi17
Copy link
Member

BTW: don't worry about the CI, it gets stuck somewhere, probably in the render_texample bit...I will probably have to clean it up at some point!

@asinghvi17
Copy link
Member

https://typst.app/universe/package/cetz/ has some nice illustrations that one could render in a similar way to render_texample as tests.

@jakobjpeters
Copy link
Contributor Author

Alright, I think it's almost done. I ran test/typst.jl locally and those examples worked. Do you have suggestions for additional tests or things to check manually?

Is there anything to consider regarding Typst and Makie integration, such as the preamble?

@jakobjpeters jakobjpeters marked this pull request as ready for review June 9, 2024 11:08
@asinghvi17 asinghvi17 merged commit e255725 into MakieOrg:master Jun 9, 2024
@asinghvi17
Copy link
Member

asinghvi17 commented Jun 9, 2024

Ah, I was trying to push to the PR and it looks like I somehow pushed to this repo's master instead! Oops.

No worries - the PR was in pretty good shape anyway and local testing seemed to work well. Thanks again @jakobjpeters!

In terms of additional tests, not really - that the basic functionality works is mostly what I wanted. It seems as though Typst doesn't really need a preamble unless you're using a package, and if you are then you can just write that import statement anywhere if I understand correctly? So we can use a preamble later, especially in text integration for fonts, colors, etc, but right now there doesn't seem to be a need for it.

asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
asinghvi17 pushed a commit that referenced this pull request Jun 9, 2024
@jakobjpeters
Copy link
Contributor Author

Woo! You're welcome

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.

2 participants