don't use MASK_FORMAT_UNKNOWN to represent scaler context state

MASK_FORMAT_UNKNOWN was shared state between glyphs, and
SkScalerContext making hard to reason about.

Change-Id: Ifb70bcc288bc46905de5f2c32891c940a0b2a11f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334157
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This commit is contained in:
Herb Derby 2020-11-11 14:54:00 -05:00 committed by Skia Commit-Bot
parent 7ac2be2020
commit 4168981bb0
9 changed files with 70 additions and 90 deletions

View File

@ -15,9 +15,6 @@
constexpr SkIPoint SkPackedGlyphID::kXYFieldMask;
SkMask SkGlyph::mask() const {
// getMetrics had to be called.
SkASSERT(fMaskFormat != MASK_FORMAT_UNKNOWN);
SkMask mask;
mask.fImage = (uint8_t*)fImage;
mask.fBounds.setXYWH(fLeft, fTop, fWidth, fHeight);

View File

@ -19,9 +19,6 @@
class SkArenaAlloc;
class SkScalerContext;
// needs to be != to any valid SkMask::Format
#define MASK_FORMAT_UNKNOWN (0xFF)
// A combination of SkGlyphID and sub-pixel position information.
struct SkPackedGlyphID {
static constexpr uint32_t kImpossibleID = ~0u;
@ -64,7 +61,6 @@ struct SkPackedGlyphID {
: fID{PackIDSkPoint(glyphID, pt, mask)} { }
constexpr explicit SkPackedGlyphID(uint32_t v) : fID{v & kMaskAll} { }
constexpr SkPackedGlyphID() : fID{kImpossibleID} {}
bool operator==(const SkPackedGlyphID& that) const {
@ -400,15 +396,12 @@ private:
float fAdvanceX = 0,
fAdvanceY = 0;
// This is a combination of SkMask::Format and SkGlyph state. The SkGlyph can be in one of two
// states, just the advances have been calculated, and all the metrics are available. The
// illegal mask format is used to signal that only the advances are available.
uint8_t fMaskFormat = MASK_FORMAT_UNKNOWN;
uint8_t fMaskFormat = 0;
// Used by the DirectWrite scaler to track state.
int8_t fForceBW = 0;
const SkPackedGlyphID fID;
SkPackedGlyphID fID;
};
#endif

View File

@ -474,12 +474,10 @@ void RemoteStrike::commonMaskLoop(
MaskSummary* summary = fSentGlyphs.find(packedID);
if (summary == nullptr) {
// Put the new SkGlyph in the glyphs to send.
fMasksToSend.emplace_back(packedID);
this->ensureScalerContext();
fMasksToSend.emplace_back(fContext->makeGlyph(packedID));
SkGlyph* glyph = &fMasksToSend.back();
// Build the glyph
this->ensureScalerContext();
fContext->getMetrics(glyph);
MaskSummary newSummary =
{packedID.value(), CanDrawAsMask(*glyph), CanDrawAsSDFT(*glyph)};
summary = fSentGlyphs.set(newSummary);
@ -507,13 +505,11 @@ void RemoteStrike::prepareForMaskDrawing(
MaskSummary* summary = fSentGlyphs.find(packedID);
if (summary == nullptr) {
// Put the new SkGlyph in the glyphs to send.
fMasksToSend.emplace_back(packedID);
SkGlyph* glyph = &fMasksToSend.back();
// Build the glyph
// Put the new SkGlyph in the glyphs to send.
this->ensureScalerContext();
fContext->getMetrics(glyph);
fMasksToSend.emplace_back(fContext->makeGlyph(packedID));
SkGlyph* glyph = &fMasksToSend.back();
MaskSummary newSummary =
{packedID.value(), CanDrawAsMask(*glyph), CanDrawAsSDFT(*glyph)};
@ -545,13 +541,11 @@ void RemoteStrike::prepareForPathDrawing(
SkGlyphID glyphID = packedID.glyphID();
PathSummary* summary = fSentPaths.find(glyphID);
if (summary == nullptr) {
// Put the new SkGlyph in the glyphs to send.
fPathsToSend.emplace_back(SkPackedGlyphID{glyphID});
SkGlyph* glyph = &fPathsToSend.back();
// Build the glyph
// Put the new SkGlyph in the glyphs to send.
this->ensureScalerContext();
fContext->getMetrics(glyph);
fPathsToSend.emplace_back(fContext->makeGlyph(SkPackedGlyphID{glyphID}));
SkGlyph* glyph = &fPathsToSend.back();
uint16_t maxDimensionOrPath = glyph->maxDimension();
// Only try to get the path if the glyphs is not color.

View File

@ -48,8 +48,7 @@ std::tuple<SkGlyphDigest, size_t> SkScalerCache::digest(SkPackedGlyphID packedGl
return {*digest, 0};
}
SkGlyph* glyph = fAlloc.make<SkGlyph>(packedGlyphID);
fScalerContext->getMetrics(glyph);
SkGlyph* glyph = fAlloc.make<SkGlyph>(fScalerContext->makeGlyph(packedGlyphID));
return {this->addGlyph(glyph), sizeof(SkGlyph)};
}

View File

@ -177,70 +177,67 @@ bool SkScalerContext::GetGammaLUTData(SkScalar contrast, SkScalar paintGamma, Sk
return true;
}
void SkScalerContext::getMetrics(SkGlyph* glyph) {
SkGlyph SkScalerContext::makeGlyph(SkPackedGlyphID packedID) {
return internalMakeGlyph(packedID, (SkMask::Format) fRec.fMaskFormat);
}
SkGlyph SkScalerContext::internalMakeGlyph(SkPackedGlyphID packedID, SkMask::Format format) {
SkGlyph glyph{packedID};
glyph.fMaskFormat = format;
bool generatingImageFromPath = fGenerateImageFromPath;
if (!generatingImageFromPath) {
generateMetrics(glyph);
SkASSERT(glyph->fMaskFormat != MASK_FORMAT_UNKNOWN);
generateMetrics(&glyph);
} else {
SkPath devPath;
generatingImageFromPath = this->internalGetPath(glyph->getPackedID(), &devPath);
generatingImageFromPath = this->internalGetPath(glyph.getPackedID(), &devPath);
if (!generatingImageFromPath) {
generateMetrics(glyph);
SkASSERT(glyph->fMaskFormat != MASK_FORMAT_UNKNOWN);
generateMetrics(&glyph);
} else {
uint8_t originMaskFormat = glyph->fMaskFormat;
if (!generateAdvance(glyph)) {
generateMetrics(glyph);
}
if (originMaskFormat != MASK_FORMAT_UNKNOWN) {
glyph->fMaskFormat = originMaskFormat;
} else {
glyph->fMaskFormat = fRec.fMaskFormat;
if (!generateAdvance(&glyph)) {
generateMetrics(&glyph);
}
// If we are going to create the mask, then we cannot keep the color
if (SkMask::kARGB32_Format == glyph->fMaskFormat) {
glyph->fMaskFormat = SkMask::kA8_Format;
if (SkMask::kARGB32_Format == glyph.fMaskFormat) {
glyph.fMaskFormat = SkMask::kA8_Format;
}
const SkIRect ir = devPath.getBounds().roundOut();
if (ir.isEmpty() || !SkRectPriv::Is16Bit(ir)) {
goto SK_ERROR;
}
glyph->fLeft = ir.fLeft;
glyph->fTop = ir.fTop;
glyph->fWidth = SkToU16(ir.width());
glyph->fHeight = SkToU16(ir.height());
glyph.fLeft = ir.fLeft;
glyph.fTop = ir.fTop;
glyph.fWidth = SkToU16(ir.width());
glyph.fHeight = SkToU16(ir.height());
const bool a8FromLCD = fRec.fFlags & SkScalerContext::kGenA8FromLCD_Flag;
const bool fromLCD = (glyph->fMaskFormat == SkMask::kLCD16_Format) ||
(glyph->fMaskFormat == SkMask::kA8_Format && a8FromLCD);
if (0 < glyph->fWidth && fromLCD) {
const bool fromLCD = (glyph.fMaskFormat == SkMask::kLCD16_Format) ||
(glyph.fMaskFormat == SkMask::kA8_Format && a8FromLCD);
if (0 < glyph.fWidth && fromLCD) {
if (fRec.fFlags & SkScalerContext::kLCD_Vertical_Flag) {
glyph->fHeight += 2;
glyph->fTop -= 1;
glyph.fHeight += 2;
glyph.fTop -= 1;
} else {
glyph->fWidth += 2;
glyph->fLeft -= 1;
glyph.fWidth += 2;
glyph.fLeft -= 1;
}
}
}
}
// if either dimension is empty, zap the image bounds of the glyph
if (0 == glyph->fWidth || 0 == glyph->fHeight) {
glyph->fWidth = 0;
glyph->fHeight = 0;
glyph->fTop = 0;
glyph->fLeft = 0;
glyph->fMaskFormat = 0;
return;
if (0 == glyph.fWidth || 0 == glyph.fHeight) {
glyph.fWidth = 0;
glyph.fHeight = 0;
glyph.fTop = 0;
glyph.fLeft = 0;
glyph.fMaskFormat = 0;
return glyph;
}
if (fMaskFilter) {
SkMask src = glyph->mask(),
SkMask src = glyph.mask(),
dst;
SkMatrix matrix;
@ -252,24 +249,23 @@ void SkScalerContext::getMetrics(SkGlyph* glyph) {
goto SK_ERROR;
}
SkASSERT(dst.fImage == nullptr);
glyph->fLeft = dst.fBounds.fLeft;
glyph->fTop = dst.fBounds.fTop;
glyph->fWidth = SkToU16(dst.fBounds.width());
glyph->fHeight = SkToU16(dst.fBounds.height());
glyph->fMaskFormat = dst.fFormat;
glyph.fLeft = dst.fBounds.fLeft;
glyph.fTop = dst.fBounds.fTop;
glyph.fWidth = SkToU16(dst.fBounds.width());
glyph.fHeight = SkToU16(dst.fBounds.height());
glyph.fMaskFormat = dst.fFormat;
}
}
return;
return glyph;
SK_ERROR:
// draw nothing 'cause we failed
glyph->fLeft = 0;
glyph->fTop = 0;
glyph->fWidth = 0;
glyph->fHeight = 0;
// put a valid value here, in case it was earlier set to
// MASK_FORMAT_JUST_ADVANCE
glyph->fMaskFormat = fRec.fMaskFormat;
glyph.fLeft = 0;
glyph.fTop = 0;
glyph.fWidth = 0;
glyph.fHeight = 0;
glyph.fMaskFormat = fRec.fMaskFormat;
return glyph;
}
#define SK_SHOW_TEXT_BLIT_COVERAGE 0
@ -533,17 +529,16 @@ static void generateMask(const SkMask& mask, const SkPath& path,
void SkScalerContext::getImage(const SkGlyph& origGlyph) {
const SkGlyph* glyph = &origGlyph;
SkGlyph tmpGlyph{origGlyph.getPackedID()};
// in case we need to call generateImage on a mask-format that is different
// (i.e. larger) than what our caller allocated by looking at origGlyph.
SkAutoMalloc tmpGlyphImageStorage;
SkGlyph tmpGlyph;
if (fMaskFilter) { // restore the prefilter bounds
// need the original bounds, sans our maskfilter
sk_sp<SkMaskFilter> mf = std::move(fMaskFilter);
this->getMetrics(&tmpGlyph);
tmpGlyph = this->internalMakeGlyph(origGlyph.getPackedID(),
(SkMask::Format) fRec.fMaskFormat);
fMaskFilter = std::move(mf);
// we need the prefilter bounds to be <= filter bounds

View File

@ -284,7 +284,7 @@ public:
bool isVertical() const { return false; }
unsigned getGlyphCount() { return this->generateGlyphCount(); }
void getMetrics(SkGlyph*);
SkGlyph makeGlyph(SkPackedGlyphID);
void getImage(const SkGlyph&);
bool SK_WARN_UNUSED_RESULT getPath(SkPackedGlyphID, SkPath*);
void getFontMetrics(SkFontMetrics*);
@ -371,7 +371,7 @@ protected:
/** Generates the contents of glyph.fWidth, fHeight, fTop, fLeft,
* as well as fAdvanceX and fAdvanceY if not already set.
*
* TODO: fMaskFormat is set by getMetrics later; cannot be set here.
* TODO: fMaskFormat is set by internalMakeGlyph later; cannot be set here.
*/
virtual void generateMetrics(SkGlyph* glyph) = 0;
@ -421,6 +421,7 @@ private:
/** Returns false if the glyph has no path at all. */
bool internalGetPath(SkPackedGlyphID id, SkPath* devPath);
SkGlyph internalMakeGlyph(SkPackedGlyphID packedID, SkMask::Format format);
// SkMaskGamma::PreBlend converts linear masks to gamma correcting masks.
protected:

View File

@ -217,7 +217,7 @@ protected:
void generateMetrics(SkGlyph* glyph) override {
glyph->zeroMetrics();
this->generateAdvance(glyph);
// Always generates from paths, so SkScalerContext::getMetrics will figure the bounds.
// Always generates from paths, so SkScalerContext::makeGlyph will figure the bounds.
}
void generateImage(const SkGlyph&) override { SK_ABORT("Should have generated from path."); }

View File

@ -55,14 +55,15 @@ bool RandomScalerContext::generateAdvance(SkGlyph* glyph) { return fProxy->gener
void RandomScalerContext::generateMetrics(SkGlyph* glyph) {
// Here we will change the mask format of the glyph
// NOTE: this may be overridden by the base class (e.g. if a mask filter is applied).
SkMask::Format format = SkMask::kA8_Format;
switch (glyph->getGlyphID() % 4) {
case 0: glyph->fMaskFormat = SkMask::kLCD16_Format; break;
case 1: glyph->fMaskFormat = SkMask::kA8_Format; break;
case 2: glyph->fMaskFormat = SkMask::kARGB32_Format; break;
case 3: glyph->fMaskFormat = SkMask::kBW_Format; break;
case 0: format = SkMask::kLCD16_Format; break;
case 1: format = SkMask::kA8_Format; break;
case 2: format = SkMask::kARGB32_Format; break;
case 3: format = SkMask::kBW_Format; break;
}
fProxy->getMetrics(glyph);
*glyph = fProxy->internalMakeGlyph(glyph->getPackedID(), format);
if (fFakeIt || (glyph->getGlyphID() % 4) != 2) {
return;

View File

@ -175,7 +175,7 @@ protected:
void generateMetrics(SkGlyph* glyph) override {
glyph->zeroMetrics();
this->generateAdvance(glyph);
// Always generates from paths, so SkScalerContext::getMetrics will figure the bounds.
// Always generates from paths, so SkScalerContext::makeGlyph will figure the bounds.
}
void generateImage(const SkGlyph&) override { SK_ABORT("Should have generated from path."); }