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

Rich text #2321

Merged
merged 47 commits into from
Nov 23, 2022
Merged

Rich text #2321

merged 47 commits into from
Nov 23, 2022

Conversation

jkrumbiegel
Copy link
Member

@jkrumbiegel jkrumbiegel commented Oct 8, 2022

Description

Adds a rich text data structure which allows to plot strings with colors, subscripts, superscripts, different fonts and fontsizes specified internally. Also adds the ability to specify fonts by symbols such as :regular and passing a top-level dictionary that resolves these symbols.

This PR is breaking in two ways:

  • Symbols are not resolved equivalently to strings anymore. This should mostly not matter because all our examples used strings and the symbol behavior was not advertised.
  • Doing Figure(..., font = xyz) will not have the expected effect anymore, because all the Blocks like Axis which previously inherited their fonts from this field do not do this anymore. Instead, they are set to :regular or :bold. The new way to change fonts should therefore be Figure(..., fonts = (; regular = xyz, bold = abc)). The benefit of this is obviously that you can theme more than one font category, before you would have had to change Axis3 title, Axis title, Legend title all separately if you didn't want the bold font used there.

Examples:

using Random
Random.seed!(123)

scatter(rand(100, 2),
    axis = (
        yscale = log10,
        xscale = log,
        title = rich(
            "H",
            subscript("2"),
            "O is a ",
            rich("great", font = :bold_italic, color = :dodgerblue1),
            " way to hydrate"
        )
    ),
)

image

Using rich superscripts also fixes the mismatching exponents because TeX Gyre Heros only had superscripts 1 to 3:

Before:

image

After:

image

Delete options that do not apply:

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@MakieBot
Copy link
Collaborator

MakieBot commented Oct 8, 2022

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running:

using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))
using create display create display
GLMakie 28.59s (28.47, 28.70) 0.09+- 17.44s (17.33, 17.59) 0.09+- 16.48s (16.35, 16.59) 0.09+- 12.93ms (12.75, 13.10) 0.13+- 49.66ms (49.01, 50.82) 0.68+-
master 28.17s (28.12, 28.21) 0.03+- 17.51s (17.37, 17.60) 0.08+- 16.44s (16.29, 16.53) 0.09+- 12.56ms (12.41, 12.66) 0.08+- 50.27ms (49.36, 50.77) 0.54+-
evaluation +1.46%, 0.42s slower X (6.40d, 0.00p, 0.06std) -0.39%, -0.07s invariant (-0.81d, 0.15p, 0.08std) +0.25%, 0.04s invariant (0.46d, 0.41p, 0.09std) +2.92%, 0.38ms slower X (3.53d, 0.00p, 0.10std) -1.24%, -0.61ms invariant (-1.00d, 0.09p, 0.61std)
CairoMakie 25.71s (25.64, 25.83) 0.06+- 17.54s (17.36, 17.75) 0.12+- 2.51s (2.50, 2.53) 0.01+- 13.04ms (12.87, 13.21) 0.11+- 1.03ms (1.02, 1.05) 0.01+-
master 24.92s (24.74, 25.03) 0.10+- 17.51s (17.40, 17.61) 0.07+- 2.55s (2.53, 2.58) 0.02+- 12.52ms (12.34, 12.69) 0.12+- 1.03ms (1.02, 1.05) 0.01+-
evaluation +3.07%, 0.79s slower X (9.13d, 0.00p, 0.08std) +0.20%, 0.04s invariant (0.36d, 0.52p, 0.10std) -1.36%, -0.03s faster ✓ (-2.46d, 0.00p, 0.01std) +3.99%, 0.52ms slower X (4.51d, 0.00p, 0.12std) -0.15%, -0.0ms invariant (-0.17d, 0.76p, 0.01std)
WGLMakie 34.49s (33.73, 35.36) 0.51+- 26.88s (26.14, 27.74) 0.55+- 44.95s (43.55, 47.29) 1.34+- 20.68ms (18.95, 22.32) 1.19+- 120.86ms (114.27, 127.08) 4.68+-
master 34.13s (33.39, 34.86) 0.53+- 27.15s (26.65, 28.12) 0.50+- 44.94s (44.34, 45.93) 0.57+- 18.01ms (17.51, 18.67) 0.44+- 107.96ms (88.51, 164.61) 26.64+-
evaluation +1.06%, 0.36s invariant (0.70d, 0.21p, 0.52std) -0.98%, -0.26s invariant (-0.50d, 0.37p, 0.52std) +0.00%, 0.0s invariant (0.00d, 1.00p, 0.96std) +12.93%, 2.67ms slower❌ (2.97d, 0.00p, 0.82std) +10.67%, 12.89ms noisy🤷‍♀️ (0.67d, 0.25p, 15.66std)

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@asinghvi17
Copy link
Member

Can I suggest that instead of specifying bold etc fonts directly, we allow the user to simply pass a FontFamily object, which could contain preresolved fonts? This would allow people to use arbitrary styles (small caps, semibold, etc).

What I thought of was something like this:

...
font = "TeX Gyre Heros Makie"
# or
texgyre_family = FontFamily("TeX Gyre Heros Makie"; semibold = "Calibri") # for example, can set any font
...
font = texgyre_family

This way, one could simply specify via the x-label formatter if the font should be bold etc.

@jkrumbiegel
Copy link
Member Author

Can I suggest that instead of specifying bold etc fonts directly, we allow the user to simply pass a FontFamily object, which could contain preresolved fonts? This would allow people to use arbitrary styles (small caps, semibold, etc).

What I thought of was something like this:

...
font = "TeX Gyre Heros Makie"
# or
texgyre_family = FontFamily("TeX Gyre Heros Makie"; semibold = "Calibri") # for example, can set any font
...

That's kind of how it works in this PR, no?

fonts = Dict(
    :regular => "Makie Regular",
    :bold => "Makie Bold",
    :semibold => "Calibri" # whatever
)

set_theme!(fonts = fonts)

text(1, 2, text = "hi", font = :semibold)

@asinghvi17
Copy link
Member

Hmm, that does make sense. Basically what I wanted to make sure of was that (a) people can still set axis.font="....", and that (b) the user should be able to set set fonts = "Calibri", and then axis.font = :bold would resolve to Calibri Bold automatically, i.e., the user should deal in font families, not fonts - which is what most layman users would think in. One doesn't set the font to "Calibri Bold" in Word, after all :D

For (a) it looks like you can already do this, but (b) would basically just require an easier way to set the font family. One way we could do this is by somehow auto-converting when a String is given to fonts, into a similar dictionary...MakieSlides had some utilities for this as I recall. Have to step away for a bit but will post where that is later, it's basically just searching through FreeType for a bold, italic, etc version of the font.

@jkrumbiegel
Copy link
Member Author

jkrumbiegel commented Nov 3, 2022

One doesn't set the font to "Calibri Bold" in Word, after all

I personally really dislike choosing fonts not by name but by abstract attributes. I know that there are some fonts that only have the regular, italic, bold, bold italic combination, but I tend to want to use fonts that have like 30 variants. That's why I wanted to make it easy to pick exactly the fonts you want (by name) and not make it convenient to do something that is often correct but bites you in all other cases. That's matplotlib's system to me, for example. I once dove down all the way into their font handling code just to understand why my font theme didn't resolve correctly, and it was all heuristics about font weights and stuff. The family plus style name is actually really easy to do find out and use in my opinion, so I stuck with that in Makie.

What you can do on the other hand, is add a convenience function like get_fonts("Calibri", [:bold, :regular, :italic, ...]) and it would try to resolve those and error if it can't.

@asinghvi17
Copy link
Member

So I was looking into this, and ended up diving a bit into Fontconfig/Freetype interaction. Ended up writing a basic integration which allows us to use Fontconfig. Will make that a PR to this one in the next couple of days.

Good news: figured out how to use Fontconfig with Makie, meaning that we can use that as a somewhat better search engine...

This raises another edge case which should be solved before this gets merged - how should we handle font names? Some fonts have them labelled as oblique, instead of italic, which is fair enough but liable to get users confused. Additionally, most style names in fonts come with spaces - meaning that we would either have to force the users to use the exact syntax of the font, or create some sort of lookup engine for the most common ones (possibly both!)

P.S. With this, we can actually use single-file fonts with multiple faces! If you tried "Helvetica Bold" as a font now, it wouldn't work, since we currently have no way of distinguishing between multiple fonts in a single file in Makie.

@jkrumbiegel
Copy link
Member Author

I did already think a lot about these problems years back when I rewrote the font finding in FreeTypeAbstraction, maybe have a look at the closed PRs there, there might still be bits of discussion. Anyway, I settled on using font names with a shortest match policy, and that seems to have worked well so far. I wouldn't want you to spend tons of time on getting fontconfig to work if that doesn't clearly improve on a problem we currently have. And so far I haven't seen that problem, so if you want to go forward, probably best to collect problematic use cases first and present them here for further discussion :)

@asinghvi17
Copy link
Member

Well, the problem as I see it is the following:

to_font("Helvetica Bold") returns FTFont (family = Helvetica, style = Medium), which is not what was requested - and on assigning this font, there's no change in the plot. This is despite the fact that there is a bold font in the ttf file, but the font-finding algorithm we have now cannot find it, since it can only access the 0th index of the file.

Similar things apply to basically any font which a user downloads as a single file, which is somewhat prevalent on common font sites.

I just put up #2400 which implements this interface - after looking at the Fontconfig docs, it was surprisingly easy. Do give it a look!

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

Copy link
Member

@SimonDanisch SimonDanisch left a comment

Choose a reason for hiding this comment

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

Really great to have this!! :)
From what I can tell, this is not breaking, right?
If it is, we should tag current master as a patch/bugfix release first.
Apart from this, I think this is ready to be merged!
We should not forget to add the new refimage, though!

@jkrumbiegel
Copy link
Member Author

The description notes two ways in which the PR is breaking, so it should go into a minor release. I was thinking to bundle it with the fontsize refactor.

@jkrumbiegel jkrumbiegel changed the base branch from master to jk/release-v0.19 November 23, 2022 11:48
@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@stevengj
Copy link
Contributor

stevengj commented Nov 16, 2023

It would be nice to open a separate PR that contains the hyphen/minus change, as suggested above.

Matplotlib does this, and it seems like all professional mathematical typesetting (e.g. LaTeX or CMOS) uses a different (wider) glyph for the minus sign than for a hyphen. It's quite painful to ask users to do this manually.

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.

7 participants