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

\@@_declare_shapes_smcaps:nn setup a slsc -> itsc substitution #425

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

DamienRobert
Copy link

If a slanted font is not provided, fontspec setup automatic sl -> it
substitutions in \@@_declare_shape_slanted:nn. But if a small caps
font was provided, an itsc shape is defined but there is no
corresponding slsc -> itsc substitution.

Fix this by adapting the code of \@@_declare_shape_slanted:nn into
\@@_declare_shapes_smcaps:nn.

Status

FOR DISCUSSION (but should be ready)

Todos

  • Tests added to cover new/fixed functionality
    (Did a manual test)
  • Documentation if necessary
    (Added developer documentation)
  • Code follows expl3 style guidelines

Minimal example demonstrating the new/fixed functionality

\documentclass{article}
\usepackage{fontspec}
\setmainfont{Latin Modern Roman}[
  ItalicFeatures  = { SmallCapsFont = {Latin Modern Roman Caps/I} },
]
\begin{document}
\fontshape{slsc}\selectfont Slanted Small Caps.
\end{document}

@DamienRobert
Copy link
Author

For instance here is my Latin Modern Sans fontspec file.
With this patch I am able to comment all Slanted features and still have the correct sl->it substitutions.

\defaultfontfeatures[Latin Modern Sans]{
  Ligatures={TeX},
  UprightFont = {*/R},
  SmallCapsFont = {Latin Modern Roman Caps},
  ItalicFeatures  = { SmallCapsFont = {Latin Modern Roman Caps/I} },
  BoldFeatures = { SmallCapsFeatures = { FakeBold=4 } },
  BoldItalicFeatures = {
    SmallCapsFont = {Latin Modern Roman Caps/I},
    SmallCapsFeatures = { FakeBold=4 } },
  SlantedFont={Latin Modern Sans/I},
  BoldSlantedFont={Latin Modern Sans/BI},
  SlantedFeatures  = { SmallCapsFont = {Latin Modern Roman Caps/I} },
  BoldSlantedFeatures = {
    SmallCapsFont = {Latin Modern Roman Caps/I},
    SmallCapsFeatures = { FakeBold=4 } },
  FontFace = {b}{n}{Font = {Latin Modern Sans/B}},
  FontFace = {b}{it}{Font = {Latin Modern Sans/BI}},
  FontFace = {b}{sl}{Font = {Latin Modern Sans/BI}},
  FontFace = {sbc}{n}{Font = {Latin Modern Sans Demi Cond/B}},
  FontFace = {sbc}{it}{Font = {Latin Modern Sans Demi Cond/BI}},
  FontFace = {sbc}{sl}{Font = {Latin Modern Sans Demi Cond/BI}},
}

The substitution was missing a `\@@_combo_sc_shape:n`
This would mean that a `bx/scit` would be substituted to `b/it` rather
than to `b/scit`.
@DamienRobert
Copy link
Author

I just pushed a new version that also set up a bx/scsl -> bx/scit substitution, and correct the
bx/scit -> b/scit substitution (previously it would substitute to b/it instead).

@wspr
Copy link
Collaborator

wspr commented Jun 18, 2020

Thanks! Code changes looks great so far. I can't look into detail right now but will merge once I've wrapped my head around a test file. Before I take a look, does it make sense to you that \fontshape{slsc}\selectfont doesn't work but `\textsl{\textsc{...}}`` does?

@DamienRobert
Copy link
Author

DamienRobert commented Jun 18, 2020

It depends on the LaTeX version.

From version 2020-02, LaTeX has a new fontaxe like feature, where we can combine several shapes and setup fallbacks. For your example, the \DeclareFontShapeChangeRule {sl}{sc} {scsl} {scit} applies, we are in sl mode and we request an sc, so the new mode should be scsl, with a fallback to scit if scsl does not exist.

When we directly ask for a scsl shape, since there is no \DeclareFontShapeChangeRule for it, the standard nfss fallback of 'n' kicks in.

This could be corrected directly at the latex level by adding suitable DeclareFontShapeChangeRule {n}{scsl} {scsl} {scit} DeclareFontShapeChangeRule {it}{scsl} {scsl} {scit}... rules. By the way it is a bit painful that the current LaTeX api does not provide a convenient way to ask for scit as a fallback for scsl whatever the current shape we are in now.

In versions prior to 2020-02-02, a \textsl{\textsc{...}} would set up a sc shape, to get a scsl shape we would need to ask for it directly. (Fontspec patches \textsc so that it asks for a scsl shape first, but since it does not exists it fallsback to sc and we are back to the standard LaTeX situation.)

Note that there is a similar fallback to 'b' for the 'bx' series:

\DeclareFontSeriesChangeRule {m}{bx} {bx} {b} 
\DeclareFontSeriesChangeRule {b}{bx} {bx}  {b} %<-----
\DeclareFontSeriesChangeRule {c}{bx} {bx}  {b}  %<-----
\DeclareFontSeriesChangeRule {ec}{bx} {bx}  {b}  %<-----
\DeclareFontSeriesChangeRule {sc}{bx} {bx}  {b}  %<-----
\DeclareFontSeriesChangeRule {l}{bx} {bx}  {b} %<-----

And a it fallback for the sl shape:

\DeclareFontShapeChangeRule {n}{sl}  {sl}  {it}
\DeclareFontShapeChangeRule {it}{sl}  {sl}    {it}

@DamienRobert
Copy link
Author

DamienRobert commented Jun 18, 2020

Now it gets tricky. With the current way fontspec does things, it processes custom FontFace first before processing the m/n, b/n, m/it, m/sl, b/it, b/sl faces (in this order, cf \@@_set_faces).

When fontspec set up a face, it also set up the sc shape, the sl shape if the shape is it, and the bx weight if the weight is b. This is not a problem for sl since the sl calls are after the it calls, but this is a problem for bx.

For instance in my LatinModernRoman fontspec file, I want

  BoldFont = {* Demi/B},
  BoldItalicFont = {* Demi/BI},
  FontFace = {bx}{n}{Font = {*/B}},
  FontFace = {bx}{it}{Font = {*/BI}},

because that is also what tulmr.fd does.

But since the FontFace is processed first, the automatic bx -> b substitution set up by fontspec when it process the bold font erases my custom bx FontFace.

Indeed, patching DeclareFontShape to add logging, I get:

LaTeX Info: DeclareFontShape: TU/LatinModernRoman(0)/bx/n=<->"LatinModernRoman/B
:mode=node;script=latn;language=dflt;+tlig;" () on input line 81.
LaTeX Info: DeclareFontShape: TU/LatinModernRoman(0)/bx/n=<->ssub*LatinModernRom
an(0)/b/n () on input line 81.

There are two solutions:

  • either use a different version of \@@_DeclareFontShape in \@@_declare_shapes_bx that checks first if the bx weight exists (by testing if the csname #1/#2/#3/#4 exists) and in this case do nothing.
  • or (for LaTeX version 2020-02) use the existing bx -> b and sl -> it fallbacks and remove
    \@@_declare_shape_slanted and \@@_declare_shapes_bx.

@DamienRobert
Copy link
Author

DamienRobert commented Jun 18, 2020

Now here are a few more tricky situations:

  • because of the sl -> it substitution set up by fontspec, if a user wants to setup both a
    eg l/sl FontFace and a l/it FontFace, then the order mater, the l/sl FontFace has to be put afterwards so that it does not get overwritten by the l/it one.
    Once again this could be solved by checking first for an existing sl shape before doing the substitution, or by relying on the new LaTeX fallback.
  • \@@_merge_shape:n is somewhat redundant with the new LaTeX version.
    Here is what really happens with \textsl{\textsc{...}}: first \@@_merge_shape:n kicks in, combine the sl shape with the existing sc shape and checks if the scsl shape exists. Since it does not it fallsback to \fontshape{sl} where the new LaTeX shape substitution now kicks in.
  • For \sishape, since it calls \fontshape{\scitdefault}, ie it directly sets  scit, the existing scit -> scsl fallback does not kick in (LaTeX set up both way fallbacks scit <-> scsl). This could be solved by calling \fontshape{\scdefault}\fontshape{\itdefault} instead (or better on the LaTeX side by extending \DeclareFontShapeChangeRule to have a global scit -> scsl fallback).

If a slanted font is not provided, fontspec setup automatic sl -> it
substitutions in \@@_declare_shape_slanted:nn. But if a small caps
font was provided, an scit shape is defined but there is no
corresponding scsl -> scit substitution.

Fix this by adapting the code of \@@_declare_shapes_smcaps:nn into
\@@_declare_shape_slanted:nn.

Use a similar fix in \@@_declare_shapes_bx:nn for the bx/scsl -> bx/scit
substitution.
@DamienRobert
Copy link
Author

(The new push simply correct the commit message, I was using itsc instead of scit...)

@DamienRobert
Copy link
Author

DamienRobert commented Jun 18, 2020

A few last thoughts on this subject of bx->b substitution (this is unrelated to the pr itself, sorry I probably should have made a separate bug report):

  • relying on the new LaTeX fallback instead has some odd corner cases too. For instance if only bx/m and bx/it are defined, then \fontseries{bx}\fontshape{sc} will give bx/m but \fontshape{sc}\fontseries{bx} will give b/sc. On the other hand it is not clear which would be best anyway.
  • puting the FontFace definitions after the setup of the normal font, italic font and so on, rather than before like it is currently the case, so that bx fontfaces are not overwritten is not quite possible because of the automatic sc handling. For instance setting up an m/ui shape (like in lmr) would also set up the m/sc shape and we would not want to overwrite the normal m/sc font.
  • For the automatic handling of the sc font, we cannot rely on the new latex behavior anyway because it is not simply a substitution. But this mean that a user setting up a sb/sc fontshape before a sb/n fontshape will have it overwritten. So I wonder if checking first if the sc shape exists before overwriting it could help. The problem from this approach is that \addfontfeatures rely on being able to overwrite existing shape settings...

So unfortunately I don't see any easy solution to solve all corner cases. What I currently do is patch fontspec to remove the sl->it and bx->b substitutions and rely instead on the LaTeX 2020-02 fallbacks, patch LaTeX to add globals scsl -> scit and bx->b substitutions (rather than only the specific existing ones), and be careful in my fontface declarations order for small caps fonts...

@wspr
Copy link
Collaborator

wspr commented Jun 18, 2020 via email

@DamienRobert
Copy link
Author

DamienRobert commented Jun 18, 2020

Ok, so if we are allowed to assume LaTeX 2020, then my feeling is that the best would simply to remove \@@_declare_shape_{slanted,bx} and let the new LaTeX's fallback work their substitutions instead.

In this case this pull-request that correct some bugs for both these functions is no longer needed ;)

I am happy to provide another pull request that does this if you agree. As I argued (too lengthy) above, this is not the perfect solution but imho the least worse one. Maybe you have better ideas than me!

Anyway there is no hurry, meanwhile I am happy with simply patching fontspec in fontspec.cfg :)

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