1321031 - Support font variations in the Mac font backend

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

The new OpenType Variations technology is already supported (at least for TrueType-flavored fonts) in CoreGraphics/CoreText, as it is the same technology that Apple originally implemented in QuickDraw GX and then AAT fonts.

Therefore, we should be able to render variations specified via CSS font-variation-settings (bug 1321022) on macOS by hooking this up to the Core Text variations support.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: pt 1 - Implement support for variations in gfxMacFont, passing settings through to Core Text

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: pt 2 - Preserve variations settings in ScaledFontMac when creating a new CTFont

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: pt 3 - Preserve variations settings when creating a new CTFont in SkFontHost_mac

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: pt 4 - Preserve variations settings if gfxCoreTextShaper needs to instantiate a new CTFont

Comment 5

[Jonathan Kew [:jfkthame]]

Try build with these patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=730390619f2d8043506ceb33ea47f5396ca7d42f. With this, examples like [1] or [2] should work on a current macOS system.

[1] https://jsfiddle.net/k6pkvj66/1/
[2] https://people-mozilla.org/~jkew/tests/skia.html

Comment 6

[Jonathan Kew [:jfkthame]]

Testing with the try build from comment 5 reveals that variations are not handled properly when printing. We need to preserve variation settings in a couple more places... within cairo-quartz-font.c (fixes the printed output when e10s is disabled), and in a Moz2d recording (required for parent-process printing when e10s is enabled).

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: pt 5 - Preserve any variation settings present in the CGFont when the cairo quartz backend creates a new CTFont

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: pt 6 - Include variation settings in moz2d recording stream when using the native Mac font backend

Comment 9

[Jonathan Kew [:jfkthame]]

New try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9aff609184aa3b666dce5e4cbb2d98306629277c

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: pt 6 - Include variation settings in moz2d recording stream when using the native Mac font backend

Updated patch to fix build error on Windows. (No actual implementation of variations in the Windows font backends yet.)

Comment 11

[Bas Schouten (:bas.schouten)]

Comment on attachment 8817895 [details] [diff] [review]
pt 6 - Include variation settings in moz2d recording stream when using the native Mac font backend

Review of attachment 8817895 [details] [diff] [review]:
-----------------------------------------------------------------

It would be good if jrmuizel reviewed the CoreFont parts of this.

::: gfx/2d/NativeFontResourceMac.cpp
@@ +177,5 @@
>    }
>  
> +  if (aVariationCount > 0) {
> +    MOZ_ASSERT(aVariations);
> +    CFDictionaryRef varDict =

Why not use AutoRelease here?

::: gfx/2d/RecordedEvent.cpp
@@ +1512,5 @@
>  
>  RecordedFontData::~RecordedFontData()
>  {
>    delete[] mData;
> +  delete[] mVariations;

While we're in this code let's make sure these are initialized to nullptr so we don't risk anything when getting to the destructor without SetFontData being called.

@@ +1565,5 @@
> +  mFontDetails.variationCount = aVariationCount;
> +  if (aVariationCount > 0) {
> +    mVariations = new ScaledFont::VariationSetting[aVariationCount];
> +    memcpy(mVariations, aVariations, varDataSize);
> +  } else {

Then afaict you will be able to remove these else clauses.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: pt 6 - Include variation settings in moz2d recording stream when using the native Mac font backend

Updated following Bas's comments (above); tagging Jeff for an additional review, as suggested.

Comment 13

[Jonathan Kew [:jfkthame]]

jrmuizel: r? ping... thx!

Comment 14

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4ae57d5358f14e6df0aa848eed3191a900f9adc
Bug 1321031 pt 1 - Implement support for variations in gfxMacFont, passing settings through to Core Text. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/c551913ae89236623ee320537d6f204e3eeec7fb
Bug 1321031 pt 2 - Preserve variations settings in ScaledFontMac when creating a new CTFont. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc58e479eb58556445e95d194e6bf30e7585c2c0
Bug 1321031 pt 3 - Preserve variations settings when creating a new CTFont in SkFontHost_mac. r=lsalzman

https://hg.mozilla.org/integration/mozilla-inbound/rev/7cb4242dc636a9be72f992e57c6d7251c280600b
Bug 1321031 pt 4 - Preserve variations settings if gfxCoreTextShaper needs to instantiate a new CTFont. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba071046f8abffbc8207e1d8cdf184b70f757dfd
Bug 1321031 pt 5 - Preserve any variation settings present in the CGFont when the cairo quartz backend creates a new CTFont. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0be4f5390fbc8189584e8c5af18523403e8bb1a
Bug 1321031 pt 6 - Include variation settings in moz2d recording stream when using the native Mac font backend. r=bas,jrmuizel

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage.
https://treeherder.mozilla.org/logviewer.html#?job_id=66838224&repo=mozilla-inbound&lineNumber=13727

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c31063a2cd3d
Backed out 6 changesets for bustage.

Comment 17

[Jonathan Kew [:jfkthame]]

Argh, sorry... I guess that'll be due to changes from bug 1309205, which landed since last time I pushed this to try. :(

Comment 18

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4290bb5169142fe9423304458b5b1360a699f0ec
Bug 1321031 pt 1 - Implement support for variations in gfxMacFont, passing settings through to Core Text. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/829377878aad6f9ba5f790d782292a5cead54af0
Bug 1321031 pt 2 - Preserve variations settings in ScaledFontMac when creating a new CTFont. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/83046b21acd7887cf14ccde82df900e4e61d3f0e
Bug 1321031 pt 3 - Preserve variations settings when creating a new CTFont in SkFontHost_mac. r=lsalzman

https://hg.mozilla.org/integration/mozilla-inbound/rev/53365854908ea9138fe130f6aade217851272ebc
Bug 1321031 pt 4 - Preserve variations settings if gfxCoreTextShaper needs to instantiate a new CTFont. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/77f41c0e5d6fd011c7494da45607716ff7351909
Bug 1321031 pt 5 - Preserve any variation settings present in the CGFont when the cairo quartz backend creates a new CTFont. r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/273ce98bb9e831b70d9ce2b34590859ea2ce80b8
Bug 1321031 pt 6 - Include variation settings in moz2d recording stream when using the native Mac font backend. r=bas,jrmuizel

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4290bb516914
https://hg.mozilla.org/mozilla-central/rev/829377878aad
https://hg.mozilla.org/mozilla-central/rev/83046b21acd7
https://hg.mozilla.org/mozilla-central/rev/53365854908e
https://hg.mozilla.org/mozilla-central/rev/77f41c0e5d6f
https://hg.mozilla.org/mozilla-central/rev/273ce98bb9e8

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/eefd48f36d82
Follow-up to fix build bustage on the graphics branch. r=jrmuizel?