Skip to content

Commit

Permalink
Bug 1856035 - Merge implementations of CreateCTFontFromCGFontWithVari…
Browse files Browse the repository at this point in the history
…ations from gfxMacFont.cpp and 2d/ScaledFontMac.cpp (no change in behavior). r=gfx-reviewers,lsalzman, a=dmeehan

Differential Revision: https://phabricator.services.mozilla.com/D191123
  • Loading branch information
jfkthame committed Oct 16, 2023
1 parent 1dca8e0 commit c705f67
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 137 deletions.
59 changes: 18 additions & 41 deletions gfx/2d/ScaledFontMac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,14 @@ class AutoRelease final {
}
}

void operator=(T aObject) {
if (mObject) {
CFRelease(mObject);
AutoRelease<T>& operator=(const T& aObject) {
if (aObject != mObject) {
if (mObject) {
CFRelease(mObject);
}
mObject = aObject;
}
mObject = aObject;
return *this;
}

operator T() { return mObject; }
Expand All @@ -67,50 +70,24 @@ class AutoRelease final {

// Helper to create a CTFont from a CGFont, copying any variations that were
// set on the original CGFont.
static CTFontRef CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont,
CGFloat aSize,
bool aInstalledFont) {
// Avoid calling potentially buggy variation APIs on pre-Sierra macOS
// versions (see bug 1331683).
//
// And on HighSierra, CTFontCreateWithGraphicsFont properly carries over
// variation settings from the CGFont to CTFont, so we don't need to do
// the extra work here -- and this seems to avoid Core Text crashiness
// seen in bug 1454094.
//
// However, for installed fonts it seems we DO need to copy the variations
// explicitly even on 10.13, otherwise fonts fail to render (as in bug
// 1455494) when non-default values are used. Fortunately, the crash
// mentioned above occurs with data fonts, not (AFAICT) with system-
// installed fonts.
//
// So we only need to do this "the hard way" on Sierra, and for installed
// fonts on HighSierra+; otherwise, just let the standard CTFont function
// do its thing.
//
// NOTE in case this ever needs further adjustment: there is similar logic
// in four places in the tree (sadly):
// CreateCTFontFromCGFontWithVariations in gfxMacFont.cpp
// CreateCTFontFromCGFontWithVariations in ScaledFontMac.cpp
// CreateCTFontFromCGFontWithVariations in cairo-quartz-font.c
// ctfont_create_exact_copy in SkFontHost_mac.cpp
CTFontRef CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont, CGFloat aSize,
bool aInstalledFont,
CTFontDescriptorRef aFontDesc) {
CTFontRef ctFont;
if (nsCocoaFeatures::OnSierraExactly() ||
(aInstalledFont && nsCocoaFeatures::OnHighSierraOrLater())) {
CFDictionaryRef vars = CGFontCopyVariations(aCGFont);
if (aInstalledFont) {
AutoRelease<CFDictionaryRef> vars(CGFontCopyVariations(aCGFont));
if (vars) {
CFDictionaryRef varAttr = CFDictionaryCreate(
AutoRelease<CFDictionaryRef> varAttr(CFDictionaryCreate(
nullptr, (const void**)&kCTFontVariationAttribute,
(const void**)&vars, 1, &kCFTypeDictionaryKeyCallBacks,
&kCFTypeDictionaryValueCallBacks);
CFRelease(vars);
&kCFTypeDictionaryValueCallBacks));

CTFontDescriptorRef varDesc =
CTFontDescriptorCreateWithAttributes(varAttr);
CFRelease(varAttr);
AutoRelease<CTFontDescriptorRef> varDesc(
aFontDesc
? ::CTFontDescriptorCreateCopyWithAttributes(aFontDesc, varAttr)
: ::CTFontDescriptorCreateWithAttributes(varAttr));

ctFont = CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, varDesc);
CFRelease(varDesc);
} else {
ctFont = CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, nullptr);
}
Expand Down
48 changes: 24 additions & 24 deletions gfx/2d/ScaledFontMac.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,28 @@
namespace mozilla {
namespace gfx {

// Utility to create a CTFont from a CGFont, copying any variations that were
// set on the original CGFont, and applying additional attributes from aDesc
// (which may be NULL).
// Exposed here because it is also used by gfxMacFont and gfxCoreTextShaper.
CTFontRef CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont, CGFloat aSize,
bool aInstalledFont,
CTFontDescriptorRef aFontDesc = nullptr);

class UnscaledFontMac;

class ScaledFontMac : public ScaledFontBase {
public:
MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(ScaledFontMac, override)
ScaledFontMac(
CGFontRef aFont, const RefPtr<UnscaledFont>& aUnscaledFont, Float aSize,
bool aOwnsFont = false,
const DeviceColor& aFontSmoothingBackgroundColor = DeviceColor(),
bool aUseFontSmoothing = true, bool aApplySyntheticBold = false,
bool aHasColorGlyphs = false);
ScaledFontMac(
CTFontRef aFont, const RefPtr<UnscaledFont>& aUnscaledFont,
const DeviceColor& aFontSmoothingBackgroundColor = DeviceColor(),
bool aUseFontSmoothing = true, bool aApplySyntheticBold = false,
bool aHasColorGlyphs = false);
ScaledFontMac(CGFontRef aFont, const RefPtr<UnscaledFont>& aUnscaledFont, Float aSize,
bool aOwnsFont = false,
const DeviceColor& aFontSmoothingBackgroundColor = DeviceColor(),
bool aUseFontSmoothing = true, bool aApplySyntheticBold = false,
bool aHasColorGlyphs = false);
ScaledFontMac(CTFontRef aFont, const RefPtr<UnscaledFont>& aUnscaledFont,
const DeviceColor& aFontSmoothingBackgroundColor = DeviceColor(),
bool aUseFontSmoothing = true, bool aApplySyntheticBold = false,
bool aHasColorGlyphs = false);
~ScaledFontMac();

FontType GetType() const override { return FontType::MAC; }
Expand All @@ -47,31 +53,26 @@ class ScaledFontMac : public ScaledFontBase {

bool GetFontInstanceData(FontInstanceDataOutput aCb, void* aBaton) override;

bool GetWRFontInstanceOptions(
Maybe<wr::FontInstanceOptions>* aOutOptions,
Maybe<wr::FontInstancePlatformOptions>* aOutPlatformOptions,
std::vector<FontVariation>* aOutVariations) override;
bool GetWRFontInstanceOptions(Maybe<wr::FontInstanceOptions>* aOutOptions,
Maybe<wr::FontInstancePlatformOptions>* aOutPlatformOptions,
std::vector<FontVariation>* aOutVariations) override;

bool CanSerialize() override { return true; }

bool MayUseBitmaps() override { return mHasColorGlyphs; }

bool UseSubpixelPosition() const override { return true; }

DeviceColor FontSmoothingBackgroundColor() {
return mFontSmoothingBackgroundColor;
}
DeviceColor FontSmoothingBackgroundColor() { return mFontSmoothingBackgroundColor; }

cairo_font_face_t* CreateCairoFontFace(
cairo_font_options_t* aFontOptions) override;
cairo_font_face_t* CreateCairoFontFace(cairo_font_options_t* aFontOptions) override;

private:
friend class DrawTargetSkia;
friend class UnscaledFontMac;

CGFontRef mFont;
CTFontRef
mCTFont; // only created if CTFontDrawGlyphs is available, otherwise null
CTFontRef mCTFont; // only created if CTFontDrawGlyphs is available, otherwise null

DeviceColor mFontSmoothingBackgroundColor;
bool mUseFontSmoothing;
Expand All @@ -80,8 +81,7 @@ class ScaledFontMac : public ScaledFontBase {

struct InstanceData {
explicit InstanceData(ScaledFontMac* aScaledFont)
: mFontSmoothingBackgroundColor(
aScaledFont->mFontSmoothingBackgroundColor),
: mFontSmoothingBackgroundColor(aScaledFont->mFontSmoothingBackgroundColor),
mUseFontSmoothing(aScaledFont->mUseFontSmoothing),
mApplySyntheticBold(aScaledFont->mApplySyntheticBold),
mHasColorGlyphs(aScaledFont->mHasColorGlyphs) {}
Expand Down
2 changes: 2 additions & 0 deletions gfx/2d/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ EXPORTS.mozilla.gfx += ["ssse3-scaler.h"]
if CONFIG["MOZ_WIDGET_TOOLKIT"] in ("cocoa", "uikit"):
EXPORTS.mozilla.gfx += [
"MacIOSurface.h",
"ScaledFontBase.h",
"ScaledFontMac.h",
"UnscaledFontMac.h",
]
UNIFIED_SOURCES += [
Expand Down
6 changes: 4 additions & 2 deletions gfx/thebes/gfxCoreTextShaper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
#include "gfxFontUtils.h"
#include "gfxTextRun.h"
#include "mozilla/gfx/2D.h"
#include "mozilla/gfx/ScaledFontMac.h"
#include "mozilla/UniquePtrExtensions.h"

#include <algorithm>

#include <dlfcn.h>

using namespace mozilla;
using namespace mozilla::gfx;

// standard font descriptors that we construct the first time they're needed
CTFontDescriptorRef gfxCoreTextShaper::sFeaturesDescriptor[kMaxFontInstances];
Expand Down Expand Up @@ -634,8 +636,8 @@ CTFontRef gfxCoreTextShaper::CreateCTFontWithFeatures(
const gfxFontEntry* fe = mFont->GetFontEntry();
bool isInstalledFont = !fe->IsUserFont() || fe->IsLocalUserFont();
CGFontRef cgFont = static_cast<gfxMacFont*>(mFont)->GetCGFontRef();
return gfxMacFont::CreateCTFontFromCGFontWithVariations(
cgFont, aSize, isInstalledFont, aDescriptor);
return CreateCTFontFromCGFontWithVariations(cgFont, aSize, isInstalledFont,
aDescriptor);
}

void gfxCoreTextShaper::Shutdown() // [static]
Expand Down
57 changes: 1 addition & 56 deletions gfx/thebes/gfxMacFont.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "mozilla/MemoryReporting.h"
#include "mozilla/Sprintf.h"
#include "mozilla/StaticPrefs_gfx.h"
#include "mozilla/gfx/ScaledFontMac.h"

#include "gfxCoreTextShaper.h"
#include <algorithm>
Expand Down Expand Up @@ -435,62 +436,6 @@ gfxFloat gfxMacFont::GetCharWidth(CFDataRef aCmap, char16_t aUniChar,
return 0;
}

/* static */
CTFontRef gfxMacFont::CreateCTFontFromCGFontWithVariations(
CGFontRef aCGFont, CGFloat aSize, bool aInstalledFont,
CTFontDescriptorRef aFontDesc) {
// Avoid calling potentially buggy variation APIs on pre-Sierra macOS
// versions (see bug 1331683).
//
// And on HighSierra, CTFontCreateWithGraphicsFont properly carries over
// variation settings from the CGFont to CTFont, so we don't need to do
// the extra work here -- and this seems to avoid Core Text crashiness
// seen in bug 1454094.
//
// However, for installed fonts it seems we DO need to copy the variations
// explicitly even on 10.13, otherwise fonts fail to render (as in bug
// 1455494) when non-default values are used. Fortunately, the crash
// mentioned above occurs with data fonts, not (AFAICT) with system-
// installed fonts.
//
// So we only need to do this "the hard way" on Sierra, and on HighSierra
// for system-installed fonts; in other cases just let the standard CTFont
// function do its thing.
//
// NOTE in case this ever needs further adjustment: there is similar logic
// in four places in the tree (sadly):
// CreateCTFontFromCGFontWithVariations in gfxMacFont.cpp
// CreateCTFontFromCGFontWithVariations in ScaledFontMac.cpp
// CreateCTFontFromCGFontWithVariations in cairo-quartz-font.c
// ctfont_create_exact_copy in SkFontHost_mac.cpp

CTFontRef ctFont;
if (nsCocoaFeatures::OnSierraExactly() ||
(aInstalledFont && nsCocoaFeatures::OnHighSierraOrLater())) {
AutoCFRelease<CFDictionaryRef> variations = ::CGFontCopyVariations(aCGFont);
if (variations) {
AutoCFRelease<CFDictionaryRef> varAttr = ::CFDictionaryCreate(
nullptr, (const void**)&kCTFontVariationAttribute,
(const void**)&variations, 1, &kCFTypeDictionaryKeyCallBacks,
&kCFTypeDictionaryValueCallBacks);

AutoCFRelease<CTFontDescriptorRef> varDesc =
aFontDesc
? ::CTFontDescriptorCreateCopyWithAttributes(aFontDesc, varAttr)
: ::CTFontDescriptorCreateWithAttributes(varAttr);

ctFont = ::CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, varDesc);
} else {
ctFont =
::CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, aFontDesc);
}
} else {
ctFont = ::CTFontCreateWithGraphicsFont(aCGFont, aSize, nullptr, aFontDesc);
}

return ctFont;
}

int32_t gfxMacFont::GetGlyphWidth(uint16_t aGID) {
if (mVariationFont) {
// Avoid a potential Core Text crash (bug 1450209) by using
Expand Down
25 changes: 11 additions & 14 deletions gfx/thebes/gfxMacFont.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,24 @@ class MacOSFontEntry;

class gfxMacFont final : public gfxFont {
public:
gfxMacFont(const RefPtr<mozilla::gfx::UnscaledFontMac>& aUnscaledFont, MacOSFontEntry* aFontEntry,
const gfxFontStyle* aFontStyle);
gfxMacFont(const RefPtr<mozilla::gfx::UnscaledFontMac>& aUnscaledFont,
MacOSFontEntry* aFontEntry, const gfxFontStyle* aFontStyle);

CGFontRef GetCGFontRef() const { return mCGFont; }

/* override Measure to add padding for antialiasing */
RunMetrics Measure(const gfxTextRun* aTextRun, uint32_t aStart, uint32_t aEnd,
BoundingBoxType aBoundingBoxType, DrawTarget* aDrawTargetForTightBoundingBox,
Spacing* aSpacing, mozilla::gfx::ShapedTextFlags aOrientation) override;
BoundingBoxType aBoundingBoxType,
DrawTarget* aDrawTargetForTightBoundingBox,
Spacing* aSpacing,
mozilla::gfx::ShapedTextFlags aOrientation) override;

// We need to provide hinted (non-linear) glyph widths if using a font
// with embedded color bitmaps (Apple Color Emoji), as Core Text renders
// the glyphs with non-linear scaling at small pixel sizes.
bool ProvidesGlyphWidths() const override {
return mVariationFont || mFontEntry->HasFontTable(TRUETYPE_TAG('s', 'b', 'i', 'x'));
return mVariationFont ||
mFontEntry->HasFontTable(TRUETYPE_TAG('s', 'b', 'i', 'x'));
}

int32_t GetGlyphWidth(uint16_t aGID) override;
Expand All @@ -51,21 +54,15 @@ class gfxMacFont final : public gfxFont {

bool UseNativeColrFontSupport() const override;

// Helper to create a CTFont from a CGFont, with optional font descriptor
// (for features), and copying any variations that were set on the CGFont.
// This is public so that gfxCoreTextShaper can also use it.
static CTFontRef CreateCTFontFromCGFontWithVariations(CGFontRef aCGFont, CGFloat aSize,
bool aInstalledFont,
CTFontDescriptorRef aFontDesc = nullptr);

protected:
~gfxMacFont() override;

const Metrics& GetHorizontalMetrics() const override { return mMetrics; }

// override to prefer CoreText shaping with fonts that depend on AAT
bool ShapeText(DrawTarget* aDrawTarget, const char16_t* aText, uint32_t aOffset, uint32_t aLength,
Script aScript, nsAtom* aLanguage, bool aVertical, RoundingFlags aRounding,
bool ShapeText(DrawTarget* aDrawTarget, const char16_t* aText,
uint32_t aOffset, uint32_t aLength, Script aScript,
nsAtom* aLanguage, bool aVertical, RoundingFlags aRounding,
gfxShapedText* aShapedText) override;

void InitMetrics();
Expand Down

0 comments on commit c705f67

Please sign in to comment.