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

latex template: set fonts after Beamer theme #8316

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

knuesel
Copy link
Contributor

@knuesel knuesel commented Sep 23, 2022

Beamer themes such as metropolis and saintpetersburg change the default fonts. This PR gives precedence to the user font settings by moving them after the loading of the Beamer theme.

This fixes #6260

@jgm
Copy link
Owner

jgm commented Sep 26, 2022

I'm slightly nervous about the way in which this breaks up the current font settings, moving some of them after the beamer theme options and leaving some where they are. That adds complexity. Wouldn't it be simpler just to move the block concerning beamer themes before all the font settings? Or is there some reason that won't work?

@knuesel
Copy link
Contributor Author

knuesel commented Sep 26, 2022

I agree splitting the font-related code is unfortunate. I did it this way to minimize the risk of introducing problems or changing current behavior: Moving some \setXXXfont calls later is unlikely to cause an issue. On the other hand, moving the \useXXXtheme calls earlier (for example at the end of the previous beamer-related stuff) might be more impactful: some theme files load several packages, and I wanted to avoid changing the package loading order.

Now looking in more details at what the themes actually do, I don't see anything obvious that should cause troubles. I also just tested a template with beamer theme loading moved upwards together with other beamer commands, and was able to compile a few presentations without issue. Shall I amend this PR accordingly?

@jgm
Copy link
Owner

jgm commented Sep 26, 2022

Yes, I think it's desirable if possible to keep the font stuff together in the template.
If you could amend accordingly and test with a number of theme/font combinations, that would be great.

@knuesel
Copy link
Contributor Author

knuesel commented Sep 27, 2022

I used the following document:

---
theme: metropolis
monofont: JuliaMono
...

# Title

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua.

```
ẋ, x⃗, x⃗̇, x͍⃡, x↑, θ, θ̂
```

and compared the output between these two versions:

  1. Current pandoc release
  2. A variant of this PR with the beamer theme logic all moved before the font logic: knuesel@0aca259

This uncovers at least two issues with the second version:

  • The default font settings of the Metropolis theme are lost because \usepackage{lmodern} now comes later and overwrites them.
  • Commenting \usepackage{lmodern} fixes the font family but the sizes are different from the current pandoc release, because the \defaultfontfeatures{Scale=MatchLowercase} now come too late to have an effect. (It's not clear to me if this change is a bug or is fixing previously wrong behavior).

Another possibilty is to move the Beamer logic after the main font package loading and defaults, but before the font setting code: knuesel@e4d6a95 . Compared to the current PR, this leaves the loading of packages xeCJK, luatexja-preset and luatexja-fontxpec after the Beamer stuff, together with other CJK code, so it looks a bit less messy. I haven't found any issue with this version. Maybe it's a good trade off?

@jgm
Copy link
Owner

jgm commented Sep 27, 2022

Another possibilty is to move the Beamer logic after the main font package loading and defaults, but before the font setting code: knuesel@e4d6a95 . Compared to the current PR, this leaves the loading of packages xeCJK, luatexja-preset and luatexja-fontxpec after the Beamer stuff, together with other CJK code, so it looks a bit less messy. I haven't found any issue with this version. Maybe it's a good trade off?

Thanks for investigating. That approach seems cleaner to me. But it would be worth checking with e.g. a Chinese document, with lang set to zh-CN, so that CJK code will be used.

@knuesel
Copy link
Contributor Author

knuesel commented Sep 28, 2022

I tried compiling a Beamer presentation from https://github.com/mba811/dev-study/tree/master/Markdown with metadata

---
theme: metropolis
monofont: JuliaMono
lang: zh-CN
CJKmainfont: SimSun
...

and it works fine (with and without the proposed changes) with xelatex and lualatex. (With lualatex there's an unrelated error that looks like https://tex.stackexchange.com/questions/609104/lualatex-ctex-and-microtype-together-produce-errors-when-using-scshape; it works when disabling microtype.)

I've updated the PR to the new approach.

@jgm
Copy link
Owner

jgm commented Sep 28, 2022

To fix the tests, you can do make TESTARGS=--accept.
This will replace the expected test values with what your modified pandoc produces.
Just be sure to look over the changes to the tests and make sure everything is correct.

@knuesel
Copy link
Contributor Author

knuesel commented Sep 29, 2022

Ah thanks, the tests should pass now. The golden files now have a silly section

\ifPDFTeX
\else
\fi

but that seems difficult to avoid without making the template much more verbose (with a pdftex check for each font setting).

@jgm
Copy link
Owner

jgm commented Oct 1, 2022

That is odd.
Maybe we could make it less odd by doing it this way:

\ifPDFTeX
\else
% Font selection: after Beamer theme selection to override e.g. metropolis or
% saintpetersburg font selection
$if(mainfont)$
  \setmainfont[$for(mainfontoptions)$$mainfontoptions$$sep$,$endfor$]{$mainfont$}
$endif$

Then it will look like

\ifPDFTeX
\else
% Font selection: after Beamer theme selection to override e.g. metropolis or
% saintpetersburg font selection
\fi

and it least it will be clear what this section is for. I don't know, what do you think?

The other thing that's a bit odd is that fonts can be selected to override beamer theme fonts ONLY in xelatex/lualatex. Why should that be?

@knuesel
Copy link
Contributor Author

knuesel commented Oct 3, 2022

Good idea, and I also see it's odd for the comment to mention a Beamer theme, when using regular LaTeX. I pushed some changes, so now in the worst case it looks like this:

\usepackage{amsmath,amssymb}
\usepackage{lmodern}
\usepackage{iftex}
\ifPDFTeX
  \usepackage[T1]{fontenc}
  \usepackage[utf8]{inputenc}
  \usepackage{textcomp} % provide euro and other symbols
\else % if luatex or xetex
  \usepackage{unicode-math} % this also loads fontspec
  \defaultfontfeatures{Scale=MatchLowercase}
  \defaultfontfeatures[\rmfamily]{Ligatures=TeX,Scale=1}
\fi
\ifPDFTeX\else
  % xetex/luatex font selection
\fi

It still looks a bit odd to have the empty "if" block split from the rest, but that's already an improvement I think?

Regarding the second point: another good point... in the PR I'm only moving the fontspec commands \setmainfont, \setsansfont, etc. which correspond to the pandoc settings mainfont, etc. Pdflatex doesn't support these commands since it cannot use truetype/opentype fonts. Currently the template does this very early:

$if(fontfamily)$
\usepackage[$for(fontfamilyoptions)$$fontfamilyoptions$$sep$,$endfor$]{$fontfamily$}
$else$
\usepackage{lmodern}
$endif$

This applies to all engines. I think the lmodern default must stay there, but it would make sense to move the fontfamily case later with the xetex/luatex commands, at the risk of changing the behavior for some users... I can make some tests if that approach sounds good to you.

@jgm
Copy link
Owner

jgm commented Oct 3, 2022

It still looks a bit odd to have the empty "if" block split from the rest, but that's already an improvement I think?

\fi
\ifPDFTeX\else

We could just put these lines under the $if(beamer)$ so that they only appear in beamer output, with the result that the font selection is included in the regular else branch for standard tex output. That is, just move the endif at line 131 down a couple lines...

@jgm
Copy link
Owner

jgm commented Oct 3, 2022

I think the \lmodern default must stay there, but it would make sense to move the fontfamily case later with the xetex/luatex commands, at the risk of changing the behavior for some users... I can make some tests if that approach sounds good to you.

This does sound more consistent, so please do.

Beamer themes such as metropolis and saintpetersburg change the default
fonts. This change gives precedence to the user font settings by moving
them after the loading of the Beamer theme.
@knuesel
Copy link
Contributor Author

knuesel commented Oct 6, 2022

We could just put these lines under the $if(beamer)$ so that they only appear in beamer output, with the result that the font selection is included in the regular else branch for standard tex output. That is, just move the endif at line 131 down a couple lines...

That would work well, but it's kind of incompatible with the other change (moving the fontfamily setting between the Beamer theme and the xetex/luatex font selection) because fontfamily applies to all engines so that's another need for interrupting the xetex/luatex \if block.

However I think it actually makes perfect sense to move the lmodern default closer to the other font selection but just before the Beamer theme, and this also helps with the useless blocks. I pushed a new version implementing this and better documenting the template code. Now in the worst case it looks like this:

\ifPDFTeX
  \usepackage[T1]{fontenc}
  \usepackage[utf8]{inputenc}
  \usepackage{textcomp} % provide euro and other symbols
\else % if luatex or xetex
  \usepackage{unicode-math} % this also loads fontspec
  \defaultfontfeatures{Scale=MatchLowercase}
  \defaultfontfeatures[\rmfamily]{Ligatures=TeX,Scale=1}
\fi
\usepackage{lmodern}  % Beamer theme if defined would be set *after* this line
\ifPDFTeX\else
  % xetex/luatex font selection
\fi

or if fontfamily is defined:

\ifPDFTeX
  \usepackage[T1]{fontenc}
  \usepackage[utf8]{inputenc}
  \usepackage{textcomp} % provide euro and other symbols
\else % if luatex or xetex
  \usepackage{unicode-math} % this also loads fontspec
  \defaultfontfeatures{Scale=MatchLowercase}
  \defaultfontfeatures[\rmfamily]{Ligatures=TeX,Scale=1}
\fi
\usepackage[]{times}  % Beamer theme if defined would be set *before* this line
\ifPDFTeX\else
  % xetex/luatex font selection
\fi

I find this finally starts to make sense... What do you think?

@jgm jgm merged commit 644c984 into jgm:master Oct 6, 2022
@jgm
Copy link
Owner

jgm commented Oct 6, 2022

This looks good, thanks for iterating!

@knuesel
Copy link
Contributor Author

knuesel commented Oct 6, 2022

Thanks for the comments!

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.

latex template sets fonts before applying beamer theme
2 participants