Add empty check for mask case. Small cleanups elsewhere.

We were overly cautious with spaces close to a cut. Re-enable
empty glyph removal from the Renderer' diff canvas processing.

* rename prepareForDrawing -> prepareForDrawingRemoveEmpty

Bug: chromium:881505
Change-Id: I9b5d51cd66645bccfc3b1f6fefb317f9eaf621e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/233417
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Herb Derby 2019-08-08 18:02:27 -04:00 committed by Skia Commit-Bot
parent c86cf3cdb3
commit 1436a13881
7 changed files with 86 additions and 78 deletions

View File

@ -188,7 +188,8 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
fPositions,
fPackedGlyphIDs);
auto glyphPosSpan = strike->prepareForDrawing(packedGlyphIDs.data(),
auto glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
packedGlyphIDs.data(),
fPositions,
glyphRun.runSize(),
0,
@ -241,7 +242,7 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
fPositions,
fPackedGlyphIDs);
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawing(
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
packedGlyphIDs.data(),
fPositions,
glyphRun.runSize(),
@ -333,7 +334,7 @@ void SkGlyphRunListPainter::processARGBFallback(SkScalar maxSourceGlyphDimension
*cursor++ = SkPackedGlyphID{glyphID};
}
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawing(
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
fPackedGlyphIDs,
fARGBPositions.data(),
fARGBGlyphsIDs.size(),
@ -359,7 +360,8 @@ void SkGlyphRunListPainter::processARGBFallback(SkScalar maxSourceGlyphDimension
*cursor++ = SkPackedGlyphID{glyphID};
}
auto glyphPosSpan = strike->prepareForDrawing(fPackedGlyphIDs,
auto glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
fPackedGlyphIDs,
fARGBPositions.data(),
fARGBGlyphsIDs.size(),
SkStrikeCommon::kSkSideTooBigForAtlas,
@ -423,7 +425,7 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
fPositions,
fPackedGlyphIDs);
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawing(
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
packedGlyphIDs.data(),
fPositions,
glyphRun.runSize(),
@ -438,18 +440,13 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
// The SDF scaler context system ensures that a glyph is empty, kSDF_Format, or
// kARGB32_Format. The following if statements use this assumption.
SkASSERT(glyph.isEmpty()
|| glyph.maskFormat() == SkMask::kSDF_Format
|| glyph.isColor());
SkASSERT(glyph.maskFormat() == SkMask::kSDF_Format || glyph.isColor());
if (glyph.isEmpty()) {
// do nothing
} else if (glyph.maskFormat() == SkMask::kSDF_Format
if (glyph.maskFormat() == SkMask::kSDF_Format
&& glyph.maxDimension() <= SkStrikeCommon::kSkSideTooBigForAtlas) {
// SDF mask will work.
fGlyphPos[glyphsWithMaskCount++] = glyphPos;
} else if (!glyph.isColor()
&& glyph.path() != nullptr) {
} else if (!glyph.isColor() && glyph.path() != nullptr) {
// If not color but too big, use a path.
fPaths.push_back(glyphPos);
} else {
@ -485,7 +482,6 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
runPaint, runFont, viewMatrix, process);
}
} else if (SkStrikeSpec::ShouldDrawAsPath(runPaint, runFont, viewMatrix)) {
SkStrikeSpec strikeSpec = SkStrikeSpec::MakePath(
runFont, runPaint, fDeviceProps, fScalerContextFlags);
@ -499,7 +495,7 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
fPositions,
fPackedGlyphIDs);
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawing(
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
packedGlyphIDs.data(),
fPositions,
glyphRun.runSize(),
@ -512,10 +508,7 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
for (const SkGlyphPos& glyphPos : glyphPosSpan) {
const SkGlyph& glyph = *glyphPos.glyph;
SkPoint position = glyphPos.position;
if (glyph.isEmpty()) {
// do nothing
} else if (!glyph.isColor()
&& glyph.path() != nullptr) {
if (!glyph.isColor() && glyph.path() != nullptr) {
// Place paths in fGlyphPos
fGlyphPos[glyphsWithPathCount++] = glyphPos;
} else {
@ -537,7 +530,6 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
runPaint, runFont, viewMatrix, process);
}
} else {
SkStrikeSpec strikeSpec =
SkStrikeSpec::MakeMask(runFont, runPaint,
fDeviceProps, fScalerContextFlags, viewMatrix);
@ -554,8 +546,8 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
fPositions,
fPackedGlyphIDs);
// Lookup all the glyphs from the cache.
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawing(
// Lookup all the glyphs from the cache. Strip empty glyphs.
SkSpan<const SkGlyphPos> glyphPosSpan = strike->prepareForDrawingRemoveEmpty(
packedGlyphIDs.data(),
fPositions,
glyphRun.runSize(),
@ -569,15 +561,12 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
const SkGlyph& glyph = *glyphPos.glyph;
const SkPoint position = glyphPos.position;
// Able to position glyph?
// Does the glyph have work to do or is the code able to position the glyph?
if (!SkScalarsAreFinite(position.x(), position.y())) {
continue;
}
if (glyph.maxDimension() <= SkStrikeCommon::kSkSideTooBigForAtlas) {
// Do nothing;
} else if (glyph.maxDimension() <= SkStrikeCommon::kSkSideTooBigForAtlas) {
fGlyphPos[glyphsWithMaskCount++] = glyphPos;
} else if (!glyph.isColor()
&& glyph.path() != nullptr) {
} else if (!glyph.isColor() && glyph.path() != nullptr) {
fPaths.push_back(glyphPos);
} else {
addFallback(glyph, origin + glyphRun.positions()[glyphPos.index]);

View File

@ -547,14 +547,18 @@ void SkStrikeServer::SkGlyphCacheState::writeGlyphPath(const SkPackedGlyphID& gl
}
// Be sure to read and understand the comment for prepareForDrawing in SkStrikeInterface.h before
// working on this code.
// Be sure to read and understand the comment for prepareForDrawingRemoveEmpty in
// SkStrikeInterface.h before working on this code.
SkSpan<const SkGlyphPos>
SkStrikeServer::SkGlyphCacheState::prepareForDrawing(const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[], size_t n,
int maxDimension, PreparationDetail detail,
SkStrikeServer::SkGlyphCacheState::prepareForDrawingRemoveEmpty(
const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[],
size_t n,
int maxDimension,
PreparationDetail detail,
SkGlyphPos results[]) {
size_t drawableGlyphCount = 0;
for (size_t i = 0; i < n; i++) {
SkPoint glyphPos = positions[i];
@ -573,9 +577,8 @@ SkStrikeServer::SkGlyphCacheState::prepareForDrawing(const SkPackedGlyphID packe
if (glyphPtr->maxDimension() <= maxDimension) {
// do nothing
} else if (!glyphPtr->isColor()) {
// The glyph is too big for the atlas, but it is not color, so it is handled with a
// path.
// The glyph is too big for the atlas, but it is not color, so it is handled
// with a path.
if (glyphPtr->setPath(&fAlloc, fContext.get())) {
// Always send the path data, even if its not available, to make sure empty
// paths are not incorrectly assumed to be cache misses.
@ -592,12 +595,14 @@ SkStrikeServer::SkGlyphCacheState::prepareForDrawing(const SkPackedGlyphID packe
fPendingGlyphImages.push_back(packedGlyphIDs[i]);
}
// Each glyph needs to be added as per the contract for prepareForDrawing.
// TODO(herb): check if the empty glyphs need to be added here. They certainly need to be
// sent, but do the need to be processed by the painter?
results[i] = {i, glyphPtr, glyphPos};
// Each non-empty glyph needs to be added as per the contract for
// prepareForDrawingRemoveEmpty.
// TODO(herb): Change the code to only send the glyphs for fallback?
if (!glyphPtr->isEmpty()) {
results[drawableGlyphCount++] = {i, glyphPtr, glyphPos};
}
return SkSpan<const SkGlyphPos>{results, n};
}
return SkMakeSpan(results, drawableGlyphCount);
}
// SkStrikeClient -----------------------------------------

View File

@ -42,8 +42,12 @@ public:
}
SkSpan<const SkGlyphPos>
prepareForDrawing(const SkPackedGlyphID packedGlyphIDs[], const SkPoint positions[], size_t n,
int maxDimension, PreparationDetail detail, SkGlyphPos results[]) override;
prepareForDrawingRemoveEmpty(const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[],
size_t n,
int maxDimension,
PreparationDetail detail,
SkGlyphPos results[]) override;
void onAboutToExitScope() override {}

View File

@ -189,10 +189,12 @@ SkStrike::prepareImages(SkSpan<const SkPackedGlyphID> glyphIDs, const SkGlyph* r
// N.B. This glyphMetrics call culls all the glyphs which will not display based on a non-finite
// position or that there are no mask pixels.
SkSpan<const SkGlyphPos>
SkStrike::prepareForDrawing(const SkPackedGlyphID packedGlyphIDs[], const SkPoint positions[],
SkStrike::prepareForDrawingRemoveEmpty(const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[],
size_t n,
int maxDimension, PreparationDetail detail, SkGlyphPos results[]) {
int maxDimension,
PreparationDetail detail,
SkGlyphPos results[]) {
size_t drawableGlyphCount = 0;
for (size_t i = 0; i < n; i++) {
SkPoint pos = positions[i];

View File

@ -119,7 +119,7 @@ public:
SkSpan<const SkGlyph*> prepareImages(SkSpan<const SkPackedGlyphID> glyphIDs,
const SkGlyph* results[]);
SkSpan<const SkGlyphPos> prepareForDrawing(const SkPackedGlyphID packedGlyphIDs[],
SkSpan<const SkGlyphPos> prepareForDrawingRemoveEmpty(const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[],
size_t n,
int maxDimension,

View File

@ -37,9 +37,17 @@ public:
}
SkSpan<const SkGlyphPos>
prepareForDrawing(const SkPackedGlyphID packedGlyphIDs[], const SkPoint positions[], size_t n,
int maxDimension, PreparationDetail detail, SkGlyphPos results[]) override {
return fStrike.prepareForDrawing(packedGlyphIDs, positions, n, maxDimension, detail,
prepareForDrawingRemoveEmpty(const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[],
size_t n,
int maxDimension,
PreparationDetail detail,
SkGlyphPos results[]) override {
return fStrike.prepareForDrawingRemoveEmpty(packedGlyphIDs,
positions,
n,
maxDimension,
detail,
results);
}

View File

@ -56,15 +56,16 @@ public:
kImageIfNeeded,
};
// prepareForDrawing takes glyphIDs, and position, and returns a list of SkGlyphs and
// positions where all the data to draw the glyph has been created. The maxDimension
// prepareForDrawingRemoveEmpty takes glyphIDs, and position, and returns a list of SkGlyphs
// and positions where all the data to draw the glyph has been created. The maxDimension
// parameter determines if the mask/SDF version will be created, or an alternate drawing
// format should be used. For path-only drawing set maxDimension to 0, and for bitmap-device
// drawing (where there is no upper limit to the glyph in the cache) use INT_MAX.
// * PreparationDetail determines, in the mask case, if the mask/SDF should be generated.
// This does not affect the path or fallback cases.
// prepareForDrawingRemoveEmpty should remove all empty glyphs from the returned span.
virtual SkSpan<const SkGlyphPos>
prepareForDrawing(const SkPackedGlyphID packedGlyphIDs[],
prepareForDrawingRemoveEmpty(const SkPackedGlyphID packedGlyphIDs[],
const SkPoint positions[],
size_t n,
int maxDimension,
@ -100,5 +101,4 @@ public:
const SkScalerContextEffects& effects,
const SkTypeface& typeface) = 0;
};
#endif //SkStrikeInterface_DEFINED