Reland fonts: Cleanup cache miss logging for font remoting.

This reverts commit fc6cf92e4b.

TBR=herb@google.com
Bug: skia:7913
Change-Id: I93301e49839fdb895a5f1f5845bb9975981c69fc
Reviewed-on: https://skia-review.googlesource.com/129880
Reviewed-by: Khusal Sagar <khushalsagar@chromium.org>
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>
This commit is contained in:
Khushal 2018-05-23 18:16:00 -07:00 committed by Skia Commit-Bot
parent 3e7548ca60
commit d416083eef
5 changed files with 82 additions and 82 deletions

View File

@ -801,40 +801,7 @@ sk_sp<SkTypeface> SkStrikeClient::addTypeface(const WireTypeface& wire) {
if (typeface) return *typeface; if (typeface) return *typeface;
auto newTypeface = sk_make_sp<SkTypefaceProxy>(wire.typefaceID, wire.glyphCount, wire.style, auto newTypeface = sk_make_sp<SkTypefaceProxy>(wire.typefaceID, wire.glyphCount, wire.style,
wire.isFixed, this); wire.isFixed, fDiscardableHandleManager);
fRemoteFontIdToTypeface.set(wire.typefaceID, newTypeface); fRemoteFontIdToTypeface.set(wire.typefaceID, newTypeface);
return std::move(newTypeface); return std::move(newTypeface);
} }
void SkStrikeClient::generateFontMetrics(const SkTypefaceProxy& typefaceProxy,
const SkScalerContextRec& rec,
SkPaint::FontMetrics* metrics) {
TRACE_EVENT1("skia", "generateFontMetrics", "rec", TRACE_STR_COPY(rec.dump().c_str()));
SkDebugf("generateFontMetrics: %s\n", rec.dump().c_str());
SkStrikeCache::Dump();
SkDEBUGFAIL("GlyphCacheMiss");
sk_bzero(metrics, sizeof(*metrics));
}
void SkStrikeClient::generateMetricsAndImage(const SkTypefaceProxy& typefaceProxy,
const SkScalerContextRec& rec,
SkArenaAlloc* alloc,
SkGlyph* glyph) {
TRACE_EVENT1("skia", "generateMetricsAndImage", "rec", TRACE_STR_COPY(rec.dump().c_str()));
SkDebugf("generateMetricsAndImage: %s\n", rec.dump().c_str());
SkStrikeCache::Dump();
SkDEBUGFAIL("GlyphCacheMiss");
glyph->zeroMetrics();
}
void SkStrikeClient::generatePath(const SkTypefaceProxy& typefaceProxy,
const SkScalerContextRec& rec,
SkGlyphID glyphID,
SkPath* path) {
TRACE_EVENT1("skia", "generatePath", "rec", TRACE_STR_COPY(rec.dump().c_str()));
SkDebugf("generatePath: %s\n", rec.dump().c_str());
SkStrikeCache::Dump();
SkDEBUGFAIL("GlyphCacheMiss");
}

View File

@ -189,6 +189,14 @@ private:
class SK_API SkStrikeClient { class SK_API SkStrikeClient {
public: public:
enum CacheMissType : uint32_t {
kFontMetrics,
kGlyphMetrics,
kGlyphImage,
kGlyphPath,
kLast = kGlyphPath
};
// An interface to delete handles that may be pinned by the remote server. // An interface to delete handles that may be pinned by the remote server.
class DiscardableHandleManager : public SkRefCnt { class DiscardableHandleManager : public SkRefCnt {
public: public:
@ -197,6 +205,8 @@ public:
// Returns true if the handle was unlocked and can be safely deleted. Once // Returns true if the handle was unlocked and can be safely deleted. Once
// successful, subsequent attempts to delete the same handle are invalid. // successful, subsequent attempts to delete the same handle are invalid.
virtual bool deleteHandle(SkDiscardableHandleId) = 0; virtual bool deleteHandle(SkDiscardableHandleId) = 0;
virtual void NotifyCacheMiss(CacheMissType) {}
}; };
SkStrikeClient(sk_sp<DiscardableHandleManager>); SkStrikeClient(sk_sp<DiscardableHandleManager>);
@ -212,19 +222,6 @@ public:
// Returns false if the data is invalid. // Returns false if the data is invalid.
bool readStrikeData(const volatile void* memory, size_t memorySize); bool readStrikeData(const volatile void* memory, size_t memorySize);
// TODO: Remove these since we don't support pulling this data on-demand.
void generateFontMetrics(const SkTypefaceProxy& typefaceProxy,
const SkScalerContextRec& rec,
SkPaint::FontMetrics* metrics);
void generateMetricsAndImage(const SkTypefaceProxy& typefaceProxy,
const SkScalerContextRec& rec,
SkArenaAlloc* alloc,
SkGlyph* glyph);
void generatePath(const SkTypefaceProxy& typefaceProxy,
const SkScalerContextRec& rec,
SkGlyphID glyphID,
SkPath* path);
private: private:
class DiscardableStrikePinner; class DiscardableStrikePinner;

View File

@ -6,15 +6,16 @@
*/ */
#include "SkTypeface_remote.h" #include "SkTypeface_remote.h"
#include "SkRemoteGlyphCache.h"
#include "SkPaint.h" #include "SkPaint.h"
#include "SkRemoteGlyphCache.h"
#include "SkStrikeCache.h"
#include "SkTraceEvent.h"
SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf, SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf,
const SkScalerContextEffects& effects, const SkScalerContextEffects& effects,
const SkDescriptor* desc, const SkDescriptor* desc,
SkStrikeClient* rsc) sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
: SkScalerContext{std::move(tf), effects, desc}, fClient{rsc} {} : SkScalerContext{std::move(tf), effects, desc}, fDiscardableManager{std::move(manager)} {}
unsigned SkScalerContextProxy::generateGlyphCount() { unsigned SkScalerContextProxy::generateGlyphCount() {
SK_ABORT("Should never be called."); SK_ABORT("Should never be called.");
@ -31,21 +32,34 @@ void SkScalerContextProxy::generateAdvance(SkGlyph* glyph) {
} }
void SkScalerContextProxy::generateMetrics(SkGlyph* glyph) { void SkScalerContextProxy::generateMetrics(SkGlyph* glyph) {
fClient->generateMetricsAndImage(*this->typefaceProxy(), this->getRec(), &fAlloc, glyph); TRACE_EVENT1("skia", "generateMetrics", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
SkDebugf("GlyphCacheMiss generateMetrics: %s\n", this->getRec().dump().c_str());
fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphMetrics);
glyph->zeroMetrics();
} }
void SkScalerContextProxy::generateImage(const SkGlyph& glyph) { void SkScalerContextProxy::generateImage(const SkGlyph& glyph) {
TRACE_EVENT1("skia", "generateImage", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
SkDebugf("GlyphCacheMiss generateImage: %s\n", this->getRec().dump().c_str());
fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphImage);
} }
bool SkScalerContextProxy::generatePath(SkGlyphID glyphID, SkPath* path) { bool SkScalerContextProxy::generatePath(SkGlyphID glyphID, SkPath* path) {
fClient->generatePath(*this->typefaceProxy(), this->getRec(), glyphID, path); TRACE_EVENT1("skia", "generatePath", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
SkDebugf("GlyphCacheMiss generatePath: %s\n", this->getRec().dump().c_str());
fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphPath);
return false; return false;
} }
void SkScalerContextProxy::generateFontMetrics(SkPaint::FontMetrics* metrics) { void SkScalerContextProxy::generateFontMetrics(SkPaint::FontMetrics* metrics) {
fClient->generateFontMetrics(*this->typefaceProxy(), this->getRec(), metrics); TRACE_EVENT1(
} "skia", "generateFontMetrics", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
SkDebugf("GlyphCacheMiss generateFontMetrics: %s\n", this->getRec().dump().c_str());
SkDEBUGCODE(SkStrikeCache::Dump());
SkTypefaceProxy* SkScalerContextProxy::typefaceProxy() { fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kFontMetrics);
return SkTypefaceProxy::DownCast(this->getTypeface()); sk_bzero(metrics, sizeof(*metrics));
} }

View File

@ -13,18 +13,16 @@
#include "SkFontDescriptor.h" #include "SkFontDescriptor.h"
#include "SkFontStyle.h" #include "SkFontStyle.h"
#include "SkPaint.h" #include "SkPaint.h"
#include "SkRemoteGlyphCache.h"
#include "SkScalerContext.h" #include "SkScalerContext.h"
#include "SkTypeface.h" #include "SkTypeface.h"
class SkStrikeClient;
class SkTypefaceProxy;
class SkScalerContextProxy : public SkScalerContext { class SkScalerContextProxy : public SkScalerContext {
public: public:
SkScalerContextProxy(sk_sp<SkTypeface> tf, SkScalerContextProxy(sk_sp<SkTypeface> tf,
const SkScalerContextEffects& effects, const SkScalerContextEffects& effects,
const SkDescriptor* desc, const SkDescriptor* desc,
SkStrikeClient* rsc); sk_sp<SkStrikeClient::DiscardableHandleManager> manager);
protected: protected:
unsigned generateGlyphCount() override; unsigned generateGlyphCount() override;
@ -42,10 +40,8 @@ private:
static constexpr size_t kMinGlyphImageSize = 16 /* height */ * 8 /* width */; static constexpr size_t kMinGlyphImageSize = 16 /* height */ * 8 /* width */;
static constexpr size_t kMinAllocAmount = kMinGlyphImageSize * kMinGlyphCount; static constexpr size_t kMinAllocAmount = kMinGlyphImageSize * kMinGlyphCount;
SkTypefaceProxy* typefaceProxy();
SkArenaAlloc fAlloc{kMinAllocAmount}; SkArenaAlloc fAlloc{kMinAllocAmount};
SkStrikeClient* const fClient; sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
typedef SkScalerContext INHERITED; typedef SkScalerContext INHERITED;
}; };
@ -55,14 +51,13 @@ public:
int glyphCount, int glyphCount,
const SkFontStyle& style, const SkFontStyle& style,
bool isFixed, bool isFixed,
SkStrikeClient* rsc) sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
: INHERITED{style, false}, fFontId{fontId}, fGlyphCount{glyphCount}, fRsc{rsc} {} : INHERITED{style, false}
, fFontId{fontId}
, fGlyphCount{glyphCount}
, fDiscardableManager{std::move(manager)} {}
SkFontID remoteTypefaceID() const {return fFontId;} SkFontID remoteTypefaceID() const {return fFontId;}
int glyphCount() const {return fGlyphCount;} int glyphCount() const {return fGlyphCount;}
static SkTypefaceProxy* DownCast(SkTypeface* typeface) {
// TODO: how to check the safety of the down cast?
return (SkTypefaceProxy*)typeface;
}
protected: protected:
int onGetUPEM() const override { SK_ABORT("Should never be called."); return 0; } int onGetUPEM() const override { SK_ABORT("Should never be called."); return 0; }
@ -97,16 +92,11 @@ protected:
SkScalerContext* onCreateScalerContext(const SkScalerContextEffects& effects, SkScalerContext* onCreateScalerContext(const SkScalerContextEffects& effects,
const SkDescriptor* desc) const override { const SkDescriptor* desc) const override {
return new SkScalerContextProxy(sk_ref_sp(const_cast<SkTypefaceProxy*>(this)), effects, return new SkScalerContextProxy(sk_ref_sp(const_cast<SkTypefaceProxy*>(this)), effects,
desc, fRsc); desc, fDiscardableManager);
} }
void onFilterRec(SkScalerContextRec* rec) const override { void onFilterRec(SkScalerContextRec* rec) const override {
// Add all the device information here. // The rec filtering is already applied by the server when generating
//rec->fPost2x2[0][0] = 0.5f; // the glyphs.
// This would be the best place to run the host SkTypeface_* onFilterRec.
// Can we move onFilterRec to the FongMgr, that way we don't need to cross the boundary to
// filter.
} }
void onGetFontDescriptor(SkFontDescriptor*, bool*) const override { void onGetFontDescriptor(SkFontDescriptor*, bool*) const override {
SK_ABORT("Should never be called."); SK_ABORT("Should never be called.");
@ -138,8 +128,7 @@ private:
const SkFontID fFontId; const SkFontID fFontId;
const int fGlyphCount; const int fGlyphCount;
// TODO: Does this need a ref to the strike client? If yes, make it a weak ref. sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
SkStrikeClient* const fRsc;
typedef SkTypeface INHERITED; typedef SkTypeface INHERITED;
}; };

View File

@ -22,7 +22,7 @@
class DiscardableManager : public SkStrikeServer::DiscardableHandleManager, class DiscardableManager : public SkStrikeServer::DiscardableHandleManager,
public SkStrikeClient::DiscardableHandleManager { public SkStrikeClient::DiscardableHandleManager {
public: public:
DiscardableManager() = default; DiscardableManager() { sk_bzero(&fCacheMissCount, sizeof(fCacheMissCount)); }
~DiscardableManager() override = default; ~DiscardableManager() override = default;
// Server implementation. // Server implementation.
@ -39,6 +39,7 @@ public:
// Client implementation. // Client implementation.
bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; } bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
void NotifyCacheMiss(SkStrikeClient::CacheMissType type) override { fCacheMissCount[type]++; }
void unlockAll() { fLockedHandles.reset(); } void unlockAll() { fLockedHandles.reset(); }
void unlockAndDeleteAll() { void unlockAndDeleteAll() {
@ -47,11 +48,13 @@ public:
} }
const SkTHashSet<SkDiscardableHandleId>& lockedHandles() const { return fLockedHandles; } const SkTHashSet<SkDiscardableHandleId>& lockedHandles() const { return fLockedHandles; }
SkDiscardableHandleId handleCount() { return fNextHandleId; } SkDiscardableHandleId handleCount() { return fNextHandleId; }
int cacheMissCount(SkStrikeClient::CacheMissType type) { return fCacheMissCount[type]; }
private: private:
SkDiscardableHandleId fNextHandleId = 0u; SkDiscardableHandleId fNextHandleId = 0u;
SkDiscardableHandleId fLastDeletedHandleId = 0u; SkDiscardableHandleId fLastDeletedHandleId = 0u;
SkTHashSet<SkDiscardableHandleId> fLockedHandles; SkTHashSet<SkDiscardableHandleId> fLockedHandles;
int fCacheMissCount[SkStrikeClient::CacheMissType::kLast + 1u];
}; };
sk_sp<SkTextBlob> buildTextBlob(sk_sp<SkTypeface> tf, int glyphCount) { sk_sp<SkTextBlob> buildTextBlob(sk_sp<SkTypeface> tf, int glyphCount) {
@ -114,7 +117,7 @@ DEF_TEST(SkRemoteGlyphCache_TypefaceSerialization, reporter) {
auto client_tf = client.deserializeTypeface(tf_data->data(), tf_data->size()); auto client_tf = client.deserializeTypeface(tf_data->data(), tf_data->size());
REPORTER_ASSERT(reporter, client_tf); REPORTER_ASSERT(reporter, client_tf);
REPORTER_ASSERT(reporter, SkTypefaceProxy::DownCast(client_tf.get())->remoteTypefaceID() == REPORTER_ASSERT(reporter, static_cast<SkTypefaceProxy*>(client_tf.get())->remoteTypefaceID() ==
server_tf->uniqueID()); server_tf->uniqueID());
// Must unlock everything on termination, otherwise valgrind complains about memory leaks. // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
@ -373,4 +376,34 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c
// Must unlock everything on termination, otherwise valgrind complains about memory leaks. // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
discardableManager->unlockAndDeleteAll(); discardableManager->unlockAndDeleteAll();
} }
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_CacheMissReporting, reporter, ctxInfo) {
sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
SkStrikeServer server(discardableManager.get());
SkStrikeClient client(discardableManager);
auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
auto tfData = server.serializeTypeface(serverTf.get());
auto clientTf = client.deserializeTypeface(tfData->data(), tfData->size());
REPORTER_ASSERT(reporter, clientTf);
int glyphCount = 10;
auto clientBlob = buildTextBlob(clientTf, glyphCount);
// Raster the client-side blob without the glyph data, we should get cache miss notifications.
SkPaint paint;
SkMatrix matrix = SkMatrix::I();
RasterBlob(clientBlob, 10, 10, paint, ctxInfo.grContext(), &matrix);
REPORTER_ASSERT(reporter,
discardableManager->cacheMissCount(SkStrikeClient::kFontMetrics) == 1);
REPORTER_ASSERT(reporter,
discardableManager->cacheMissCount(SkStrikeClient::kGlyphMetrics) == 10);
// There shouldn't be any image or path requests, since we mark the glyph as empty on a cache
// miss.
REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(SkStrikeClient::kGlyphImage) == 0);
REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(SkStrikeClient::kGlyphPath) == 0);
// Must unlock everything on termination, otherwise valgrind complains about memory leaks.
discardableManager->unlockAndDeleteAll();
}
#endif #endif