From 97f85bb7fd65fad0768578ea2ed3a931eb301381 Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Thu, 6 May 2021 08:59:26 -0400 Subject: [PATCH] Remove SkTextBlobDiffCanvas, use tracking device directly with base SkCanvas Chromium has been updated to use makeAnalysisCanvas directly and there are no more references to SkTextBlobDiffCanvas as a type in its code base. Since the GlyphTrackingDevice extends SkNoPixelsDevice, any SkCanvas that uses it is effectively a "no-draw" canvas. However, by returning a base SkCanvas the text tracking now automatically happens in the context of the base's AutoLayerForImageFilter handling it applies on every draw. This means that drawing a text blob with an image filter that modifies the transform state will now be analyzed in that context automatically (simplifying code in chrome after this lands). Another behavioral change is that all non-text draws will still go through the base SkCanvas' virtuals and invoke the device function. Since it's an SkNoPixelsDevice, it'll still be a no-op, it just happens a little later. This won't really impact performance because oop-r already inspects their operations and only plays back text and transform related ones to the analysis canvas, so we shouldn't really see non-text draws being invoked anyways. Bug: chromium:1187246 Change-Id: I83f86571300751f385b3065dfe889f218fa1edc6 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405196 Reviewed-by: Herb Derby Commit-Queue: Michael Ludwig --- bench/SkGlyphCacheBench.cpp | 5 +- include/core/SkFont.h | 1 - src/core/SkRemoteGlyphCache.cpp | 51 ++++-------------- src/core/SkRemoteGlyphCache.h | 28 +--------- tests/SkRemoteGlyphCacheTest.cpp | 90 +++++++++++++++++--------------- tools/remote_demo.cpp | 7 ++- 6 files changed, 65 insertions(+), 117 deletions(-) diff --git a/bench/SkGlyphCacheBench.cpp b/bench/SkGlyphCacheBench.cpp index dded96ef6f..d5351c1dd6 100644 --- a/bench/SkGlyphCacheBench.cpp +++ b/bench/SkGlyphCacheBench.cpp @@ -223,11 +223,12 @@ class DiffCanvasBench : public Benchmark { void onDraw(int loops, SkCanvas* modelCanvas) override { SkSurfaceProps props; if (modelCanvas) { modelCanvas->getProps(&props); } - SkTextBlobCacheDiffCanvas canvas{1024, 1024, props, fServer.get()}; + std::unique_ptr canvas = fServer->makeAnalysisCanvas(1024, 1024, props, + nullptr, true); loops *= 100; while (loops --> 0) { for (const auto& record : fTrace) { - canvas.drawTextBlob( + canvas->drawTextBlob( record.blob.get(), record.offset.x(), record.offset.y(),record.paint); } } diff --git a/include/core/SkFont.h b/include/core/SkFont.h index 42c91ae892..2bcc31632a 100644 --- a/include/core/SkFont.h +++ b/include/core/SkFont.h @@ -512,7 +512,6 @@ private: friend class SkFontPriv; friend class SkGlyphRunListPainter; - friend class SkTextBlobCacheDiffCanvas; friend class SkStrikeSpec; }; diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp index 76e6d9cbbf..0c268adf19 100644 --- a/src/core/SkRemoteGlyphCache.cpp +++ b/src/core/SkRemoteGlyphCache.cpp @@ -809,10 +809,10 @@ RemoteStrike* SkStrikeServerImpl::getOrCreateCache( return remoteStrikePtr; } -// -- TrackLayerDevice ----------------------------------------------------------------------------- -class SkTextBlobCacheDiffCanvas::TrackLayerDevice final : public SkNoPixelsDevice { +// -- GlyphTrackingDevice -------------------------------------------------------------------------- +class GlyphTrackingDevice final : public SkNoPixelsDevice { public: - TrackLayerDevice( + GlyphTrackingDevice( const SkISize& dimensions, const SkSurfaceProps& props, SkStrikeServerImpl* server, sk_sp colorSpace, bool DFTSupport) : SkNoPixelsDevice(SkIRect::MakeSize(dimensions), props, std::move(colorSpace)) @@ -824,8 +824,8 @@ public: SkBaseDevice* onCreateDevice(const CreateInfo& cinfo, const SkPaint*) override { const SkSurfaceProps surfaceProps(this->surfaceProps().flags(), cinfo.fPixelGeometry); - return new TrackLayerDevice(cinfo.fInfo.dimensions(), surfaceProps, fStrikeServerImpl, - cinfo.fInfo.refColorSpace(), fDFTSupport); + return new GlyphTrackingDevice(cinfo.fInfo.dimensions(), surfaceProps, fStrikeServerImpl, + cinfo.fInfo.refColorSpace(), fDFTSupport); } protected: @@ -857,40 +857,6 @@ private: SkGlyphRunListPainter fPainter; }; -// -- SkTextBlobCacheDiffCanvas ------------------------------------------------------------------- -SkTextBlobCacheDiffCanvas::SkTextBlobCacheDiffCanvas(int width, int height, - const SkSurfaceProps& props, - SkStrikeServer* strikeServer, - bool DFTSupport) - : SkTextBlobCacheDiffCanvas{width, height, props, strikeServer, nullptr, DFTSupport} { } - -SkTextBlobCacheDiffCanvas::SkTextBlobCacheDiffCanvas(int width, int height, - const SkSurfaceProps& props, - SkStrikeServer* strikeServer, - sk_sp colorSpace, - bool DFTSupport) - : SkNoDrawCanvas{sk_make_sp(SkISize::Make(width, height), - props, - strikeServer->impl(), - std::move(colorSpace), - DFTSupport)} { } - -SkTextBlobCacheDiffCanvas::~SkTextBlobCacheDiffCanvas() = default; - -SkCanvas::SaveLayerStrategy SkTextBlobCacheDiffCanvas::getSaveLayerStrategy( - const SaveLayerRec& rec) { - return kFullLayer_SaveLayerStrategy; -} - -bool SkTextBlobCacheDiffCanvas::onDoSaveBehind(const SkRect*) { - return false; -} - -void SkTextBlobCacheDiffCanvas::onDrawTextBlob(const SkTextBlob* blob, SkScalar x, SkScalar y, - const SkPaint& paint) { - SkCanvas::onDrawTextBlob(blob, x, y, paint); -} - // -- SkStrikeServer ------------------------------------------------------------------------------- SkStrikeServer::SkStrikeServer(DiscardableHandleManager* dhm) : fImpl(new SkStrikeServerImpl{dhm}) { } @@ -901,8 +867,11 @@ std::unique_ptr SkStrikeServer::makeAnalysisCanvas(int width, int heig const SkSurfaceProps& props, sk_sp colorSpace, bool DFTSupport) { - return std::make_unique(width, height, props, this, - std::move(colorSpace), DFTSupport); + sk_sp trackingDevice(new GlyphTrackingDevice(SkISize::Make(width, height), + props, this->impl(), + std::move(colorSpace), + DFTSupport)); + return std::make_unique(std::move(trackingDevice)); } sk_sp SkStrikeServer::serializeTypeface(SkTypeface* tf) { diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h index c896a8691f..f5c04902ff 100644 --- a/src/core/SkRemoteGlyphCache.h +++ b/src/core/SkRemoteGlyphCache.h @@ -28,31 +28,6 @@ class SkStrikeServer; class SkStrikeServerImpl; class SkTypeface; -// A SkTextBlobCacheDiffCanvas is used to populate the SkStrikeServer with ops -// which will be serialized and rendered using the SkStrikeClient. -class SkTextBlobCacheDiffCanvas : public SkNoDrawCanvas { -public: - - // For testing use only - SkTextBlobCacheDiffCanvas(int width, int height, const SkSurfaceProps& props, - SkStrikeServer* strikeServer, bool DFTSupport = true); - - SK_SPI SkTextBlobCacheDiffCanvas(int width, int height, const SkSurfaceProps& props, - SkStrikeServer* strikeServer, sk_sp colorSpace, - bool DFTSupport); - - SK_SPI ~SkTextBlobCacheDiffCanvas() override; - -protected: - SkCanvas::SaveLayerStrategy getSaveLayerStrategy(const SaveLayerRec& rec) override; - bool onDoSaveBehind(const SkRect*) override; - void onDrawTextBlob(const SkTextBlob* blob, SkScalar x, SkScalar y, - const SkPaint& paint) override; - -private: - class TrackLayerDevice; -}; - using SkDiscardableHandleId = uint32_t; // This class is not thread-safe. class SkStrikeServer { @@ -94,7 +69,7 @@ public: // Serializes the typeface to be transmitted using this server. SK_SPI sk_sp serializeTypeface(SkTypeface*); - // Serializes the strike data captured using a SkTextBlobCacheDiffCanvas. Any + // Serializes the strike data captured using a canvas returned by ::makeAnalysisCanvas. Any // handles locked using the DiscardableHandleManager will be assumed to be // unlocked after this call. SK_SPI void writeStrikeData(std::vector* memory); @@ -104,7 +79,6 @@ public: size_t remoteStrikeMapSizeForTesting() const; private: - friend class SkTextBlobCacheDiffCanvas; SkStrikeServerImpl* impl(); std::unique_ptr fImpl; diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp index 85b495ef95..df56e707ff 100644 --- a/tests/SkRemoteGlyphCacheTest.cpp +++ b/tests/SkRemoteGlyphCacheTest.cpp @@ -213,9 +213,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_StrikeSerialization, repor int glyphCount = 10; auto serverBlob = buildTextBlob(serverTf, glyphCount); auto props = FindSurfaceProps(dContext); - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server, - dContext->supportsDistanceFieldText()); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, dContext->supportsDistanceFieldText()); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -250,9 +250,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_ReleaseTypeFace, reporter, int glyphCount = 10; auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 10, 10, props, &server, ctxInfo.directContext()->supportsDistanceFieldText()); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, ctxInfo.directContext()->supportsDistanceFieldText()); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); REPORTER_ASSERT(reporter, !serverTf->unique()); std::vector serverStrikeData; @@ -275,9 +275,10 @@ DEF_TEST(SkRemoteGlyphCache_StrikeLockingServer, reporter) { auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server); + std::unique_ptr cache_diff_canvas = + server.makeAnalysisCanvas(10, 10, props, nullptr, true); SkPaint paint; - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); // The strike from the blob should be locked after it has been drawn on the canvas. REPORTER_ASSERT(reporter, discardableManager->handleCount() == 1u); @@ -290,7 +291,7 @@ DEF_TEST(SkRemoteGlyphCache_StrikeLockingServer, reporter) { discardableManager->unlockAll(); REPORTER_ASSERT(reporter, discardableManager->lockedHandles().count() == 0u); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); REPORTER_ASSERT(reporter, discardableManager->handleCount() == 1u); REPORTER_ASSERT(reporter, discardableManager->lockedHandles().count() == 1u); @@ -309,9 +310,10 @@ DEF_TEST(SkRemoteGlyphCache_StrikeDeletionServer, reporter) { auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server); + std::unique_ptr cache_diff_canvas = + server.makeAnalysisCanvas(10, 10, props, nullptr, true); SkPaint paint; - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); REPORTER_ASSERT(reporter, discardableManager->handleCount() == 1u); // Write the strike data and delete all the handles. Re-analyzing the blob should create new @@ -321,12 +323,12 @@ DEF_TEST(SkRemoteGlyphCache_StrikeDeletionServer, reporter) { // Another analysis pass, to ensure that deleting handles after a complete cache hit still // works. This is a regression test for crbug.com/999682. - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); server.writeStrikeData(&fontData); REPORTER_ASSERT(reporter, discardableManager->handleCount() == 1u); discardableManager->unlockAndDeleteAll(); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); REPORTER_ASSERT(reporter, discardableManager->handleCount() == 2u); // Must unlock everything on termination, otherwise valgrind complains about memory leaks. @@ -346,9 +348,10 @@ DEF_TEST(SkRemoteGlyphCache_StrikePinningClient, reporter) { auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server); + std::unique_ptr cache_diff_canvas = + server.makeAnalysisCanvas(10, 10, props, nullptr, true); SkPaint paint; - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -385,9 +388,10 @@ DEF_TEST(SkRemoteGlyphCache_ClientMemoryAccounting, reporter) { auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server); + std::unique_ptr cache_diff_canvas = + server.makeAnalysisCanvas(10, 10, props, nullptr, true); SkPaint paint; - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -412,10 +416,11 @@ DEF_TEST(SkRemoteGlyphCache_PurgesServerEntries, reporter) { auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server); + std::unique_ptr cache_diff_canvas = + server.makeAnalysisCanvas(10, 10, props, nullptr, true); SkPaint paint; REPORTER_ASSERT(reporter, server.remoteStrikeMapSizeForTesting() == 0u); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); REPORTER_ASSERT(reporter, server.remoteStrikeMapSizeForTesting() == 1u); } @@ -433,10 +438,11 @@ DEF_TEST(SkRemoteGlyphCache_PurgesServerEntries, reporter) { auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, props, &server); + std::unique_ptr cache_diff_canvas = + server.makeAnalysisCanvas(10, 10, props, nullptr, true); SkPaint paint; REPORTER_ASSERT(reporter, server.remoteStrikeMapSizeForTesting() == 1u); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); REPORTER_ASSERT(reporter, server.remoteStrikeMapSizeForTesting() == 1u); } @@ -462,9 +468,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsPath, reporter, int glyphCount = 10; auto serverBlob = buildTextBlob(serverTf, glyphCount); auto props = FindSurfaceProps(direct); - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 10, 10, props, &server, direct->supportsDistanceFieldText()); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, direct->supportsDistanceFieldText()); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -538,9 +544,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsMaskWithPathFall auto serverBlob = make_blob_causing_fallback(serverTf, serverTf.get(), reporter); auto props = FindSurfaceProps(direct); - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 10, 10, props, &server, direct->supportsDistanceFieldText()); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, direct->supportsDistanceFieldText()); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -609,9 +615,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsSDFTWithAllARGBF auto serverBlob = makeBlob(serverTf); auto props = FindSurfaceProps(ctxInfo.grContext()); - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 800, 800, props, &server, ctxInfo.grContext()->supportsDistanceFieldText()); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 400, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, ctxInfo.directContext()->supportsDistanceFieldText()); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 400, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -651,9 +657,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextXY, reporter, ctxI int glyphCount = 10; auto serverBlob = buildTextBlob(serverTf, glyphCount); auto props = FindSurfaceProps(direct); - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 10, 10, props, &server, direct->supportsDistanceFieldText()); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0.5, 0, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, direct->supportsDistanceFieldText()); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0.5, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -693,10 +699,10 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c int glyphCount = 10; auto serverBlob = buildTextBlob(serverTf, glyphCount); const SkSurfaceProps props; - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 10, 10, props, &server, direct->supportsDistanceFieldText()); - cache_diff_canvas.concat(matrix); - cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 10, 10, props, nullptr, direct->supportsDistanceFieldText()); + cache_diff_canvas->concat(matrix); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 0, 0, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -781,13 +787,14 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_TypefaceWithNoPaths, repor auto serverTfData = server.serializeTypeface(serverTf.get()); auto clientTf = client.deserializeTypeface(serverTfData->data(), serverTfData->size()); + auto props = FindSurfaceProps(direct); + std::unique_ptr cache_diff_canvas = server.makeAnalysisCanvas( + 500, 500, props, nullptr, direct->supportsDistanceFieldText()); for (SkScalar textSize : { 70, 180, 270, 340}) { auto serverBlob = MakeEmojiBlob(serverTf, textSize); - auto props = FindSurfaceProps(direct); - SkTextBlobCacheDiffCanvas cache_diff_canvas( - 500, 500, props, &server, direct->supportsDistanceFieldText()); + SkPaint paint; - cache_diff_canvas.drawTextBlob(serverBlob.get(), 100, 100, paint); + cache_diff_canvas->drawTextBlob(serverBlob.get(), 100, 100, paint); std::vector serverStrikeData; server.writeStrikeData(&serverStrikeData); @@ -807,4 +814,3 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_TypefaceWithNoPaths, repor // Must unlock everything on termination, otherwise valgrind complains about memory leaks. discardableManager->unlockAndDeleteAll(); } - diff --git a/tools/remote_demo.cpp b/tools/remote_demo.cpp index 3dc6216f3a..23aeb539dc 100644 --- a/tools/remote_demo.cpp +++ b/tools/remote_demo.cpp @@ -136,9 +136,9 @@ static bool push_font_data(const SkPicture& pic, SkStrikeServer* strikeServer, sk_sp colorSpace, int writeFd) { const SkIRect bounds = pic.cullRect().round(); const SkSurfaceProps props(0, kRGB_H_SkPixelGeometry); - SkTextBlobCacheDiffCanvas filter(bounds.width(), bounds.height(), props, - strikeServer, std::move(colorSpace), true); - pic.playback(&filter); + std::unique_ptr filter = strikeServer->makeAnalysisCanvas( + bounds.width(), bounds.height(), props, std::move(colorSpace), true); + pic.playback(filter.get()); std::vector fontData; strikeServer->writeStrikeData(&fontData); @@ -324,4 +324,3 @@ int main(int argc, char** argv) { return 0; } -