Change SkFont to match needs of textblob
Tentative roadmap:
- land this
- extend TextBlobBuilder to take SkFont for its runs (eventually removing paint option)
- change SkTextBlob to store SkFont instead of SkRunFont (not critical, but makes sense)
After the above, (or during) also work towards:
- removing callers of SkPaint set... (textsize, textscalex, typeface, etc.)
- eventually physically remove those setters/getters/fields
- rev as desired the SkFont API to clean up flags, hinting, etc.
Bug: skia:2664
Change-Id: I0e323c58aef055e26d697911b078797453cb3626
Reviewed-on: https://skia-review.googlesource.com/c/163783
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Reed <reed@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
diff --git a/include/core/SkFont.h b/include/core/SkFont.h
index 6aba230..0d3ad76 100644
--- a/include/core/SkFont.h
+++ b/include/core/SkFont.h
@@ -8,8 +8,8 @@
#ifndef SkFont_DEFINED
#define SkFont_DEFINED
-#include "SkRefCnt.h"
#include "SkScalar.h"
+#include "SkRefCnt.h"
class SkPaint;
class SkTypeface;
@@ -21,119 +21,60 @@
kGlyphID_SkTextEncoding,
};
-/*
- 1. The Hinting enum in SkPaint is gone entirely, absorbed into SkFont's flags.
-
- 2. SkPaint Flags look like this today
-
- enum Flags {
- kAntiAlias_Flag = 0x01, //!< mask to enable antialiasing
- kDither_Flag = 0x04, //!< mask to enable dithering
- kFakeBoldText_Flag = 0x20, //!< mask to enable fake-bold text
- kLinearText_Flag = 0x40, //!< mask to enable linear-text
- kSubpixelText_Flag = 0x80, //!< mask to enable subpixel text positioning
- kLCDRenderText_Flag = 0x200, //!< mask to enable subpixel glyph renderering
- kEmbeddedBitmapText_Flag = 0x400, //!< mask to enable embedded bitmap strikes
- kAutoHinting_Flag = 0x800, //!< mask to force Freetype's autohinter
- kVerticalText_Flag = 0x1000,
- };
-
- SkFont would absorb these:
-
- kFakeBoldText_Flag = 0x20, //!< mask to enable fake-bold text
- kLinearText_Flag = 0x40, //!< mask to enable linear-text
- kSubpixelText_Flag = 0x80, //!< mask to enable subpixel text positioning
- kLCDRenderText_Flag = 0x200, //!< mask to enable subpixel glyph renderering
- kEmbeddedBitmapText_Flag = 0x400, //!< mask to enable embedded bitmap strikes
- kAutoHinting_Flag = 0x800, //!< mask to force Freetype's autohinter
- kVerticalText_Flag = 0x1000,
-
- leaving these still in paint
-
- kAntiAlias_Flag = 0x01, //!< mask to enable antialiasing
- kDither_Flag = 0x04, //!< mask to enable dithering
-
- 3. Antialiasing
-
- SkFont has a mask-type: BW, AA, LCD
- SkPaint has antialias boolean
-
- What to do if the font's mask-type disagrees with the paint?
-
- */
-
-class SkFont : public SkRefCnt {
+class SkFont {
public:
enum Flags {
/**
* Use the system's automatic hinting mechanism to hint the typeface.
- * This is a last resort hinting method applied only if other hinting methods do not apply.
- * TODO: where to put auto-normal vs auto-light?
*/
- kEnableAutoHints_Flag = 1 << 0,
-
- /**
- * If the typeface contains explicit bytecodes for hinting, use them.
- * If both bytecode and auto hints are specified, attempt to use the bytecodes first;
- * if that fails (e.g. there are no codes), then attempt to autohint.
- */
- kEnableByteCodeHints_Flag = 1 << 1,
+ kForceAutoHinting_Flag = 1 << 0,
/**
* If the typeface contains explicit bitmaps for hinting, use them.
* If both bytecode and auto hints are also specified, attempt to use the bitmaps first;
* if that fails (e.g. there are no bitmaps), then attempt to bytecode or autohint.
*/
- kEmbeddedBitmaps_Flag = 1 << 2,
+ kEmbeddedBitmaps_Flag = 1 << 1,
- /**
- * Use rounded metric values (e.g. advance).
- * If either auto or bytecode hinting was used, apply those results to the metrics of the
- * glyphs as well. If no hinting was applied, the metrics will just be rounded to the
- * nearest integer.
- *
- * This applies to calls that return metrics (e.g. measureText) and to drawing the glyphs
- * (see SkCanvas drawText and drawPosText).
- */
- kUseNonlinearMetrics_Flag = 1 << 3,
+ kSubpixel_Flag = 1 << 2,
+ kLinearMetrics_Flag = 1 << 3,
kVertical_Flag = 1 << 4,
+ kEmbolden_Flag = 1 << 5,
- kEmbolden_Flag = 1 << 6,
+ kHinting_FlagShift = 6,
+ kHinting_FlagMask = 3, // 2 bits
+
+ kDEPRECATED_Antialias_Flag = 1 << 8, // want to rely on paint for this
};
- enum MaskType {
- kBW_MaskType,
- kA8_MaskType,
- kLCD_MaskType,
- };
-
- static sk_sp<SkFont> Make(sk_sp<SkTypeface>, SkScalar size, MaskType, uint32_t flags);
- static sk_sp<SkFont> Make(sk_sp<SkTypeface>, SkScalar size, SkScalar scaleX, SkScalar skewX,
- MaskType, uint32_t flags);
+ SkFont(sk_sp<SkTypeface>, SkScalar size, uint32_t flags);
+ SkFont(sk_sp<SkTypeface>, SkScalar size, SkScalar scaleX, SkScalar skewX, uint32_t flags);
/**
* Return a font with the same attributes of this font, but with the specified size.
* If size is not supported (e.g. <= 0 or non-finite) NULL will be returned.
*/
- sk_sp<SkFont> makeWithSize(SkScalar size) const;
+ SkFont makeWithSize(SkScalar size) const;
+
/**
* Return a font with the same attributes of this font, but with the flags.
*/
- sk_sp<SkFont> makeWithFlags(uint32_t newFlags) const;
+ SkFont makeWithFlags(uint32_t newFlags) const;
SkTypeface* getTypeface() const { return fTypeface.get(); }
SkScalar getSize() const { return fSize; }
SkScalar getScaleX() const { return fScaleX; }
SkScalar getSkewX() const { return fSkewX; }
uint32_t getFlags() const { return fFlags; }
- MaskType getMaskType() const { return (MaskType)fMaskType; }
- bool isVertical() const { return SkToBool(fFlags & kVertical_Flag); }
- bool isEmbolden() const { return SkToBool(fFlags & kEmbolden_Flag); }
- bool isEnableAutoHints() const { return SkToBool(fFlags & kEnableAutoHints_Flag); }
- bool isEnableByteCodeHints() const { return SkToBool(fFlags & kEnableByteCodeHints_Flag); }
- bool isUseNonLinearMetrics() const { return SkToBool(fFlags & kUseNonlinearMetrics_Flag); }
+ sk_sp<SkTypeface> refTypeface() const { return fTypeface; }
+
+ void setTypeface(sk_sp<SkTypeface>);
+ void setSize(SkScalar);
+ void setScaleX(SkScalar);
+ void setSkewX(SkScalar);
+ void setFlags(uint32_t);
int textToGlyphs(const void* text, size_t byteLength, SkTextEncoding,
SkGlyphID glyphs[], int maxGlyphCount) const;
@@ -144,21 +85,17 @@
SkScalar measureText(const void* text, size_t byteLength, SkTextEncoding) const;
- static sk_sp<SkFont> Testing_CreateFromPaint(const SkPaint&);
+ void LEGACY_applyToPaint(SkPaint*) const;
+ static SkFont LEGACY_ExtractFromPaint(const SkPaint&);
private:
- static constexpr int kAllFlags = 0xFF;
-
- SkFont(sk_sp<SkTypeface>, SkScalar size, SkScalar scaleX, SkScalar skewX, MaskType,
- uint32_t flags);
+ static constexpr unsigned kAllFlags = 0x1FF;
sk_sp<SkTypeface> fTypeface;
SkScalar fSize;
SkScalar fScaleX;
SkScalar fSkewX;
- uint16_t fFlags;
- uint8_t fMaskType;
-// uint8_t fPad;
+ uint32_t fFlags;
};
#endif
diff --git a/src/core/SkFont.cpp b/src/core/SkFont.cpp
index f9423a2..821c020 100644
--- a/src/core/SkFont.cpp
+++ b/src/core/SkFont.cpp
@@ -11,14 +11,13 @@
#include "SkTypeface.h"
#include "SkUTF.h"
-SkFont::SkFont(sk_sp<SkTypeface> face, SkScalar size, SkScalar scaleX, SkScalar skewX, MaskType mt,
+SkFont::SkFont(sk_sp<SkTypeface> face, SkScalar size, SkScalar scaleX, SkScalar skewX,
uint32_t flags)
: fTypeface(face ? std::move(face) : SkTypeface::MakeDefault())
, fSize(size)
, fScaleX(scaleX)
, fSkewX(skewX)
, fFlags(flags)
- , fMaskType(SkToU8(mt))
{
SkASSERT(size > 0);
SkASSERT(scaleX > 0);
@@ -26,33 +25,15 @@
SkASSERT(0 == (flags & ~kAllFlags));
}
-sk_sp<SkFont> SkFont::Make(sk_sp<SkTypeface> face, SkScalar size, SkScalar scaleX, SkScalar skewX,
- MaskType mt, uint32_t flags) {
- if (size <= 0 || !SkScalarIsFinite(size)) {
- return nullptr;
- }
- if (scaleX <= 0 || !SkScalarIsFinite(scaleX)) {
- return nullptr;
- }
- if (!SkScalarIsFinite(skewX)) {
- return nullptr;
- }
- flags &= kAllFlags;
- return sk_sp<SkFont>(new SkFont(std::move(face), size, scaleX, skewX, mt, flags));
+SkFont::SkFont(sk_sp<SkTypeface> face, SkScalar size, uint32_t flags)
+ : SkFont(std::move(face), size, 1, 0, flags) {}
+
+SkFont SkFont::makeWithSize(SkScalar newSize) const {
+ return {this->refTypeface(), newSize, this->getScaleX(), this->getSkewX(), this->getFlags()};
}
-sk_sp<SkFont> SkFont::Make(sk_sp<SkTypeface> face, SkScalar size, MaskType mt, uint32_t flags) {
- return SkFont::Make(std::move(face), size, 1, 0, mt, flags);
-}
-
-sk_sp<SkFont> SkFont::makeWithSize(SkScalar newSize) const {
- return SkFont::Make(sk_ref_sp(this->getTypeface()), newSize, this->getScaleX(),
- this->getSkewX(), this->getMaskType(), this->getFlags());
-}
-
-sk_sp<SkFont> SkFont::makeWithFlags(uint32_t newFlags) const {
- return SkFont::Make(sk_ref_sp(this->getTypeface()), this->getSize(), this->getScaleX(),
- this->getSkewX(), this->getMaskType(), newFlags);
+SkFont SkFont::makeWithFlags(uint32_t newFlags) const {
+ return {this->refTypeface(), this->getSize(), this->getScaleX(), this->getSkewX(), newFlags};
}
///////////////////////////////////////////////////////////////////////////////////////////////////
@@ -116,7 +97,20 @@
#include "SkPaint.h"
-sk_sp<SkFont> SkFont::Testing_CreateFromPaint(const SkPaint& paint) {
+void SkFont::LEGACY_applyToPaint(SkPaint* paint) const {
+ paint->setEmbeddedBitmapText(SkToBool(fFlags & kEmbeddedBitmaps_Flag));
+ paint->setFakeBoldText(SkToBool(fFlags & kEmbolden_Flag));
+ paint->setAutohinted(SkToBool(fFlags & kForceAutoHinting_Flag));
+ paint->setSubpixelText(SkToBool(fFlags & kSubpixel_Flag));
+ paint->setLinearText(SkToBool(fFlags & kLinearMetrics_Flag));
+
+ unsigned hinting = (fFlags >> kHinting_FlagShift) & kHinting_FlagMask;
+ paint->setHinting((SkPaint::Hinting)hinting);
+
+ paint->setAntiAlias(SkToBool(fFlags & kDEPRECATED_Antialias_Flag));
+}
+
+SkFont SkFont::LEGACY_ExtractFromPaint(const SkPaint& paint) {
uint32_t flags = 0;
if (paint.isVerticalText()) {
flags |= kVertical_Flag;
@@ -127,24 +121,23 @@
if (paint.isFakeBoldText()) {
flags |= kEmbolden_Flag;
}
-
- if (SkPaint::kFull_Hinting == paint.getHinting()) {
- flags |= kEnableByteCodeHints_Flag;
- }
if (paint.isAutohinted()) {
- flags |= kEnableAutoHints_Flag;
+ flags |= kForceAutoHinting_Flag;
}
- if (paint.isSubpixelText() || paint.isLinearText()) {
- // this is our default
- } else {
- flags |= kUseNonlinearMetrics_Flag;
+ if (paint.isSubpixelText()) {
+ flags |= kSubpixel_Flag;
+ }
+ if (paint.isLinearText()) {
+ flags |= kLinearMetrics_Flag;
}
- MaskType maskType = SkFont::kBW_MaskType;
if (paint.isAntiAlias()) {
- maskType = paint.isLCDRenderText() ? kLCD_MaskType : kA8_MaskType;
+ flags |= kDEPRECATED_Antialias_Flag;
}
- return Make(sk_ref_sp(paint.getTypeface()), paint.getTextSize(), paint.getTextScaleX(),
- paint.getTextSkewX(), maskType, flags);
+ unsigned hinting = (unsigned)paint.getHinting();
+ flags |= (hinting << kHinting_FlagShift);
+
+ return SkFont(sk_ref_sp(paint.getTypeface()), paint.getTextSize(), paint.getTextScaleX(),
+ paint.getTextSkewX(), flags);
}
diff --git a/tests/FontMgrTest.cpp b/tests/FontMgrTest.cpp
index 4abbd19..1ef97ee 100644
--- a/tests/FontMgrTest.cpp
+++ b/tests/FontMgrTest.cpp
@@ -19,18 +19,17 @@
static void test_font(skiatest::Reporter* reporter) {
uint32_t flags = 0;
- sk_sp<SkFont> font(SkFont::Make(nullptr, 24, SkFont::kA8_MaskType, flags));
+ SkFont font(nullptr, 24, flags);
- REPORTER_ASSERT(reporter, font->getTypeface());
- REPORTER_ASSERT(reporter, 24 == font->getSize());
- REPORTER_ASSERT(reporter, 1 == font->getScaleX());
- REPORTER_ASSERT(reporter, 0 == font->getSkewX());
- REPORTER_ASSERT(reporter, SkFont::kA8_MaskType == font->getMaskType());
+ REPORTER_ASSERT(reporter, font.getTypeface());
+ REPORTER_ASSERT(reporter, 24 == font.getSize());
+ REPORTER_ASSERT(reporter, 1 == font.getScaleX());
+ REPORTER_ASSERT(reporter, 0 == font.getSkewX());
uint16_t glyphs[5];
sk_bzero(glyphs, sizeof(glyphs));
- int count = font->textToGlyphs("Hello", 5, kUTF8_SkTextEncoding, glyphs, SK_ARRAY_COUNT(glyphs));
+ int count = font.textToGlyphs("Hello", 5, kUTF8_SkTextEncoding, glyphs, SK_ARRAY_COUNT(glyphs));
REPORTER_ASSERT(reporter, 5 == count);
for (int i = 0; i < count; ++i) {
@@ -39,18 +38,15 @@
REPORTER_ASSERT(reporter, glyphs[0] != glyphs[1]); // 'h' != 'e'
REPORTER_ASSERT(reporter, glyphs[2] == glyphs[3]); // 'l' == 'l'
- sk_sp<SkFont> newFont(font->makeWithSize(36));
- REPORTER_ASSERT(reporter, newFont.get());
- REPORTER_ASSERT(reporter, font->getTypeface() == newFont->getTypeface());
- REPORTER_ASSERT(reporter, 36 == newFont->getSize()); // double check we haven't changed
- REPORTER_ASSERT(reporter, 24 == font->getSize()); // double check we haven't changed
+ const SkFont newFont(font.makeWithSize(36));
+ REPORTER_ASSERT(reporter, font.getTypeface() == newFont.getTypeface());
+ REPORTER_ASSERT(reporter, 36 == newFont.getSize()); // double check we haven't changed
+ REPORTER_ASSERT(reporter, 24 == font.getSize()); // double check we haven't changed
SkPaint paint;
paint.setTextSize(18);
- font = SkFont::Testing_CreateFromPaint(paint);
- REPORTER_ASSERT(reporter, font.get());
- REPORTER_ASSERT(reporter, font->getSize() == paint.getTextSize());
- REPORTER_ASSERT(reporter, SkFont::kBW_MaskType == font->getMaskType());
+ font = SkFont::LEGACY_ExtractFromPaint(paint);
+ REPORTER_ASSERT(reporter, font.getSize() == paint.getTextSize());
}
/*
diff --git a/tests/FontObjTest.cpp b/tests/FontObjTest.cpp
index 44f799e..fa96112 100644
--- a/tests/FontObjTest.cpp
+++ b/tests/FontObjTest.cpp
@@ -10,35 +10,28 @@
#include "SkTypeface.h"
#include "Test.h"
-static bool is_use_nonlinear_metrics(const SkPaint& paint) {
- return !paint.isSubpixelText() && !paint.isLinearText();
-}
-
-static bool is_enable_auto_hints(const SkPaint& paint) {
- return paint.isAutohinted();
-}
-
-static bool is_enable_bytecode_hints(const SkPaint& paint) {
- return paint.getHinting() >= SkPaint::kFull_Hinting;
-}
-
-static void test_cachedfont(skiatest::Reporter* reporter, const SkPaint& paint) {
- sk_sp<SkFont> font(SkFont::Testing_CreateFromPaint(paint));
-
+static void test_cachedfont(skiatest::Reporter* reporter,
+ const SkPaint& paint, const SkFont& font) {
// Currently SkFont resolves null into the default, so only test if paint's is not null
if (paint.getTypeface()) {
- REPORTER_ASSERT(reporter, font->getTypeface() == paint.getTypeface());
+ REPORTER_ASSERT(reporter, font.getTypeface() == paint.getTypeface());
}
- REPORTER_ASSERT(reporter, font->getSize() == paint.getTextSize());
- REPORTER_ASSERT(reporter, font->getScaleX() == paint.getTextScaleX());
- REPORTER_ASSERT(reporter, font->getSkewX() == paint.getTextSkewX());
+ REPORTER_ASSERT(reporter, font.getSize() == paint.getTextSize());
+ REPORTER_ASSERT(reporter, font.getScaleX() == paint.getTextScaleX());
+ REPORTER_ASSERT(reporter, font.getSkewX() == paint.getTextSkewX());
- REPORTER_ASSERT(reporter, font->isVertical() == paint.isVerticalText());
- REPORTER_ASSERT(reporter, font->isEmbolden() == paint.isFakeBoldText());
+ uint32_t mask = SkPaint::kLinearText_Flag |
+ SkPaint::kSubpixelText_Flag |
+ SkPaint::kFakeBoldText_Flag |
+ SkPaint::kEmbeddedBitmapText_Flag |
+ SkPaint::kAutoHinting_Flag;
- REPORTER_ASSERT(reporter, font->isUseNonLinearMetrics() == is_use_nonlinear_metrics(paint));
- REPORTER_ASSERT(reporter, font->isEnableAutoHints() == is_enable_auto_hints(paint));
- REPORTER_ASSERT(reporter, font->isEnableByteCodeHints() == is_enable_bytecode_hints(paint));
+ SkPaint p;
+ font.LEGACY_applyToPaint(&p);
+ uint32_t oldFlags = paint.getFlags() & mask;
+ uint32_t newFlags = p.getFlags() & mask;
+ REPORTER_ASSERT(reporter, oldFlags == newFlags);
+ REPORTER_ASSERT(reporter, paint.getHinting() == p.getHinting());
}
static void test_cachedfont(skiatest::Reporter* reporter) {
@@ -89,7 +82,9 @@
paint.setTextScaleX(gScaleRec[k].fScaleX);
paint.setTextSkewX(gScaleRec[k].fSkewX);
- test_cachedfont(reporter, paint);
+ const SkFont font(SkFont::LEGACY_ExtractFromPaint(paint));
+
+ test_cachedfont(reporter, paint, font);
SkRect bounds;
@@ -102,8 +97,7 @@
REPORTER_ASSERT(reporter, width1 == width2);
- sk_sp<SkFont> font(SkFont::Testing_CreateFromPaint(paint));
- SkScalar font_width1 = font->measureText(txt, strlen(txt), kUTF8_SkTextEncoding);
+ SkScalar font_width1 = font.measureText(txt, strlen(txt), kUTF8_SkTextEncoding);
// measureText not yet implemented...
REPORTER_ASSERT(reporter, font_width1 == -1);
// REPORTER_ASSERT(reporter, width1 == font_width1);
@@ -112,8 +106,26 @@
}
}
+static void test_aa_hinting(skiatest::Reporter* reporter) {
+ SkPaint paint;
+
+ for (bool aa : {false, true}) {
+ paint.setAntiAlias(aa);
+ for (int hint = 0; hint <= 3; ++hint) {
+ paint.setHinting((SkPaint::Hinting)hint);
+ SkFont font = SkFont::LEGACY_ExtractFromPaint(paint);
+
+ SkPaint p2;
+ font.LEGACY_applyToPaint(&p2);
+ REPORTER_ASSERT(reporter, paint.isAntiAlias() == p2.isAntiAlias());
+ REPORTER_ASSERT(reporter, paint.getHinting() == p2.getHinting());
+ }
+ }
+}
+
DEF_TEST(FontObj, reporter) {
test_cachedfont(reporter);
+ test_aa_hinting(reporter);
}
// need tests for SkStrSearch
