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

Use System fonts for sphinx documentation #2084

Merged
merged 2 commits into from
Jan 4, 2023

Conversation

dgovil
Copy link
Collaborator

@dgovil dgovil commented Nov 6, 2022

Description of Change(s)

This PR changes the sphinx font choices to use the system fonts where possible.

This used to not be supported well, but has recently become supported well enough that various frameworks and sites like Bootstrap have adopted it.

For the most part, this lets the user+OS vendor have more control over the legibility of the text, allowing for better variable weights and respects any legibility choices the user may have made.

If those are not specified, or supported, they fall back to the previous font choices that Read the Docs used.

Fixes Issue(s)

This fixes a few minor issues I was seeing:

  • Certain font sizes and weights weren't rendering differently enough than other fonts. e.g the H3 and H4 styling had very similar visual weights due to the font being picked up.

  • The USD header image text was rendering as serif for me, even though the CSS expected san-serif. This fixes that

  • Missing semicolon after calc in .usd-title-image-outer . I'm not sure what was meant to be calc'd here...but this at least fixes the syntax error.

  • [X ] I have verified that all unit tests pass with the proposed changes
  • [X ] I have submitted a signed Contributor License Agreement

@sunyab
Copy link
Contributor

sunyab commented Nov 9, 2022

Filed as internal issue #USD-7749

@dsyu-pixar
Copy link

Thanks @dgovil !

To verify this change, could you point us to some specific pages/sections of the docs that clearly demonstrate the improvements from the updated fonts? We were having some trouble verifying things like the H3/H4 differences on platforms like Windows.

Also, we noticed you've added some Apple fonts like SFMono that have some use-license restrictions, however we believe this should not be an issue in this case as we're not actually embedding the fonts in anything, just adding them to the CSS font-stack.

@dgovil
Copy link
Collaborator Author

dgovil commented Dec 8, 2022

Hey @dsyu-pixar , apologies I should have included some examples and the change might not be very noticeable on different platforms. Unfortunately I only have a Mac available, so I can't get a preview for Windows.

I forgot to pull the H4 change back to this PR (it was in https://github.com/PixarAnimationStudios/USD/pull/2086/files#diff-77095d4c61250f991affa4f9498f062e6f2786971e23639569179c082a522740R18) that might make it more obvious.

h4 {
    font-weight: 600;
}

Some screenshots:

Here's the default prior to this PR

image

Here's the difference on Mac with just the font change

image

Here's that plus a change to setting the font-weight for H4 to 600
image

and just looking at it now, I think H4 at 650 looks best

image

I found the default prior to this PR a little bit hard to differentiate between H3 and H4 (because the default resolving font was very similar). Looking at the comparisons now, I think setting H4 to 650 looks best to me because it sits nicely between Bold (600) and H3 (700). I've updated the PR with that setting.

The difference is subtle, but it moves the thickness of the strokes in slightly. (This video is really interesting if anyone's into font nerdery)

With regards to specifying the font names and copyright, since there's no distribution , it should be fine. It would just pick up whatever the browser can pick up from the system and isn't being used as part of branding. This would be similar to the other fonts that the ReadTheDocs theme was setting out of the box, that fall in the same category.

@dsyu-pixar
Copy link

Thanks for the update and examples @dgovil! We've merged this change into our dev branch for the next release, and this PR should get automatically closed when we push the release changes to github.

@pixar-oss pixar-oss merged commit 492b803 into PixarAnimationStudios:dev Jan 4, 2023
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.

4 participants