fonts: Ignore re-initialization of fallback glyphs from the server.

If the client finds that the server re-initializes a cached path/image,
we consider this an error and invalid data. But since we might
initialize a glyph using a fallback on the client, and receive the
correct version from the server later, this is not longer true.

Bug: 829622
Change-Id: I34ab17b54139d89a15179265d4aed4a1fe36fd47
Reviewed-on: https://skia-review.googlesource.com/133566
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This commit is contained in:
Khushal 2018-06-13 09:44:23 -07:00 committed by Skia Commit-Bot
parent c6530d1e5e
commit 4010c792c6
5 changed files with 140 additions and 23 deletions

View File

@ -85,6 +85,12 @@ struct SkPackedID {
return SkChecksum::CheapMix(fID);
}
SkString dump() const {
SkString str;
str.appendf("code: %d, x: %d, y:%d", code(), getSubXFixed(), getSubYFixed());
return str;
}
private:
static unsigned ID2SubX(uint32_t id) {
return id >> (kSubShift + kSubShiftX);
@ -143,8 +149,8 @@ class SkGlyph {
public:
static const SkFixed kSubpixelRound = SK_FixedHalf >> SkPackedID::kSubBits;
void* fImage;
PathData* fPathData;
void* fImage;
PathData* fPathData;
float fAdvanceX, fAdvanceY;
uint16_t fWidth, fHeight;

View File

@ -201,8 +201,10 @@ const void* SkGlyphCache::findImage(const SkGlyph& glyph) {
return glyph.fImage;
}
bool SkGlyphCache::initializeImage(const volatile void* data, size_t size, SkGlyph* glyph) {
if (glyph->fImage) return false;
void SkGlyphCache::initializeImage(const volatile void* data, size_t size, SkGlyph* glyph) {
// Don't overwrite the image if we already have one. We could have used a fallback if the
// glyph was missing earlier.
if (glyph->fImage) return;
if (glyph->fWidth > 0 && glyph->fWidth < kMaxGlyphWidth) {
size_t allocSize = glyph->allocImage(&fAlloc);
@ -213,8 +215,6 @@ bool SkGlyphCache::initializeImage(const volatile void* data, size_t size, SkGly
fMemoryUsed += size;
}
}
return true;
}
const SkPath* SkGlyphCache::findPath(const SkGlyph& glyph) {
@ -237,7 +237,9 @@ const SkPath* SkGlyphCache::findPath(const SkGlyph& glyph) {
}
bool SkGlyphCache::initializePath(SkGlyph* glyph, const volatile void* data, size_t size) {
if (glyph->fPathData) return false;
// Don't overwrite the path if we already have one. We could have used a fallback if the
// glyph was missing earlier.
if (glyph->fPathData) return true;
if (glyph->fWidth) {
SkGlyph::PathData* pathData = fAlloc.make<SkGlyph::PathData>();

View File

@ -91,10 +91,9 @@ public:
*/
const void* findImage(const SkGlyph&);
/** Initializes the image associated with the glyph with |data|. Returns false if an image
* already exists.
/** Initializes the image associated with the glyph with |data|.
*/
bool initializeImage(const volatile void* data, size_t size, SkGlyph*);
void initializeImage(const volatile void* data, size_t size, SkGlyph*);
/** If the advance axis intersects the glyph's path, append the positions scaled and offset
to the array (if non-null), and set the count to the updated array length.
@ -107,8 +106,8 @@ public:
*/
const SkPath* findPath(const SkGlyph&);
/** Initializes the path associated with the glyph with |data|. Returns false if a path
* already exits or data is invalid.
/** Initializes the path associated with the glyph with |data|. Returns false if
* data is invalid.
*/
bool initializePath(SkGlyph*, const volatile void* data, size_t size);

View File

@ -668,6 +668,8 @@ void SkStrikeServer::SkGlyphCacheState::writePendingGlyphs(Serializer* serialize
SkGlyph stationaryGlyph = *glyph;
stationaryGlyph.fImage = serializer->allocate(imageSize, stationaryGlyph.formatAlignment());
fContext->getImage(stationaryGlyph);
// TODO: Generating the image can change the mask format, do we need to update it in the
// serialized glyph?
}
fPendingGlyphImages.clear();
@ -811,10 +813,14 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi
if (!deserializer.read<SkGlyph>(&glyph)) READ_FAILURE
SkGlyph* allocatedGlyph = strike->getRawGlyphByID(glyph.getPackedID());
// Don't override the path, if the glyph has one.
auto* glyphPath = allocatedGlyph->fPathData;
*allocatedGlyph = glyph;
allocatedGlyph->fPathData = glyphPath;
// Update the glyph unless it's already got an image (from fallback),
// preserving any path that might be present.
if (allocatedGlyph->fImage == nullptr) {
auto* glyphPath = allocatedGlyph->fPathData;
*allocatedGlyph = glyph;
allocatedGlyph->fPathData = glyphPath;
}
bool tooLargeForAtlas = false;
#if SK_SUPPORT_GPU
@ -828,9 +834,8 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi
if (imageSize == 0u) continue;
auto* image = deserializer.read(imageSize, allocatedGlyph->formatAlignment());
if (!image ||
!strike->initializeImage(image, imageSize, allocatedGlyph))
READ_FAILURE
if (!image) READ_FAILURE
strike->initializeImage(image, imageSize, allocatedGlyph);
}
size_t glyphPathsCount = 0u;
@ -840,10 +845,14 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi
if (!deserializer.read<SkGlyph>(&glyph)) READ_FAILURE
SkGlyph* allocatedGlyph = strike->getRawGlyphByID(glyph.getPackedID());
// Don't override the image, if the glyph has one.
auto* glyphImage = allocatedGlyph->fImage;
*allocatedGlyph = glyph;
allocatedGlyph->fImage = glyphImage;
// Update the glyph unless it's already got a path (from fallback),
// preserving any image that might be present.
if (allocatedGlyph->fPathData == nullptr) {
auto* glyphImage = allocatedGlyph->fImage;
*allocatedGlyph = glyph;
allocatedGlyph->fImage = glyphImage;
}
if (!read_path(&deserializer, allocatedGlyph, strike.get())) READ_FAILURE
}

View File

@ -509,3 +509,104 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
// Must unlock everything on termination, otherwise valgrind complains about memory leaks.
discardableManager->unlockAndDeleteAll();
}
DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
// Build proxy typeface on the client for initializing the cache.
sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
SkStrikeServer server(discardableManager.get());
SkStrikeClient client(discardableManager, false);
auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
auto tfData = server.serializeTypeface(serverTf.get());
auto clientTf = client.deserializeTypeface(tfData->data(), tfData->size());
REPORTER_ASSERT(reporter, clientTf);
SkPaint paint;
paint.setAntiAlias(true);
paint.setColor(SK_ColorRED);
auto lostGlyphID = SkPackedGlyphID(1, SK_FixedHalf, SK_FixedHalf);
const uint8_t glyphImage[] = {0xFF, 0xFF};
uint32_t realMask;
uint32_t fakeMask;
{
SkAutoDescriptor ad;
SkScalerContextRec rec;
SkScalerContextEffects effects;
SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
paint.setTypeface(serverTf);
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto context = serverTf->createScalerContext(effects, desc, false);
SkGlyph glyph;
glyph.initWithGlyphID(lostGlyphID);
context->getMetrics(&glyph);
realMask = glyph.fMaskFormat;
REPORTER_ASSERT(reporter, realMask != MASK_FORMAT_UNKNOWN);
}
// Build a fallback cache.
{
SkAutoDescriptor ad;
SkScalerContextRec rec;
SkScalerContextEffects effects;
SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
paint.setTypeface(clientTf);
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *clientTf);
auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
fakeMask = (realMask == SkMask::kA8_Format) ? SkMask::kBW_Format : SkMask::kA8_Format;
glyph->fMaskFormat = fakeMask;
glyph->fHeight = 1;
glyph->fWidth = 2;
fallbackCache->initializeImage(glyphImage, glyph->computeImageSize(), glyph);
}
// Send over the real glyph and make sure the client cache stays intact.
{
SkAutoDescriptor ad;
SkScalerContextRec rec;
SkScalerContextEffects effects;
SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
paint.setTypeface(serverTf);
auto* cacheState = server.getOrCreateCache(paint, nullptr, nullptr, flags, &rec, &effects);
cacheState->addGlyph(serverTf.get(), effects, lostGlyphID, false);
std::vector<uint8_t> serverStrikeData;
server.writeStrikeData(&serverStrikeData);
REPORTER_ASSERT(reporter,
client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
}
{
SkAutoDescriptor ad;
SkScalerContextRec rec;
SkScalerContextEffects effects;
SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
paint.setTypeface(clientTf);
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto fallbackCache = SkStrikeCache::FindStrikeExclusive(*desc);
REPORTER_ASSERT(reporter, fallbackCache.get() != nullptr);
auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
REPORTER_ASSERT(reporter, glyph->fMaskFormat == fakeMask);
// Try overriding the image, it should stay the same.
REPORTER_ASSERT(reporter,
memcmp(glyph->fImage, glyphImage, glyph->computeImageSize()) == 0);
const uint8_t newGlyphImage[] = {0, 0};
fallbackCache->initializeImage(newGlyphImage, glyph->computeImageSize(), glyph);
REPORTER_ASSERT(reporter,
memcmp(glyph->fImage, glyphImage, glyph->computeImageSize()) == 0);
}
SkStrikeCache::Validate();
// Must unlock everything on termination, otherwise valgrind complains about memory leaks.
discardableManager->unlockAndDeleteAll();
}