Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Create tiny mathtext baseline images using svg with non-embedded fonts.#19201

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
anntzer wants to merge3 commits intomatplotlib:masterfromanntzer:mathtextsvgtests

Conversation

anntzer
Copy link
Contributor

@anntzeranntzer commentedDec 30, 2020
edited
Loading

... and using non-integer bases sqrt as example use case.

See#19186 (comment).

While I was at it I also fixed/shortened the font attribute used to specify the font (first commit).

PR Summary

PR Checklist

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (runflake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

@anntzeranntzerforce-pushed themathtextsvgtests branch 6 times, most recently from84adc51 to78df724CompareDecember 31, 2020 18:55
@timhoffm
Copy link
Member

Generally 👍 on the concept.

Questions:

  • SVG is harder to compare visually than PNG. I assume it's smaller though. Is the added complexity worth the savings compared to just using PNG?
  • Is it reasonable/necessary to check all fonts?

@anntzer
Copy link
ContributorAuthor

I agree pngs would be simpler, but...
From a quick test (on the single test case used here): the current svg baseline is 2.8K (and git should internally gzip it down, which results in a file just below 1K). "Naively" switching to png results in a much larger (7K) file. Somewhat surprisingly, even passingpil_kwargs={"optimize": True} (https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#png) only reduces it to 6.3K; passing the file through optipng does crush it down to 3.5K though. But git shouldn't be able to further compress down the resulting file via gzip.
Also, I expect svgs to diff much more efficiently if we change e.g. precise glyph positionings.

Checking all fonts is likely mostly relevant for test_mathfont_rendering only, I guess?

@timhoffm
Copy link
Member

OK, so only removing SVG and PS wouldn't gain us much because PNG uses the most space anyway.

I'm very hesitant to use optipng for testing. First that'd make it a hard dependency for testing. And second more importantly, we'd bake the compression algorithm into our tests. If optipng changes somehow it could break all our tests.

It likely has to be SVG if we want to save space.

@QuLogic
Copy link
Member

If thesqrt code itself needs to change (as opposed to its test), it should at least be in a separate commit. I'm not sure why those changes are happening as there's no context for it.

This replaces e.g.`"font-family:DejaVu Sans;font-size:12px;font-style:book;font-weight:book;"`by `"font: 400 12px 'DejaVu Sans'"`.Note that the previous font weight was plain wrong...
@anntzer
Copy link
ContributorAuthor

Done.
The changes to sqrt were mostly to make sure that the new machinery works in a concrete use case.

@timhoffm
Copy link
Member

Conclusion from the dev call today:

  • Take font spec improvement (first commit); it's independent anyway
  • Limit baseline images to SVG
  • Don't use fonts in the SVG but keep the fonts being converted to paths.
    Concern is on the added complexity; and the production code uses converted paths, so testing these is closer to the production case. We live with the larger SVG size due to paths in test images for now. Tom is planning to prioritize the separate repo for test images, so image size is a little less critical.

@anntzer
Copy link
ContributorAuthor

Don't use fonts in the SVG but keep the fonts being converted to paths.

I don't have hard numbers right now but IIRC embedding the paths makes the SVGs much larger, defeating the purpose of the PR... in that case we may just as well stick to png only for ease of use.

@timhoffm
Copy link
Member

You're right. The existing svgs undertest_mathtext are roughly 2x the size of the corresponding png. Given that, the pragmatic solution is to stick to png only. So let's

  • Take font spec improvement (first commit); it's independent anyway
  • Limit baseline images to PNG

@anntzer
Copy link
ContributorAuthor

milestoning as 3.4 as a dependency for#18916

@QuLogic
Copy link
Member

Aren't we doing#19261 instead?

@anntzer
Copy link
ContributorAuthor

Yup, milestoned the wrong one :p

@anntzeranntzer deleted the mathtextsvgtests branchJanuary 16, 2021 20:51
@QuLogicQuLogic removed this from thev3.4.0 milestoneMar 16, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@anntzer@timhoffm@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp