Revert "fonts: Use correct SurfaceProps in analysis canvas for remoting."

This reverts commit 4f078f7cfac6580dce6e213c022708922739ac6d.

Reason for revert: Seems to have caused these breakages:
* https://chromium-swarm.appspot.com/task?id=3dc6788dc7b77010&refresh=10
* https://chromium-swarm.appspot.com/task?id=3dc67452db797c10&refresh=10

Original change's description:
> fonts: Use correct SurfaceProps in analysis canvas for remoting.
> 
> SaveLayer may not preserve the behaviour for lcd text, in which case
> the surfaceProps used for desc generation are inconsistent.
> 
> R=​herb@google.com
> 
> Bug: 829622
> Change-Id: I3adfc6780f26e4eb333a8aff76eaa4b5a9f0a0a7
> Reviewed-on: https://skia-review.googlesource.com/129557
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>

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

Change-Id: I0372a3d834f8c4c929feb71c2b6b13739443eaaa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 829622
Reviewed-on: https://skia-review.googlesource.com/130680
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>

3 files changed, 28 insertions, 104 deletions

diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index f9c38dd11e..a987e5bdde 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -198,6 +198,7 @@ SkTextBlobCacheDiffCanvas::SkTextBlobCacheDiffCanvas(int width, int height,
                                                      SkStrikeServer* strikeSever, Settings settings)
         : SkNoDrawCanvas{sk_make_sp<TrackLayerDevice>(SkIRect::MakeWH(width, height), props)}
         , fDeviceMatrix{deviceMatrix}
+        , fSurfaceProps{props}
         , fStrikeServer{strikeSever}
         , fSettings{settings} {
     SkASSERT(fStrikeServer);
@@ -283,9 +284,9 @@ void SkTextBlobCacheDiffCanvas::processGlyphRun(
     options.fMinDistanceFieldFontSize = fSettings.fMinDistanceFieldFontSize;
     options.fMaxDistanceFieldFontSize = fSettings.fMaxDistanceFieldFontSize;
     GrTextContext::SanitizeOptions(&options);
-    if (GrTextContext::CanDrawAsDistanceFields(runPaint, runMatrix, surfaceProps(),
-                                               fSettings.fContextSupportsDistanceFieldText,
-                                               options)) {
+    if (GrTextContext::CanDrawAsDistanceFields(runPaint, runMatrix, fSurfaceProps,
+                                                    fSettings.fContextSupportsDistanceFieldText,
+                                                    options)) {
         SkScalar textRatio;
         SkPaint dfPaint(runPaint);
         SkScalerContextFlags flags;
@@ -357,7 +358,7 @@ void SkTextBlobCacheDiffCanvas::processGlyphRun(
     SkScalerContextEffects effects;
     auto* glyphCacheState =
             static_cast<SkStrikeServer*>(fStrikeServer)
-                    ->getOrCreateCache(runPaint, &this->surfaceProps(), &runMatrix,
+                    ->getOrCreateCache(runPaint, &fSurfaceProps, &runMatrix,
                                        SkScalerContextFlags::kFakeGammaAndBoostContrast,
                                        &deviceSpecificRec, &effects);
     SkASSERT(glyphCacheState);
@@ -396,7 +397,7 @@ void SkTextBlobCacheDiffCanvas::processGlyphRunForPaths(const SkTextBlobRunItera
     SkScalerContextEffects effects;
     auto* glyphCacheState =
             static_cast<SkStrikeServer*>(fStrikeServer)
-                    ->getOrCreateCache(pathPaint, &this->surfaceProps(), nullptr,
+                    ->getOrCreateCache(pathPaint, &fSurfaceProps, nullptr,
                                        SkScalerContextFlags::kFakeGammaAndBoostContrast,
                                        &deviceSpecificRec, &effects);
 
@@ -417,8 +418,8 @@ void SkTextBlobCacheDiffCanvas::processGlyphRunForDFT(const SkTextBlobRunIterato
     SkScalerContextRec deviceSpecificRec;
     SkScalerContextEffects effects;
     auto* glyphCacheState = static_cast<SkStrikeServer*>(fStrikeServer)
-                                    ->getOrCreateCache(runPaint, &this->surfaceProps(), nullptr,
-                                                       flags, &deviceSpecificRec, &effects);
+                                    ->getOrCreateCache(runPaint, &fSurfaceProps, nullptr, flags,
+                                                       &deviceSpecificRec, &effects);
 
     const bool asPath = false;
     const SkIPoint subPixelPos{0, 0};
@@ -431,13 +432,6 @@ void SkTextBlobCacheDiffCanvas::processGlyphRunForDFT(const SkTextBlobRunIterato
     }
 }
 
-const SkSurfaceProps& SkTextBlobCacheDiffCanvas::surfaceProps() const {
-    // SaveLayers can change the SurfaceProps used, and we ensure that the props used by the top
-    // device for the layer is correct. This is done by ensuring that TrackLayerDevice used by this
-    // canvas propagates them correctly when a new device is created for a layer.
-    return this->getTopDevice()->surfaceProps();
-}
-
 struct StrikeSpec {
     StrikeSpec() {}
     StrikeSpec(SkFontID typefaceID_, SkDiscardableHandleId discardableHandleId_)
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index 4d22a197a7..32d0b47c18 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -82,9 +82,9 @@ private:
     void processGlyphRunForPaths(const SkTextBlobRunIterator& it, const SkPaint& runPaint);
     void processGlyphRunForDFT(const SkTextBlobRunIterator& it, const SkPaint& runPaint,
                                SkScalerContextFlags flags);
-    const SkSurfaceProps& surfaceProps() const;
 
     const SkMatrix fDeviceMatrix;
+    const SkSurfaceProps fSurfaceProps;
     SkStrikeServer* const fStrikeServer;
     const Settings fSettings;
 };
diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index 56d2456fa4..7370bdafdb 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -49,12 +49,6 @@ public:
     const SkTHashSet<SkDiscardableHandleId>& lockedHandles() const { return fLockedHandles; }
     SkDiscardableHandleId handleCount() { return fNextHandleId; }
     int cacheMissCount(SkStrikeClient::CacheMissType type) { return fCacheMissCount[type]; }
-    bool hasCacheMiss() const {
-        for (uint32_t i = 0; i <= SkStrikeClient::CacheMissType::kLast; ++i) {
-            if (fCacheMissCount[i] > 0) return true;
-        }
-        return false;
-    }
 
 private:
     SkDiscardableHandleId fNextHandleId = 0u;
@@ -71,8 +65,6 @@ sk_sp<SkTextBlob> buildTextBlob(sk_sp<SkTypeface> tf, int glyphCount) {
     font.setStyle(SkPaint::kFill_Style);
     font.setHinting(SkPaint::kNormal_Hinting);
     font.setTextSize(1u);
-    font.setLCDRenderText(true);
-    font.setAntiAlias(true);
 
     SkTextBlobBuilder builder;
     SkRect bounds = SkRect::MakeWH(10, 10);
@@ -101,29 +93,15 @@ SkTextBlobCacheDiffCanvas::Settings MakeSettings(GrContext* context) {
     return settings;
 }
 
-struct RasterSettings {
-    int width = 10;
-    int height = 10;
-    SkPaint paint;
-    const SkMatrix* matrix = nullptr;
-    bool insertSaveLayer = false;
-    SkSurfaceProps surfaceProps = SkSurfaceProps(0u, kUnknown_SkPixelGeometry);
-};
-SkBitmap RasterBlob(sk_sp<SkTextBlob> blob, GrContext* context,
-                    RasterSettings settings = RasterSettings()) {
-    const SkImageInfo info = SkImageInfo::Make(settings.width, settings.height, kN32_SkColorType,
-                                               kPremul_SkAlphaType);
+SkBitmap RasterBlob(sk_sp<SkTextBlob> blob, int width, int height, const SkPaint& paint,
+                    GrContext* context, const SkMatrix* matrix = nullptr) {
+    const SkImageInfo info =
+            SkImageInfo::Make(width, height, kN32_SkColorType, kPremul_SkAlphaType);
     auto surface = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info);
-    if (settings.matrix) surface->getCanvas()->concat(*settings.matrix);
-    if (settings.insertSaveLayer) {
-        SkCanvas::SaveLayerRec rec;
-        surface->getCanvas()->saveLayer(rec);
-    }
-    surface->getCanvas()->drawTextBlob(blob.get(), 0u, 0u, settings.paint);
-    if (settings.insertSaveLayer) surface->getCanvas()->restore();
-
+    if (matrix) surface->getCanvas()->concat(*matrix);
+    surface->getCanvas()->drawTextBlob(blob.get(), 0u, 0u, paint);
     SkBitmap bitmap;
-    bitmap.allocN32Pixels(settings.width, settings.height);
+    bitmap.allocN32Pixels(width, height);
     surface->readPixels(bitmap, 0, 0);
     return bitmap;
 }
@@ -173,12 +151,10 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_StrikeSerialization, repor
                     client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
     auto clientBlob = buildTextBlob(clientTf, glyphCount);
 
-    SkBitmap expected = RasterBlob(serverBlob, ctxInfo.grContext());
-    SkBitmap actual = RasterBlob(clientBlob, ctxInfo.grContext());
+    SkBitmap expected = RasterBlob(serverBlob, 10, 10, paint, ctxInfo.grContext());
+    SkBitmap actual = RasterBlob(clientBlob, 10, 10, paint, ctxInfo.grContext());
     COMPARE_BLOBS(expected, actual, reporter);
 
-    REPORTER_ASSERT(reporter, !discardableManager->hasCacheMiss());
-
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
     discardableManager->unlockAndDeleteAll();
 }
@@ -344,12 +320,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsPath, reporter,
                     client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
     auto clientBlob = buildTextBlob(clientTf, glyphCount);
 
-    RasterSettings settings;
-    settings.paint = paint;
-    SkBitmap expected = RasterBlob(serverBlob, ctxInfo.grContext(), settings);
-    SkBitmap actual = RasterBlob(clientBlob, ctxInfo.grContext(), settings);
+    SkBitmap expected = RasterBlob(serverBlob, 10, 10, paint, ctxInfo.grContext());
+    SkBitmap actual = RasterBlob(clientBlob, 10, 10, paint, ctxInfo.grContext());
     COMPARE_BLOBS(expected, actual, reporter);
-    REPORTER_ASSERT(reporter, !discardableManager->hasCacheMiss());
     SkStrikeCache::Validate();
 
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
@@ -363,12 +336,12 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c
     SkStrikeServer server(discardableManager.get());
     SkStrikeClient client(discardableManager);
     SkPaint paint;
-    SkSurfaceProps surfaceProps(0, kUnknown_SkPixelGeometry);
 
     // A perspective transform forces fallback to dft.
     SkMatrix matrix = SkMatrix::I();
     matrix[SkMatrix::kMPersp0] = 0.5f;
     REPORTER_ASSERT(reporter, matrix.hasPerspective());
+    SkSurfaceProps surfaceProps(0, kUnknown_SkPixelGeometry);
     GrTextContext::Options options;
     GrTextContext::SanitizeOptions(&options);
     REPORTER_ASSERT(reporter, GrTextContext::CanDrawAsDistanceFields(
@@ -380,7 +353,8 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c
 
     int glyphCount = 10;
     auto serverBlob = buildTextBlob(serverTf, glyphCount);
-    SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), surfaceProps, &server,
+    const SkSurfaceProps props(SkSurfaceProps::kLegacyFontHost_InitType);
+    SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), props, &server,
                                                 MakeSettings(ctxInfo.grContext()));
     cache_diff_canvas.concat(matrix);
     cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint);
@@ -394,12 +368,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c
                     client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
     auto clientBlob = buildTextBlob(clientTf, glyphCount);
 
-    RasterSettings settings;
-    settings.matrix = &matrix;
-    SkBitmap expected = RasterBlob(serverBlob, ctxInfo.grContext(), settings);
-    SkBitmap actual = RasterBlob(clientBlob, ctxInfo.grContext(), settings);
+    SkBitmap expected = RasterBlob(serverBlob, 10, 10, paint, ctxInfo.grContext(), &matrix);
+    SkBitmap actual = RasterBlob(clientBlob, 10, 10, paint, ctxInfo.grContext(), &matrix);
     COMPARE_BLOBS(expected, actual, reporter);
-    REPORTER_ASSERT(reporter, !discardableManager->hasCacheMiss());
     SkStrikeCache::Validate();
 
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
@@ -419,7 +390,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_CacheMissReporting, report
     auto clientBlob = buildTextBlob(clientTf, glyphCount);
 
     // Raster the client-side blob without the glyph data, we should get cache miss notifications.
-    RasterBlob(clientBlob, ctxInfo.grContext());
+    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,
@@ -433,47 +406,4 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_CacheMissReporting, report
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
     discardableManager->unlockAndDeleteAll();
 }
-
-DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_SaveLayerWithLcdSurfaceProps, reporter,
-                                   ctxInfo) {
-    sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
-    SkStrikeServer server(discardableManager.get());
-    SkStrikeClient client(discardableManager);
-    SkPaint paint;
-    const SkSurfaceProps props(0, kRGB_H_SkPixelGeometry);
-
-    // Server.
-    auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
-    auto serverTfData = server.serializeTypeface(serverTf.get());
-
-    int glyphCount = 10;
-    auto serverBlob = buildTextBlob(serverTf, glyphCount);
-    SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), props, &server,
-                                                MakeSettings(ctxInfo.grContext()));
-    SkCanvas::SaveLayerRec rec;
-    cache_diff_canvas.saveLayer(rec);
-    cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint);
-    cache_diff_canvas.restore();
-
-    std::vector<uint8_t> serverStrikeData;
-    server.writeStrikeData(&serverStrikeData);
-
-    // Client.
-    auto clientTf = client.deserializeTypeface(serverTfData->data(), serverTfData->size());
-    REPORTER_ASSERT(reporter,
-                    client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
-    auto clientBlob = buildTextBlob(clientTf, glyphCount);
-
-    RasterSettings settings;
-    settings.insertSaveLayer = true;
-    settings.surfaceProps = props;
-    settings.paint = paint;
-    SkBitmap expected = RasterBlob(serverBlob, ctxInfo.grContext(), settings);
-    SkBitmap actual = RasterBlob(clientBlob, ctxInfo.grContext(), settings);
-    COMPARE_BLOBS(expected, actual, reporter);
-    REPORTER_ASSERT(reporter, !discardableManager->hasCacheMiss());
-
-    // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
-    discardableManager->unlockAndDeleteAll();
-}
 #endif