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

Gradient Part 6 - Pattern fills #2740

Merged
merged 35 commits into from
Nov 24, 2023
Merged

Gradient Part 6 - Pattern fills #2740

merged 35 commits into from
Nov 24, 2023

Conversation

Dherse
Copy link
Sponsor Collaborator

@Dherse Dherse commented Nov 22, 2023

Tracking issue: #2282

Implements basic pattern fills with an purposefully small API based on the PDF as the common denominator (as it has the most basic support). It is implemented for SVG, PNG with 100% compatibility and in PDF where some readers are slightly buggy but I will be opening bug reports shortly (most notably with mupdf).

The api is as follow

  • New pattern function and type
  • The constructor takes:
    • a Content as body
    • an array of two absolute sizes as the bounding box, the size of the content (required) creates an error if it has em set
    • an optional spacing (same rules as the bbox)
    • an optional relative that works the same as gradients

Some limitations:

  • In SVG we need to render the body twice, this is unfortunate but required because we cannot write subtrees of the XML with xmlwriter, I suggested on Discord opening a PR that replaces xmlwriter with our version that supports this, since it would allow us writing the defs at the top (for compatibility with some SVG readers) and would allow to render patterns only once
  • The SVG change means that file sizes are now slightly higher but I think this can be fixed in a second PR.
  • In PDF there can be some (albeit small) artefacts, it's unclear whether this is a bug (although I think it is since some PDF viewers work perfectly: Acrobat, PDF.js, XPDF, poppler), only mupdf and pdfium seem to be impacted (will open issues in their respective repos)
  • I had to change the way text paths are encoded in SVG, this solves the issue of text having gigantic scaling values, this helps readers with patterns on text (unclear why), at the same time it makes SVG text source a bit more readable and reusable.

Here is an example:

#rotate(45deg, scale(x: 50%, y: 70%, rect(
  width: 100%,
  height: 100%,
  stroke: 1pt,
)[
  #set text(
    fill: pattern(
      (30pt, 30pt),
      relative: "parent",
      square(size: 30pt, fill: gradient.conic(..color.map.rainbow))
    )
  )

  #lorem(10)
]))

Which looks like this:

image

I would generally consider the implementation a success, perhaps there is a way we could implement a work around for PDFs and I'd be intereted to hear your opinion/ideas. Here is a test PDF file that shows these small issues for those that want to try it:

test.pdf

@Dherse Dherse mentioned this pull request Nov 22, 2023
13 tasks
@Dherse
Copy link
Sponsor Collaborator Author

Dherse commented Nov 22, 2023

As a draft because there is an SVG bug I am still tracking down.

@Dherse
Copy link
Sponsor Collaborator Author

Dherse commented Nov 22, 2023

Ok, the SVG bug is solved and I have updated the original comment to reflect that. I just need to add some automated tests now!

@Dherse Dherse marked this pull request as ready for review November 22, 2023 15:11
@Dherse
Copy link
Sponsor Collaborator Author

Dherse commented Nov 22, 2023

Ok, it all looks good on my end, just needs some review :D

@Dherse
Copy link
Sponsor Collaborator Author

Dherse commented Nov 23, 2023

Thanks to @EpicEricEE for pointing out a bug that could occur in PNG rendering that was due to using .ceil() instead of .round(), it was fixed in 6249bb4

@Dherse
Copy link
Sponsor Collaborator Author

Dherse commented Nov 24, 2023

@laurmaedje I am thinking of adding a fill parameter for setting the background of the pattern, what do you think?

crates/typst-pdf/src/lib.rs Outdated Show resolved Hide resolved
crates/typst-pdf/src/lib.rs Outdated Show resolved Hide resolved
crates/typst-pdf/src/page.rs Outdated Show resolved Hide resolved
crates/typst-pdf/src/page.rs Outdated Show resolved Hide resolved
crates/typst-pdf/src/page.rs Outdated Show resolved Hide resolved
crates/typst/src/visualize/pattern.rs Outdated Show resolved Hide resolved
crates/typst/src/visualize/pattern.rs Outdated Show resolved Hide resolved
crates/typst/src/visualize/pattern.rs Outdated Show resolved Hide resolved
tests/typ/visualize/pattern-relative.typ Outdated Show resolved Hide resolved
tests/typ/visualize/pattern-small.typ Outdated Show resolved Hide resolved
@laurmaedje laurmaedje merged commit 1756718 into typst:main Nov 24, 2023
3 checks passed
@laurmaedje
Copy link
Member

Thank you! 🎉

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.

None yet

2 participants