GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Create tiny mathtext baseline images using svg with non-embedded fonts. by anntzer · Pull Request #19201 · matplotlib/matplotlib

Conversation

@anntzer

@anntzer anntzer commented Dec 30, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

... 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 (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@anntzer anntzer added the topic: testing label Dec 30, 2020
@anntzer
anntzer force-pushed the mathtextsvgtests branch 6 times, most recently from 84adc51 to 78df724 Compare December 31, 2020 18:55
@timhoffm

timhoffm commented Jan 1, 2021

Copy link
Copy Markdown
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer

anntzer commented Jan 1, 2021

Copy link
Copy Markdown
Contributor Author

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 passing pil_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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer
anntzer force-pushed the mathtextsvgtests branch from 78df724 to e395766 Compare January 1, 2021 17:22
@timhoffm

timhoffm commented Jan 3, 2021

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@QuLogic

QuLogic commented Jan 5, 2021

Copy link
Copy Markdown
Member

If the sqrt 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anntzer added 3 commits January 5, 2021 07:22
@anntzer
Improve font spec for SVG font referencing.
5fd8ae8
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
Add machinery for svg-only mathtext tests.
15f6905
@anntzer
Support \sqrt[not-a-number]{...}.
f9cea50
@anntzer
anntzer force-pushed the mathtextsvgtests branch from e395766 to f9cea50 Compare January 5, 2021 06:24
@anntzer

anntzer commented Jan 5, 2021

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm

timhoffm commented Jan 5, 2021

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer

anntzer commented Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@timhoffm

timhoffm commented Jan 7, 2021

Copy link
Copy Markdown
Member

You're right. The existing svgs under test_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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jan 7, 2021
Improve font spec for SVG font referencing. #19253
Merged
Add machinery for png-only, single-font mathtext tests. #19261
Merged
@anntzer anntzer added this to the v3.4.0 milestone Jan 15, 2021
@anntzer

anntzer commented Jan 15, 2021

Copy link
Copy Markdown
Contributor Author

milestoning as 3.4 as a dependency for #18916

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@QuLogic

QuLogic commented Jan 16, 2021

Copy link
Copy Markdown
Member

Aren't we doing #19261 instead?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer

anntzer commented Jan 16, 2021

Copy link
Copy Markdown
Contributor Author

Yup, milestoned the wrong one :p

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer anntzer closed this Jan 16, 2021
@anntzer
anntzer deleted the mathtextsvgtests branch January 16, 2021 20:51
@QuLogic QuLogic removed this from the v3.4.0 milestone Mar 16, 2021
@anntzer anntzer mentioned this pull request Apr 16, 2022
Implement TeX's fraction alignment for Mathtext fractions #22852
Closed
2 of 3 tasks
@anntzer anntzer mentioned this pull request Apr 23, 2022
Support not embedding glyphs in svg mathtests. #22881
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic: testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@anntzer @timhoffm @QuLogic
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.