Bug 1543584 - Always rasterize SVGs but clamp the maximum size. r=tni…

…kkel

SVG performance with the fallback path with WebRender is very bad. This
patch avoids fallback by always producing a rasterized surface we store
in SurfaceCache, but also clamping the size consistently to a configured
maximum. This will cause us to upscale rasterized SVGs which is
undesirable visually but is a lower risk change that we can uplift to
beta than fixing the underlying performance issue.

Differential Revision: https://phabricator.services.mozilla.com/D27159

image/LookupResult.h

@@ -62,6 +62,13 @@ class MOZ_STACK_CLASS LookupResult {
                "NOT_FOUND or PENDING do not make sense with a surface");
   }
 
+  LookupResult(MatchType aMatchType, const gfx::IntSize& aSuggestedSize)
+      : mMatchType(aMatchType), mSuggestedSize(aSuggestedSize) {
+    MOZ_ASSERT(
+        mMatchType == MatchType::NOT_FOUND || mMatchType == MatchType::PENDING,
+        "Only NOT_FOUND or PENDING make sense with no surface");
+  }
+
   LookupResult(DrawableSurface&& aSurface, MatchType aMatchType,
                const gfx::IntSize& aSuggestedSize)
       : mSurface(std::move(aSurface)),

image/SurfaceCache.cpp

@@ -511,26 +511,8 @@ class ImageSurfaceCache {
   IntSize SuggestedSize(const IntSize& aSize) const {
     IntSize suggestedSize = SuggestedSizeInternal(aSize);
     if (mIsVectorImage) {
-      // Whether or not we are in factor of 2 mode, vector image rasterization
-      // is clamped at a configured maximum if the caller is willing to accept
-      // substitutes.
-      MOZ_ASSERT(SurfaceCache::IsLegalSize(suggestedSize));
-
-      // If we exceed the maximum, we need to scale the size downwards to fit.
-      // It shouldn't get here if it is significantly larger because
-      // VectorImage::UseSurfaceCacheForSize should prevent us from requesting
-      // a rasterized version of a surface greater than 4x the maximum.
-      int32_t maxSizeKB = gfxPrefs::ImageCacheMaxRasterizedSVGThresholdKB();
-      int32_t proposedKB = suggestedSize.width * suggestedSize.height / 256;
-      if (maxSizeKB >= proposedKB) {
-        return suggestedSize;
-      }
-
-      double scale = sqrt(double(maxSizeKB) / proposedKB);
-      suggestedSize.width = int32_t(scale * suggestedSize.width);
-      suggestedSize.height = int32_t(scale * suggestedSize.height);
+      suggestedSize = SurfaceCache::ClampVectorSize(suggestedSize);
     }
-
     return suggestedSize;
   }
 
@@ -964,7 +946,9 @@ class SurfaceCacheImpl final : public nsIMemoryReporter {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       // No cached surfaces for this image.
-      return LookupResult(MatchType::NOT_FOUND);
+      return LookupResult(
+          MatchType::NOT_FOUND,
+          SurfaceCache::ClampSize(aImageKey, aSurfaceKey.Size()));
     }
 
     // Repeatedly look up the best match, trying again if the resulting surface
@@ -983,7 +967,7 @@ class SurfaceCacheImpl final : public nsIMemoryReporter {
 
       if (!surface) {
         return LookupResult(
-            matchType);  // Lookup in the per-image cache missed.
+            matchType, suggestedSize);  // Lookup in the per-image cache missed.
       }
 
       drawableSurface = surface->GetDrawableSurface();
@@ -1647,5 +1631,32 @@ bool SurfaceCache::IsLegalSize(const IntSize& aSize) {
   return true;
 }
 
+IntSize SurfaceCache::ClampVectorSize(const IntSize& aSize) {
+  // If we exceed the maximum, we need to scale the size downwards to fit.
+  // It shouldn't get here if it is significantly larger because
+  // VectorImage::UseSurfaceCacheForSize should prevent us from requesting
+  // a rasterized version of a surface greater than 4x the maximum.
+  int32_t maxSizeKB = gfxPrefs::ImageCacheMaxRasterizedSVGThresholdKB();
+  if (maxSizeKB <= 0) {
+    return aSize;
+  }
+
+  int32_t proposedKB = int32_t(int64_t(aSize.width) * aSize.height / 256);
+  if (maxSizeKB >= proposedKB) {
+    return aSize;
+  }
+
+  double scale = sqrt(double(maxSizeKB) / proposedKB);
+  return IntSize(int32_t(scale * aSize.width), int32_t(scale * aSize.height));
+}
+
+IntSize SurfaceCache::ClampSize(ImageKey aImageKey, const IntSize& aSize) {
+  if (aImageKey->GetType() != imgIContainer::TYPE_VECTOR) {
+    return aSize;
+  }
+
+  return ClampVectorSize(aSize);
+}
+
 }  // namespace image
 }  // namespace mozilla

image/SurfaceCache.h

@@ -434,6 +434,16 @@ struct SurfaceCache {
    */
   static bool IsLegalSize(const IntSize& aSize);
 
+  /**
+   * @return clamped size for the given vector image size to rasterize at.
+   */
+  static IntSize ClampVectorSize(const IntSize& aSize);
+
+  /**
+   * @return clamped size for the given image and size to rasterize at.
+   */
+  static IntSize ClampSize(const ImageKey aImageKey, const IntSize& aSize);
+
  private:
   virtual ~SurfaceCache() = 0;  // Forbid instantiation.
 };

image/VectorImage.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/dom/SVGSVGElement.h"
 #include "mozilla/dom/SVGDocument.h"
 #include "mozilla/gfx/2D.h"
+#include "mozilla/gfx/gfxVars.h"
 #include "mozilla/PendingAnimationTracker.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/RefPtr.h"
@@ -755,10 +756,6 @@ VectorImage::GetFrameInternal(const IntSize& aSize,
     return MakeTuple(ImgDrawResult::NOT_READY, aSize, RefPtr<SourceSurface>());
   }
 
-  // We don't allow large surfaces to be rasterized on the Draw and
-  // GetImageContainerAtSize paths, because those have alternatives. If we get
-  // here however, then we know it came from GetFrame(AtSize) and that path does
-  // not have any fallback method, so we don't check UseSurfaceCacheForSize.
   RefPtr<SourceSurface> sourceSurface;
   IntSize decodeSize;
   Tie(sourceSurface, decodeSize) =
@@ -853,8 +850,7 @@ VectorImage::IsImageContainerAvailableAtSize(LayerManager* aManager,
                                              uint32_t aFlags) {
   // Since we only support image containers with WebRender, and it can handle
   // textures larger than the hw max texture size, we don't need to check aSize.
-  return !aSize.IsEmpty() && UseSurfaceCacheForSize(aSize) &&
-         IsImageContainerAvailable(aManager, aFlags);
+  return !aSize.IsEmpty() && IsImageContainerAvailable(aManager, aFlags);
 }
 
 //******************************************************************************
@@ -864,10 +860,6 @@ VectorImage::GetImageContainerAtSize(layers::LayerManager* aManager,
                                      const Maybe<SVGImageContext>& aSVGContext,
                                      uint32_t aFlags,
                                      layers::ImageContainer** aOutContainer) {
-  if (!UseSurfaceCacheForSize(aSize)) {
-    return ImgDrawResult::NOT_SUPPORTED;
-  }
-
   Maybe<SVGImageContext> newSVGContext;
   MaybeRestrictSVGContext(newSVGContext, aSVGContext, aFlags);
 
@@ -950,9 +942,13 @@ VectorImage::Draw(gfxContext* aContext, const nsIntSize& aSize,
   // - We are using a DrawTargetRecording because we prefer the drawing commands
   //   in general to the rasterized surface. This allows blob images to avoid
   //   rasterized SVGs with WebRender.
-  // - The size exceeds what we are will to cache as a rasterized surface.
+  // - The size exceeds what we are willing to cache as a rasterized surface.
+  //   We don't do this for WebRender because the performance of the fallback
+  //   path is quite bad and upscaling the SVG from the clamped size is better
+  //   than bringing the browser to a crawl.
   if (aContext->GetDrawTarget()->GetBackendType() == BackendType::RECORDING ||
-      !UseSurfaceCacheForSize(aSize)) {
+      (!gfxVars::UseWebRender() &&
+       aSize != SurfaceCache::ClampVectorSize(aSize))) {
     aFlags |= FLAG_BYPASS_SURFACE_CACHE;
   }
 
@@ -1020,24 +1016,6 @@ already_AddRefed<gfxDrawable> VectorImage::CreateSVGDrawable(
   return svgDrawable.forget();
 }
 
-bool VectorImage::UseSurfaceCacheForSize(const IntSize& aSize) const {
-  int32_t maxSizeKB = gfxPrefs::ImageCacheMaxRasterizedSVGThresholdKB();
-  if (maxSizeKB <= 0) {
-    return true;
-  }
-
-  if (!SurfaceCache::IsLegalSize(aSize)) {
-    return false;
-  }
-
-  // With factor of 2 mode, we should be willing to use a surface which is up
-  // to twice the width, and twice the height, of the maximum sized surface
-  // before switching to drawing to the target directly. That means the size in
-  // KB works out to be:
-  //   width * height * 4 [bytes/pixel] * / 1024 [bytes/KB] <= 2 * 2 * maxSizeKB
-  return aSize.width * aSize.height / 1024 <= maxSizeKB;
-}
-
 Tuple<RefPtr<SourceSurface>, IntSize> VectorImage::LookupCachedSurface(
     const IntSize& aSize, const Maybe<SVGImageContext>& aSVGContext,
     uint32_t aFlags) {

image/VectorImage.h

@@ -103,9 +103,6 @@ class VectorImage final : public ImageResource, public nsIStreamListener {
   already_AddRefed<gfxDrawable> CreateSVGDrawable(
       const SVGDrawingParameters& aParams);
 
-  /// Returns true if we use the surface cache to store rasterized copies.
-  bool UseSurfaceCacheForSize(const IntSize& aSize) const;
-
   /// Rasterize the SVG into a surface. aWillCache will be set to whether or
   /// not the new surface was put into the cache.
   already_AddRefed<SourceSurface> CreateSurface(

modules/libpref/init/all.js

@@ -4815,9 +4815,9 @@ pref("image.animated.resume-from-last-displayed", true);
 // same data at different sizes.
 pref("image.cache.factor2.threshold-surfaces", 4);
 
-// Maximum number of pixels in either dimension that we are willing to upscale
-// an SVG to when we are in "factor of 2" mode.
-pref("image.cache.max-rasterized-svg-threshold-kb", 92160);
+// Maximum size of a surface in KB we are willing to produce when rasterizing
+// an SVG.
+pref("image.cache.max-rasterized-svg-threshold-kb", 204800);
 
 // The maximum size, in bytes, of the decoded images we cache
 pref("image.cache.size", 5242880);