fonts: Cleanup cache miss logging for font remoting.
Add hooks to notify the embedder if there is a cache miss during draw.
Also remove the reference to SkStrikeClient from SkTypefaceProxy and
SkScalerContextProxy, since the proxies can outlive the client.
R=herb@google.com
Bug: 829622
Change-Id: Ib2fd1b91ebd057856c1d4e717cf50b49f08c903b
Reviewed-on: https://skia-review.googlesource.com/129402
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>
Reviewed-by: Herb Derby <herb@google.com>
diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index d5e96fc..216a2a9d 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -801,40 +801,7 @@
if (typeface) return *typeface;
auto newTypeface = sk_make_sp<SkTypefaceProxy>(wire.typefaceID, wire.glyphCount, wire.style,
- wire.isFixed, this);
+ wire.isFixed, fDiscardableHandleManager);
fRemoteFontIdToTypeface.set(wire.typefaceID, 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");
-}
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index d1119cf..32d0b47 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -189,6 +189,14 @@
class SK_API SkStrikeClient {
public:
+ enum CacheMissType : uint32_t {
+ kFontMetrics,
+ kGlyphMetrics,
+ kGlyphImage,
+ kGlyphPath,
+ kLast = kGlyphPath
+ };
+
// An interface to delete handles that may be pinned by the remote server.
class DiscardableHandleManager : public SkRefCnt {
public:
@@ -197,6 +205,8 @@
// Returns true if the handle was unlocked and can be safely deleted. Once
// successful, subsequent attempts to delete the same handle are invalid.
virtual bool deleteHandle(SkDiscardableHandleId) = 0;
+
+ virtual void NotifyCacheMiss(CacheMissType) {}
};
SkStrikeClient(sk_sp<DiscardableHandleManager>);
@@ -212,19 +222,6 @@
// Returns false if the data is invalid.
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:
class DiscardableStrikePinner;
diff --git a/src/core/SkTypeface_remote.cpp b/src/core/SkTypeface_remote.cpp
index fe3fde8..cbddb1f 100644
--- a/src/core/SkTypeface_remote.cpp
+++ b/src/core/SkTypeface_remote.cpp
@@ -6,15 +6,16 @@
*/
#include "SkTypeface_remote.h"
-#include "SkRemoteGlyphCache.h"
-
#include "SkPaint.h"
+#include "SkRemoteGlyphCache.h"
+#include "SkStrikeCache.h"
+#include "SkTraceEvent.h"
SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf,
const SkScalerContextEffects& effects,
const SkDescriptor* desc,
- SkStrikeClient* rsc)
- : SkScalerContext{std::move(tf), effects, desc}, fClient{rsc} {}
+ sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
+ : SkScalerContext{std::move(tf), effects, desc}, fDiscardableManager{std::move(manager)} {}
unsigned SkScalerContextProxy::generateGlyphCount() {
SK_ABORT("Should never be called.");
@@ -31,21 +32,34 @@
}
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) {
+ 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) {
- 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;
}
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() {
- return SkTypefaceProxy::DownCast(this->getTypeface());
+ fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kFontMetrics);
+ sk_bzero(metrics, sizeof(*metrics));
}
diff --git a/src/core/SkTypeface_remote.h b/src/core/SkTypeface_remote.h
index ad729ad..ff5bc66 100644
--- a/src/core/SkTypeface_remote.h
+++ b/src/core/SkTypeface_remote.h
@@ -13,18 +13,16 @@
#include "SkFontDescriptor.h"
#include "SkFontStyle.h"
#include "SkPaint.h"
+#include "SkRemoteGlyphCache.h"
#include "SkScalerContext.h"
#include "SkTypeface.h"
-class SkStrikeClient;
-class SkTypefaceProxy;
-
class SkScalerContextProxy : public SkScalerContext {
public:
SkScalerContextProxy(sk_sp<SkTypeface> tf,
const SkScalerContextEffects& effects,
const SkDescriptor* desc,
- SkStrikeClient* rsc);
+ sk_sp<SkStrikeClient::DiscardableHandleManager> manager);
protected:
unsigned generateGlyphCount() override;
@@ -42,10 +40,8 @@
static constexpr size_t kMinGlyphImageSize = 16 /* height */ * 8 /* width */;
static constexpr size_t kMinAllocAmount = kMinGlyphImageSize * kMinGlyphCount;
- SkTypefaceProxy* typefaceProxy();
-
SkArenaAlloc fAlloc{kMinAllocAmount};
- SkStrikeClient* const fClient;
+ sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
typedef SkScalerContext INHERITED;
};
@@ -55,14 +51,13 @@
int glyphCount,
const SkFontStyle& style,
bool isFixed,
- SkStrikeClient* rsc)
- : INHERITED{style, false}, fFontId{fontId}, fGlyphCount{glyphCount}, fRsc{rsc} {}
+ sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
+ : INHERITED{style, false}
+ , fFontId{fontId}
+ , fGlyphCount{glyphCount}
+ , fDiscardableManager{std::move(manager)} {}
SkFontID remoteTypefaceID() const {return fFontId;}
int glyphCount() const {return fGlyphCount;}
- static SkTypefaceProxy* DownCast(SkTypeface* typeface) {
- // TODO: how to check the safety of the down cast?
- return (SkTypefaceProxy*)typeface;
- }
protected:
int onGetUPEM() const override { SK_ABORT("Should never be called."); return 0; }
@@ -97,16 +92,11 @@
SkScalerContext* onCreateScalerContext(const SkScalerContextEffects& effects,
const SkDescriptor* desc) const override {
return new SkScalerContextProxy(sk_ref_sp(const_cast<SkTypefaceProxy*>(this)), effects,
- desc, fRsc);
-
+ desc, fDiscardableManager);
}
void onFilterRec(SkScalerContextRec* rec) const override {
- // Add all the device information here.
- //rec->fPost2x2[0][0] = 0.5f;
-
- // 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.
+ // The rec filtering is already applied by the server when generating
+ // the glyphs.
}
void onGetFontDescriptor(SkFontDescriptor*, bool*) const override {
SK_ABORT("Should never be called.");
@@ -138,8 +128,7 @@
const SkFontID fFontId;
const int fGlyphCount;
- // TODO: Does this need a ref to the strike client? If yes, make it a weak ref.
- SkStrikeClient* const fRsc;
+ sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
typedef SkTypeface INHERITED;
};
diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index 5d4b9ff..6a4a4d3 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -22,7 +22,7 @@
class DiscardableManager : public SkStrikeServer::DiscardableHandleManager,
public SkStrikeClient::DiscardableHandleManager {
public:
- DiscardableManager() = default;
+ DiscardableManager() { sk_bzero(&fCacheMissCount, sizeof(fCacheMissCount)); }
~DiscardableManager() override = default;
// Server implementation.
@@ -39,6 +39,7 @@
// Client implementation.
bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
+ void NotifyCacheMiss(SkStrikeClient::CacheMissType type) override { fCacheMissCount[type]++; }
void unlockAll() { fLockedHandles.reset(); }
void unlockAndDeleteAll() {
@@ -47,11 +48,13 @@
}
const SkTHashSet<SkDiscardableHandleId>& lockedHandles() const { return fLockedHandles; }
SkDiscardableHandleId handleCount() { return fNextHandleId; }
+ int cacheMissCount(SkStrikeClient::CacheMissType type) { return fCacheMissCount[type]; }
private:
SkDiscardableHandleId fNextHandleId = 0u;
SkDiscardableHandleId fLastDeletedHandleId = 0u;
SkTHashSet<SkDiscardableHandleId> fLockedHandles;
+ int fCacheMissCount[SkStrikeClient::CacheMissType::kLast + 1u];
};
sk_sp<SkTextBlob> buildTextBlob(sk_sp<SkTypeface> tf, int glyphCount) {
@@ -114,7 +117,7 @@
auto client_tf = client.deserializeTypeface(tf_data->data(), tf_data->size());
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());
}
@@ -349,4 +352,31 @@
COMPARE_BLOBS(expected, actual, reporter);
SkStrikeCache::Validate();
}
+
+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);
+}
#endif
