Skip to content

Conversation

@bcamper
Copy link
Member

@bcamper bcamper commented Jun 7, 2020

Variable fonts allow multiple variants of a font to be stored in a single file.

Supporting these has several benefits:

  • Streamlines the Tangram font definition, e.g. for example instead of
      Open Sans:
          - weight: 200
            url: fonts/Open Sans.woff2
          - weight: 400
            url: fonts/Open Sans.woff2
          - weight: 600
            url: fonts/Open Sans.woff2
          - weight: 800
            url: fonts/Open Sans.woff2
    
    we can do
      Open Sans:
          url: fonts/Open Sans.woff2
          weight: 200 800
    
  • Reduces network requests, requesting one file instead of many.
  • Introducing new cartographic flexibility, as some variable fonts can render at intermediate weights along the spectrum, not just at predefined weights.

In CSS, the syntax for variable font weight looks like: font-weight: 125 950;.
Tangram font syntax is similarly extended to support this as weight: 125 950 for a given font within the fonts block.

Note that while the variable font spec supports variation for other properties including font stretch and even custom, per-typeface-defined properties, Tangram relies on Canvas text rendering for labels, which unfortunately does not support these properties (font stretch cannot be set even for non-variable fonts).

This PR also enables JS function-based font weight, e.g. an example mapping font weight to building heights:

weight: |
  function() {
    return (feature.height||0) * 2 + 400;
}

tangram-1591501269395

@bcamper bcamper added this to the v0.20.2 milestone Jun 7, 2020
@bcamper
Copy link
Member Author

bcamper commented Jun 7, 2020

@bdon
Copy link
Contributor

bdon commented Jun 8, 2020

I'm trying to think of a concrete use case for why I would specify a desired range of my variable font other than the entire valid range (1-1000) - it's not validated against when setting explicit weights in font draw styles.

One case might be where a user wants to swap out a font in the YAML without changing anything else, but also wants to clamp the allowed weights to an acceptably legible range based on the design of that specific font...

LGTM

@bcamper
Copy link
Member Author

bcamper commented Jun 8, 2020

@bdon that is a good question, and I was about to say maybe we should just drop it entirely and assume the full range. However... I would need to check again, but I think that the font may not report as loaded at all unless a valid weight is provided, e.g. in this syntax, if the first provided value in the range exists in the font. I'm not 100% sure, and the behavior may vary across browsers as well. If we want to add support for this in Tangram ES, we also may need explicit values as it will be using a different font mechanism.

We need to care about the font being loaded and with correct settings because if we render too soon then the first tile may render with the wrong font, but FontFaceObserver also has a 3 second timeout for font loading failures, so it is not a "fast failure" if the provided values are wrong, and instead causes a long delay for map load/display.

@bdon
Copy link
Contributor

bdon commented Jun 10, 2020

I did some testing across browsers (safari, firefox, chrome) with the 1-1000 range specified. It works fine so far, but I did discover that setting style: italic may not work for variable fonts in Chrome.

Chromium ticket: Issue 1064756: font-style: italic doesn't activate the ital axis of variable fonts
Font: Inter

I don't think this a blocker for merging this though.

@bcamper
Copy link
Member Author

bcamper commented Jun 10, 2020

@bdon that's interesting to know about the italic axis in Chrome. I didn't explicitly point out italic, but it's covered by this comment from the description:

Note that while the variable font spec supports variation for other properties including font stretch and even custom, per-typeface-defined properties, Tangram relies on Canvas text rendering for labels, which unfortunately does not support these properties (font stretch cannot be set even for non-variable fonts).

I discovered this while trying to support additional axes beyond weight, but as far as I can tell, the short-hand CSS syntax -- which is what Canvas uses for text rendering, exposed as the font property on the canvas context -- doesn't support using numbers for these properties, it appears to confuse the parsing. I thought I found something explicitly stating this somewhere in one of the specs, but I'm not recalling where at the moment...

@bcamper bcamper modified the milestones: v0.20.2, v0.21.0 Jul 5, 2020
@bdon
Copy link
Contributor

bdon commented Jul 10, 2020

I did some more digging into the italics issue, and it's only Chrome, when using the "100 900" weight specification, that italics stop working. It works fine on Safari and Firefox; it also works fine on Chrome when using the multiple single-weight definitions workaround e.g.

        - weight: 100
          url: https://example.com/fonts/woff2/Inter.var.woff2
        - weight: 200
          url: https://example.com/fonts/woff2/Inter.var.woff2

@bdon
Copy link
Contributor

bdon commented Jul 10, 2020

A workaround I discovered: If you only ever use style:italic with font weight 200, you can have this set of fonts:

        - weight: 100 900
          url: https://example.com/fonts/woff2/Inter.var.woff2
        - weight: 200
          url: https://example.com/fonts/woff2/Inter.var.woff2

And it will work properly in chrome. So any weight that might be italic needs to be explicitly loaded by FontFace.

@bcamper
Copy link
Member Author

bcamper commented Jul 13, 2020

@bdon ahhh OK thank you for the further info, I had misunderstood, I only thought that variable italics didn't work, not that all italics on variable fonts are broken (in Chrome)?? Do you still think this makes sense to merge and perhaps just document the Chrome bug?

I guess the alternative would be to have Tangram be smart enough to register additional fonts w/FontFace on the fly... right now it relies on parsing all the font definitions once upfront. Some refactoring would be needed to change that, though it would be generally more flexible. Maybe we just create an issue for that work, and proceed with merging this.

@bdon
Copy link
Contributor

bdon commented Jul 13, 2020

I think it does make sense to merge, because I don't see anything wrong with the implementation as-is. (BTW, I tried changing it so that it observes for the '900' weight load event instead of the '100' in case this was some implementation detail about the order in which italics are loaded, but that didn't fix anything)

The chrome issue has the workaround I described above, so maybe best to document that.

@bcamper
Copy link
Member Author

bcamper commented Jul 14, 2020

Thanks. Going to merge this one then and give some thought to adapting font loading to address this, which would bring other performance benefits (deferred loading).

@bcamper bcamper merged commit 9be5386 into master Jul 14, 2020
@bcamper bcamper deleted the variable-font-weight branch July 14, 2020 15:52
@bcamper
Copy link
Member Author

bcamper commented Jul 24, 2020

Note, fixed a regression in 28afd16

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.

3 participants