Revert "fonts: Cleanup cache miss logging for font remoting."

This reverts commit 2b8a0d1844.

Reason for revert: reverting a CL this builds on

Original change's description:
> 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>

TBR=herb@google.com,khushalsagar@chromium.org

Change-Id: I8a331545988885c620685008f4b60240d80f3712
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 829622
Reviewed-on: https://skia-review.googlesource.com/129682
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2018-05-23 00:16:47 +00:00 committed by Skia Commit-Bot
parent e60ae82dec
commit fc6cf92e4b
5 changed files with 83 additions and 80 deletions

View File

@ -801,7 +801,40 @@ sk_sp<SkTypeface> SkStrikeClient::addTypeface(const WireTypeface& wire) {
if (typeface) return *typeface;
auto newTypeface = sk_make_sp<SkTypefaceProxy>(wire.typefaceID, wire.glyphCount, wire.style,
wire.isFixed, fDiscardableHandleManager);
wire.isFixed, this);
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");
}

View File

@ -189,14 +189,6 @@ private:
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:
@ -205,8 +197,6 @@ public:
// 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>);
@ -222,6 +212,19 @@ public:
// 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;

View File

@ -6,16 +6,15 @@
*/
#include "SkTypeface_remote.h"
#include "SkPaint.h"
#include "SkRemoteGlyphCache.h"
#include "SkStrikeCache.h"
#include "SkTraceEvent.h"
#include "SkPaint.h"
SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf,
const SkScalerContextEffects& effects,
const SkDescriptor* desc,
sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
: SkScalerContext{std::move(tf), effects, desc}, fDiscardableManager{std::move(manager)} {}
SkStrikeClient* rsc)
: SkScalerContext{std::move(tf), effects, desc}, fClient{rsc} {}
unsigned SkScalerContextProxy::generateGlyphCount() {
SK_ABORT("Should never be called.");
@ -32,34 +31,21 @@ void SkScalerContextProxy::generateAdvance(SkGlyph* glyph) {
}
void SkScalerContextProxy::generateMetrics(SkGlyph* 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();
fClient->generateMetricsAndImage(*this->typefaceProxy(), this->getRec(), &fAlloc, 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) {
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);
fClient->generatePath(*this->typefaceProxy(), this->getRec(), glyphID, path);
return false;
}
void SkScalerContextProxy::generateFontMetrics(SkPaint::FontMetrics* 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());
fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kFontMetrics);
sk_bzero(metrics, sizeof(*metrics));
fClient->generateFontMetrics(*this->typefaceProxy(), this->getRec(), metrics);
}
SkTypefaceProxy* SkScalerContextProxy::typefaceProxy() {
return SkTypefaceProxy::DownCast(this->getTypeface());
}

View File

@ -13,16 +13,18 @@
#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,
sk_sp<SkStrikeClient::DiscardableHandleManager> manager);
SkStrikeClient* rsc);
protected:
unsigned generateGlyphCount() override;
@ -40,8 +42,10 @@ private:
static constexpr size_t kMinGlyphImageSize = 16 /* height */ * 8 /* width */;
static constexpr size_t kMinAllocAmount = kMinGlyphImageSize * kMinGlyphCount;
SkTypefaceProxy* typefaceProxy();
SkArenaAlloc fAlloc{kMinAllocAmount};
sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
SkStrikeClient* const fClient;
typedef SkScalerContext INHERITED;
};
@ -51,13 +55,14 @@ public:
int glyphCount,
const SkFontStyle& style,
bool isFixed,
sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
: INHERITED{style, false}
, fFontId{fontId}
, fGlyphCount{glyphCount}
, fDiscardableManager{std::move(manager)} {}
SkStrikeClient* rsc)
: INHERITED{style, false}, fFontId{fontId}, fGlyphCount{glyphCount}, fRsc{rsc} {}
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; }
@ -92,11 +97,16 @@ protected:
SkScalerContext* onCreateScalerContext(const SkScalerContextEffects& effects,
const SkDescriptor* desc) const override {
return new SkScalerContextProxy(sk_ref_sp(const_cast<SkTypefaceProxy*>(this)), effects,
desc, fDiscardableManager);
desc, fRsc);
}
void onFilterRec(SkScalerContextRec* rec) const override {
// The rec filtering is already applied by the server when generating
// the glyphs.
// 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.
}
void onGetFontDescriptor(SkFontDescriptor*, bool*) const override {
SK_ABORT("Should never be called.");
@ -128,7 +138,8 @@ private:
const SkFontID fFontId;
const int fGlyphCount;
sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
// TODO: Does this need a ref to the strike client? If yes, make it a weak ref.
SkStrikeClient* const fRsc;
typedef SkTypeface INHERITED;
};

View File

@ -22,7 +22,7 @@
class DiscardableManager : public SkStrikeServer::DiscardableHandleManager,
public SkStrikeClient::DiscardableHandleManager {
public:
DiscardableManager() { sk_bzero(&fCacheMissCount, sizeof(fCacheMissCount)); }
DiscardableManager() = default;
~DiscardableManager() override = default;
// Server implementation.
@ -39,7 +39,6 @@ public:
// Client implementation.
bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
void NotifyCacheMiss(SkStrikeClient::CacheMissType type) override { fCacheMissCount[type]++; }
void unlockAll() { fLockedHandles.reset(); }
void unlockAndDeleteAll() {
@ -48,13 +47,11 @@ public:
}
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) {
@ -117,7 +114,7 @@ DEF_TEST(SkRemoteGlyphCache_TypefaceSerialization, reporter) {
auto client_tf = client.deserializeTypeface(tf_data->data(), tf_data->size());
REPORTER_ASSERT(reporter, client_tf);
REPORTER_ASSERT(reporter, static_cast<SkTypefaceProxy*>(client_tf.get())->remoteTypefaceID() ==
REPORTER_ASSERT(reporter, SkTypefaceProxy::DownCast(client_tf.get())->remoteTypefaceID() ==
server_tf->uniqueID());
}
@ -352,31 +349,4 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c
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