AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Use local strike caches to avoid flaky test behavior

Browse Source 
The remote glyph cache tests assume that the strike cache will not
change during a test. This is not true because other test also
mutate the strike cache. This is causing flaky tests.

 BUG=skia:8091

Change-Id: I397d411f9412006715f6860941dfb05ad54ae1b6
Reviewed-on: https://skia-review.googlesource.com/136741
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This commit is contained in:
Herb Derby 2018-06-21 15:15:50 -04:00 committed by Skia Commit-Bot
parent c113e9e23d
commit 5c0c7983bb
7 changed files with 38 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/core/SkRemoteGlyphCache.cpp
View File

@ -734,8 +734,11 @@ private:
sk_sp<DiscardableHandleManager> fManager;
};
SkStrikeClient::SkStrikeClient(sk_sp<DiscardableHandleManager> discardableManager, bool isLogging)
SkStrikeClient::SkStrikeClient(sk_sp<DiscardableHandleManager> discardableManager,
bool isLogging,
SkStrikeCache* strikeCache)
: fDiscardableHandleManager(std::move(discardableManager))
, fStrikeCache{strikeCache ? strikeCache : SkStrikeCache::GlobalStrikeCache()}
, fIsLogging{isLogging} {}
SkStrikeClient::~SkStrikeClient() = default;
@ -792,18 +795,19 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi
SkAutoDescriptor ad;
auto* client_desc = auto_descriptor_from_desc(sourceAd.getDesc(), tf->uniqueID(), &ad);
auto strike = SkStrikeCache::FindStrikeExclusive(*client_desc);
auto strike = fStrikeCache->findStrikeExclusive(*client_desc);
if (strike == nullptr) {
// Note that we don't need to deserialize the effects since we won't be generating any
// glyphs here anyway, and the desc is still correct since it includes the serialized
// effects.
SkScalerContextEffects effects;
auto scaler = SkStrikeCache::CreateScalerContext(*client_desc, effects, *tf);
strike = SkStrikeCache::CreateStrikeExclusive(
strike = fStrikeCache->createStrikeExclusive(
*client_desc, std::move(scaler), &fontMetrics,
skstd::make_unique<DiscardableStrikePinner>(spec.discardableHandleId,
fDiscardableHandleManager));
static_cast<SkScalerContextProxy*>(strike->getScalerContext())->initCache(strike.get());
auto proxyContext = static_cast<SkScalerContextProxy*>(strike->getScalerContext());
proxyContext->initCache(strike.get(), fStrikeCache);
}
size_t glyphImagesCount = 0u;
@ -865,7 +869,7 @@ sk_sp<SkTypeface> SkStrikeClient::deserializeTypeface(const void* buf, size_t le
WireTypeface wire;
if (len != sizeof(wire)) return nullptr;
memcpy(&wire, buf, sizeof(wire));
return addTypeface(wire);
return this->addTypeface(wire);
}
sk_sp<SkTypeface> SkStrikeClient::addTypeface(const WireTypeface& wire) {

7
src/core/SkRemoteGlyphCache.h
View File

@ -20,7 +20,6 @@
#include "SkMakeUnique.h"
#include "SkNoDrawCanvas.h"
#include "SkRefCnt.h"
#include "SkRemoteGlyphCache.h"
#include "SkSerialProcs.h"
#include "SkTypeface.h"
@ -30,6 +29,7 @@ class SkGlyphCache;
struct SkPackedGlyphID;
enum SkScalerContextFlags : uint32_t;
class SkScalerContextRecDescriptor;
class SkStrikeCache;
class SkTextBlobRunIterator;
class SkTypefaceProxy;
struct WireTypeface;
@ -212,7 +212,9 @@ public:
virtual void notifyCacheMiss(CacheMissType) {}
};
SkStrikeClient(sk_sp<DiscardableHandleManager>, bool isLogging = true);
SkStrikeClient(sk_sp<DiscardableHandleManager>,
bool isLogging = true,
SkStrikeCache* strikeCache = nullptr);
~SkStrikeClient();
// Deserializes the typeface previously serialized using the SkStrikeServer. Returns null if the
@ -232,6 +234,7 @@ private:
SkTHashMap<SkFontID, sk_sp<SkTypeface>> fRemoteFontIdToTypeface;
sk_sp<DiscardableHandleManager> fDiscardableHandleManager;
SkStrikeCache* const fStrikeCache;
const bool fIsLogging;
};

10
src/core/SkStrikeCache.cpp
View File

@ -111,16 +111,6 @@ SkExclusiveStrikePtr SkStrikeCache::FindStrikeExclusive(const SkDescriptor& desc
return GlobalStrikeCache()->findStrikeExclusive(desc);
}
bool SkStrikeCache::DesperationSearchForImage(const SkDescriptor& desc, SkGlyph* glyph,
SkGlyphCache* targetCache) {
return GlobalStrikeCache()->desperationSearchForImage(desc, glyph, targetCache);
}
bool SkStrikeCache::DesperationSearchForPath(
const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path) {
return GlobalStrikeCache()->desperationSearchForPath(desc, glyphID, path);
}
std::unique_ptr<SkScalerContext> SkStrikeCache::CreateScalerContext(
const SkDescriptor& desc,
const SkScalerContextEffects& effects,

7
src/core/SkStrikeCache.h
View File

@ -97,16 +97,9 @@ public:
// Routines to find suitable data when working in a remote cache situation. These are
// suitable as substitutes for similar calls in SkScalerContext.
static bool DesperationSearchForImage(const SkDescriptor& desc,
SkGlyph* glyph,
SkGlyphCache* targetCache);
bool desperationSearchForImage(const SkDescriptor& desc,
SkGlyph* glyph,
SkGlyphCache* targetCache);
static bool DesperationSearchForPath(
const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path);
bool desperationSearchForPath(const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path);
static ExclusiveStrikePtr FindOrCreateStrikeExclusive(

7
src/core/SkTypeface_remote.cpp
View File

@ -19,11 +19,12 @@ SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf,
: SkScalerContext{std::move(tf), effects, desc}
, fDiscardableManager{std::move(manager)} {}
void SkScalerContextProxy::initCache(SkGlyphCache* cache) {
void SkScalerContextProxy::initCache(SkGlyphCache* cache, SkStrikeCache* strikeCache) {
SkASSERT(fCache == nullptr);
SkASSERT(cache != nullptr);
fCache = cache;
fStrikeCache = strikeCache;
}
unsigned SkScalerContextProxy::generateGlyphCount() {
@ -57,7 +58,7 @@ void SkScalerContextProxy::generateMetrics(SkGlyph* glyph) {
}
// Now check other caches for a desc mismatch.
if (SkStrikeCache::DesperationSearchForImage(fCache->getDescriptor(), glyph, fCache)) {
if (fStrikeCache->desperationSearchForImage(fCache->getDescriptor(), glyph, fCache)) {
fDiscardableManager->notifyCacheMiss(
SkStrikeClient::CacheMissType::kGlyphMetricsFallback);
return;
@ -88,7 +89,7 @@ bool SkScalerContextProxy::generatePath(SkGlyphID glyphID, SkPath* path) {
// Since the scaler context is being called, we don't have the needed data. Try to find a
// fallback before failing.
auto desc = SkScalerContext::DescriptorGivenRecAndEffects(this->getRec(), this->getEffects());
bool foundPath = SkStrikeCache::DesperationSearchForPath(*desc, glyphID, path);
bool foundPath = fStrikeCache->desperationSearchForPath(*desc, glyphID, path);
fDiscardableManager->notifyCacheMiss(foundPath
? SkStrikeClient::CacheMissType::kGlyphPathFallback
: SkStrikeClient::CacheMissType::kGlyphPath);

4
src/core/SkTypeface_remote.h
View File

@ -18,6 +18,7 @@
#include "SkTypeface.h"
class SkTypefaceProxy;
class SkStrikeCache;
class SkScalerContextProxy : public SkScalerContext {
public:
@ -26,7 +27,7 @@ public:
const SkDescriptor* desc,
sk_sp<SkStrikeClient::DiscardableHandleManager> manager);
void initCache(SkGlyphCache*);
void initCache(SkGlyphCache*, SkStrikeCache*);
protected:
unsigned generateGlyphCount() override;
@ -41,6 +42,7 @@ protected:
private:
sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
SkGlyphCache* fCache = nullptr;
SkStrikeCache* fStrikeCache = nullptr;
typedef SkScalerContext INHERITED;
};

27
tests/SkRemoteGlyphCacheTest.cpp
View File

@ -428,6 +428,8 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
auto lostGlyphID = SkPackedGlyphID(1, SK_FixedHalf, SK_FixedHalf);
const uint8_t glyphImage[] = {0xFF, 0xFF};
SkStrikeCache strikeCache;
// Build a fallback cache.
{
SkAutoDescriptor ad;
@ -437,7 +439,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *clientTf);
auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
glyph->fMaskFormat = SkMask::kA8_Format;
glyph->fHeight = 1;
@ -454,7 +456,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto testCache = SkStrikeCache::FindStrikeExclusive(*desc);
auto testCache = strikeCache.findStrikeExclusive(*desc);
REPORTER_ASSERT(reporter, !(testCache == nullptr));
}
@ -466,11 +468,12 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
SkScalerContextFlags flags = SkScalerContextFlags::kNone;
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
testCache = SkStrikeCache::FindStrikeExclusive(*desc);
testCache = strikeCache.findStrikeExclusive(*desc);
REPORTER_ASSERT(reporter, testCache == nullptr);
testCache = SkStrikeCache::CreateStrikeExclusive(*desc,
testCache = strikeCache.createStrikeExclusive(*desc,
clientTf->createScalerContext(effects, desc));
static_cast<SkScalerContextProxy*>(testCache->getScalerContext())->initCache(testCache.get());
auto scalerProxy = static_cast<SkScalerContextProxy*>(testCache->getScalerContext());
scalerProxy->initCache(testCache.get(), &strikeCache);
// Look for the lost glyph.
{
@ -504,7 +507,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(i) == 0);
}
}
SkStrikeCache::ValidateGlyphCacheDataSize();
strikeCache.validateGlyphCacheDataSize();
// Must unlock everything on termination, otherwise valgrind complains about memory leaks.
discardableManager->unlockAndDeleteAll();
@ -530,6 +533,8 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
uint32_t realMask;
uint32_t fakeMask;
SkStrikeCache strikeCache;
{
SkAutoDescriptor ad;
SkScalerContextRec rec;
@ -557,7 +562,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *clientTf);
auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
fakeMask = (realMask == SkMask::kA8_Format) ? SkMask::kBW_Format : SkMask::kA8_Format;
glyph->fMaskFormat = fakeMask;
@ -579,7 +584,9 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
std::vector<uint8_t> serverStrikeData;
server.writeStrikeData(&serverStrikeData);
REPORTER_ASSERT(reporter,
client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
client.readStrikeData(
serverStrikeData.data(),
serverStrikeData.size()));
}
{
@ -591,7 +598,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
auto fallbackCache = SkStrikeCache::FindStrikeExclusive(*desc);
auto fallbackCache = strikeCache.findStrikeExclusive(*desc);
REPORTER_ASSERT(reporter, fallbackCache.get() != nullptr);
auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
REPORTER_ASSERT(reporter, glyph->fMaskFormat == fakeMask);
@ -605,7 +612,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
memcmp(glyph->fImage, glyphImage, glyph->computeImageSize()) == 0);
}
SkStrikeCache::ValidateGlyphCacheDataSize();
strikeCache.validateGlyphCacheDataSize();
// Must unlock everything on termination, otherwise valgrind complains about memory leaks.
discardableManager->unlockAndDeleteAll();