Bug 1743761 - Ensure we invalidate substituted images when the preferred size is ready. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 03 Dec 2021 03:14:01 +0000
changeset 600989 356c521826254fbae435b212923b20801d171950
parent 600988 38926348a8eb9bab2f5a453daff9cc529d00a1af
child 600990 9e3d95237e945277fcfce50ceeb8691f6d9281f6
push id39032
push usermlaza@mozilla.com
push dateFri, 03 Dec 2021 09:48:16 +0000
treeherdermozilla-central@356c52182625 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1743761
milestone96.0a1
first release with
nightly linux32
356c52182625 / 96.0a1 / 20211203094816 / files
nightly linux64
356c52182625 / 96.0a1 / 20211203094816 / files
nightly mac
356c52182625 / 96.0a1 / 20211203094816 / files
nightly win32
356c52182625 / 96.0a1 / 20211203094816 / files
nightly win64
356c52182625 / 96.0a1 / 20211203094816 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1743761 - Ensure we invalidate substituted images when the preferred size is ready. r=tnikkel Previously with ImageContainers, we would put the new preferred surface into the ImageContainer. When we check if we should invalidate, it would have a different image key, and hence invalidate the image frame and schedule a paint. With ImageProviders, it returns the same key in this case, because the ImageProvider represents a particular surface. As such, we need to actually track when we get a substituted ImageProvider, and invalidate the image frame more aggressively to ensure we get the preferred size. Differential Revision: https://phabricator.services.mozilla.com/D132583
gfx/layers/wr/WebRenderCommandBuilder.cpp
gfx/layers/wr/WebRenderCommandBuilder.h
gfx/layers/wr/WebRenderUserData.cpp
gfx/layers/wr/WebRenderUserData.h
image/RasterImage.cpp
layout/generic/nsImageFrame.cpp
layout/painting/nsCSSRenderingBorders.cpp
layout/painting/nsImageRenderer.cpp
layout/svg/SVGImageFrame.cpp
layout/xul/nsImageBoxFrame.cpp
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -1992,30 +1992,31 @@ bool WebRenderCommandBuilder::PushImage(
   auto c = wr::ToLayoutRect(aClip);
   aBuilder.PushImage(r, c, !aItem->BackfaceIsHidden(), rendering, key.value());
 
   return true;
 }
 
 Maybe<wr::ImageKey> WebRenderCommandBuilder::CreateImageProviderKey(
     nsDisplayItem* aItem, image::WebRenderImageProvider* aProvider,
+    image::ImgDrawResult aDrawResult,
     mozilla::wr::IpcResourceUpdateQueue& aResources) {
   RefPtr<WebRenderImageProviderData> imageData =
       CreateOrRecycleWebRenderUserData<WebRenderImageProviderData>(aItem);
   MOZ_ASSERT(imageData);
-  return imageData->UpdateImageKey(aProvider, aResources);
+  return imageData->UpdateImageKey(aProvider, aDrawResult, aResources);
 }
 
 bool WebRenderCommandBuilder::PushImageProvider(
     nsDisplayItem* aItem, image::WebRenderImageProvider* aProvider,
-    mozilla::wr::DisplayListBuilder& aBuilder,
+    image::ImgDrawResult aDrawResult, mozilla::wr::DisplayListBuilder& aBuilder,
     mozilla::wr::IpcResourceUpdateQueue& aResources,
     const LayoutDeviceRect& aRect, const LayoutDeviceRect& aClip) {
   Maybe<wr::ImageKey> key =
-      CreateImageProviderKey(aItem, aProvider, aResources);
+      CreateImageProviderKey(aItem, aProvider, aDrawResult, aResources);
   if (!key) {
     return false;
   }
 
   auto rendering = wr::ToImageRendering(aItem->Frame()->UsedImageRendering());
   auto r = wr::ToLayoutRect(aRect);
   auto c = wr::ToLayoutRect(aClip);
   aBuilder.PushImage(r, c, !aItem->BackfaceIsHidden(), rendering, key.value());
--- a/gfx/layers/wr/WebRenderCommandBuilder.h
+++ b/gfx/layers/wr/WebRenderCommandBuilder.h
@@ -13,16 +13,17 @@
 #include "mozilla/layers/WebRenderMessages.h"
 #include "mozilla/layers/WebRenderScrollData.h"
 #include "mozilla/layers/WebRenderUserData.h"
 #include "mozilla/SVGIntegrationUtils.h"  // for WrFiltersHolder
 #include "nsDisplayList.h"
 #include "nsIFrame.h"
 #include "nsTHashSet.h"
 #include "DisplayItemCache.h"
+#include "ImgDrawResult.h"
 
 namespace mozilla {
 
 namespace image {
 class WebRenderImageProvider;
 }
 
 namespace layers {
@@ -66,30 +67,32 @@ class WebRenderCommandBuilder final {
       nsDisplayItem* aItem, ImageContainer* aContainer,
       mozilla::wr::DisplayListBuilder& aBuilder,
       mozilla::wr::IpcResourceUpdateQueue& aResources,
       mozilla::wr::ImageRendering aRendering, const StackingContextHelper& aSc,
       gfx::IntSize& aSize, const Maybe<LayoutDeviceRect>& aAsyncImageBounds);
 
   Maybe<wr::ImageKey> CreateImageProviderKey(
       nsDisplayItem* aItem, image::WebRenderImageProvider* aProvider,
+      image::ImgDrawResult aDrawResult,
       mozilla::wr::IpcResourceUpdateQueue& aResources);
 
   WebRenderUserDataRefTable* GetWebRenderUserDataTable() {
     return &mWebRenderUserDatas;
   }
 
   bool PushImage(nsDisplayItem* aItem, ImageContainer* aContainer,
                  mozilla::wr::DisplayListBuilder& aBuilder,
                  mozilla::wr::IpcResourceUpdateQueue& aResources,
                  const StackingContextHelper& aSc,
                  const LayoutDeviceRect& aRect, const LayoutDeviceRect& aClip);
 
   bool PushImageProvider(nsDisplayItem* aItem,
                          image::WebRenderImageProvider* aProvider,
+                         image::ImgDrawResult aDrawResult,
                          mozilla::wr::DisplayListBuilder& aBuilder,
                          mozilla::wr::IpcResourceUpdateQueue& aResources,
                          const LayoutDeviceRect& aRect,
                          const LayoutDeviceRect& aClip);
 
   Maybe<wr::ImageMask> BuildWrMaskImage(
       nsDisplayMasksAndClipPaths* aMaskItem, wr::DisplayListBuilder& aBuilder,
       wr::IpcResourceUpdateQueue& aResources, const StackingContextHelper& aSc,
--- a/gfx/layers/wr/WebRenderUserData.cpp
+++ b/gfx/layers/wr/WebRenderUserData.cpp
@@ -246,39 +246,47 @@ void WebRenderImageData::CreateImageClie
 
 WebRenderImageProviderData::WebRenderImageProviderData(
     RenderRootStateManager* aManager, nsDisplayItem* aItem)
     : WebRenderUserData(aManager, aItem) {}
 
 WebRenderImageProviderData::WebRenderImageProviderData(
     RenderRootStateManager* aManager, uint32_t aDisplayItemKey,
     nsIFrame* aFrame)
-    : WebRenderUserData(aManager, aDisplayItemKey, aFrame) {}
+    : WebRenderUserData(aManager, aDisplayItemKey, aFrame),
+      mDrawResult(ImgDrawResult::NOT_READY) {}
 
 WebRenderImageProviderData::~WebRenderImageProviderData() = default;
 
 Maybe<wr::ImageKey> WebRenderImageProviderData::UpdateImageKey(
-    WebRenderImageProvider* aProvider, wr::IpcResourceUpdateQueue& aResources) {
+    WebRenderImageProvider* aProvider, ImgDrawResult aDrawResult,
+    wr::IpcResourceUpdateQueue& aResources) {
   MOZ_ASSERT(aProvider);
 
   if (mProvider != aProvider) {
     mProvider = aProvider;
   }
 
   wr::ImageKey key = {};
   nsresult rv = mProvider->UpdateKey(mManager, aResources, key);
   mKey = NS_SUCCEEDED(rv) ? Some(key) : Nothing();
+  mDrawResult = aDrawResult;
   return mKey;
 }
 
 bool WebRenderImageProviderData::Invalidate(ImageProviderId aProviderId) const {
   if (!aProviderId || mProvider->GetProviderId() != aProviderId || !mKey) {
     return false;
   }
 
+  if (mDrawResult != ImgDrawResult::SUCCESS &&
+      mDrawResult != ImgDrawResult::BAD_IMAGE) {
+    return false;
+  }
+
   wr::ImageKey key = {};
   nsresult rv =
       mProvider->UpdateKey(mManager, mManager->AsyncResourceUpdates(), key);
   return NS_SUCCEEDED(rv) && mKey.ref() == key;
 }
 
 WebRenderFallbackData::WebRenderFallbackData(RenderRootStateManager* aManager,
                                              nsDisplayItem* aItem)
--- a/gfx/layers/wr/WebRenderUserData.h
+++ b/gfx/layers/wr/WebRenderUserData.h
@@ -12,16 +12,17 @@
 #include "mozilla/image/WebRenderImageProvider.h"
 #include "mozilla/layers/AnimationInfo.h"
 #include "mozilla/dom/RemoteBrowser.h"
 #include "mozilla/UniquePtr.h"
 #include "nsIFrame.h"
 #include "nsRefPtrHashtable.h"
 #include "nsTHashSet.h"
 #include "ImageTypes.h"
+#include "ImgDrawResult.h"
 #include "DisplayItemClip.h"
 
 namespace mozilla {
 
 class nsDisplayItemGeometry;
 
 namespace webgpu {
 class WebGPUChild;
@@ -192,23 +193,25 @@ class WebRenderImageProviderData final :
                              uint32_t aDisplayItemKey, nsIFrame* aFrame);
   ~WebRenderImageProviderData() override;
 
   WebRenderImageProviderData* AsImageProviderData() override { return this; }
   UserDataType GetType() override { return UserDataType::eImageProvider; }
   static UserDataType Type() { return UserDataType::eImageProvider; }
 
   Maybe<wr::ImageKey> UpdateImageKey(image::WebRenderImageProvider* aProvider,
+                                     image::ImgDrawResult aDrawResult,
                                      wr::IpcResourceUpdateQueue& aResources);
 
   bool Invalidate(image::ImageProviderId aProviderId) const;
 
  protected:
   RefPtr<image::WebRenderImageProvider> mProvider;
   Maybe<wr::ImageKey> mKey;
+  image::ImgDrawResult mDrawResult;
 };
 
 /// Used for fallback rendering.
 ///
 /// In most cases this uses blob images but it can also render on the content
 /// side directly into a texture.
 class WebRenderFallbackData : public WebRenderUserData {
  public:
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -626,17 +626,23 @@ RasterImage::GetImageProvider(WindowRend
   }
 
   if (!result.Surface()->IsFinished()) {
     result.Surface().TakeProvider(aProvider);
     return ImgDrawResult::INCOMPLETE;
   }
 
   result.Surface().TakeProvider(aProvider);
-  return ImgDrawResult::SUCCESS;
+  switch (result.Type()) {
+    case MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND:
+    case MatchType::SUBSTITUTE_BECAUSE_PENDING:
+      return ImgDrawResult::WRONG_SIZE;
+    default:
+      return ImgDrawResult::SUCCESS;
+  }
 }
 
 size_t RasterImage::SizeOfSourceWithComputedFallback(
     SizeOfState& aState) const {
   return mSourceBuffer->SizeOfIncludingThisWithComputedFallback(
       aState.mMallocSizeOf);
 }
 
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -1862,17 +1862,17 @@ ImgDrawResult nsImageFrame::DisplayAltFe
               imgCon, this, destRect, destRect, aSc, aFlags, svgContext,
               region);
       RefPtr<image::WebRenderImageProvider> provider;
       result = imgCon->GetImageProvider(aManager->LayerManager(), decodeSize,
                                         svgContext, region, aFlags,
                                         getter_AddRefs(provider));
       if (provider) {
         bool wrResult = aManager->CommandBuilder().PushImageProvider(
-            aItem, provider, aBuilder, aResources, destRect, bounds);
+            aItem, provider, result, aBuilder, aResources, destRect, bounds);
         result &= wrResult ? ImgDrawResult::SUCCESS : ImgDrawResult::NOT_READY;
       } else {
         // We don't use &= here because we want the result to be NOT_READY so
         // the next block executes.
         result = ImgDrawResult::NOT_READY;
       }
     }
 
@@ -2137,21 +2137,26 @@ bool nsDisplayImage::CreateWebRenderComm
         if (StaticPrefs::image_svg_blob_image() &&
             mPrevImage->GetType() == imgIContainer::TYPE_VECTOR) {
           prevFlags |= imgIContainer::FLAG_RECORD_BLOB;
         } else {
           prevFlags &= ~imgIContainer::FLAG_RECORD_BLOB;
         }
 
         RefPtr<image::WebRenderImageProvider> prevProvider;
-        ImgDrawResult newDrawResult = mPrevImage->GetImageProvider(
+        ImgDrawResult prevDrawResult = mPrevImage->GetImageProvider(
             aManager->LayerManager(), decodeSize, svgContext, region, prevFlags,
             getter_AddRefs(prevProvider));
-        if (prevProvider && newDrawResult == ImgDrawResult::SUCCESS) {
-          drawResult = newDrawResult;
+        if (prevProvider && (prevDrawResult == ImgDrawResult::SUCCESS ||
+                             prevDrawResult == ImgDrawResult::WRONG_SIZE)) {
+          // We use WRONG_SIZE here to ensure that when the frame next tries to
+          // invalidate due to a frame update from the current image, we don't
+          // consider the result from the previous image to be a valid result to
+          // avoid redrawing.
+          drawResult = ImgDrawResult::WRONG_SIZE;
           provider = std::move(prevProvider);
           flags = prevFlags;
           break;
         }
 
         // Previous image was unusable; we can forget about it.
         updatePrevImage = true;
       }
@@ -2171,17 +2176,17 @@ bool nsDisplayImage::CreateWebRenderComm
     frame->mPrevImage = frame->mImage;
   }
 
   // If the image container is empty, we don't want to fallback. Any other
   // failure will be due to resource constraints and fallback is unlikely to
   // help us. Hence we can ignore the return value from PushImage.
   if (provider) {
     aManager->CommandBuilder().PushImageProvider(
-        this, provider, aBuilder, aResources, destRect, destRect);
+        this, provider, drawResult, aBuilder, aResources, destRect, destRect);
   }
 
   nsDisplayItemGenericImageGeometry::UpdateDrawResult(this, drawResult);
   return true;
 }
 
 ImgDrawResult nsImageFrame::PaintImage(gfxContext& aRenderingContext,
                                        nsPoint aPt, const nsRect& aDirtyRect,
--- a/layout/painting/nsCSSRenderingBorders.cpp
+++ b/layout/painting/nsCSSRenderingBorders.cpp
@@ -3633,18 +3633,18 @@ ImgDrawResult nsCSSBorderImageRenderer::
       drawResult = img->GetImageProvider(aManager->LayerManager(), decodeSize,
                                          svgContext, region, flags,
                                          getter_AddRefs(provider));
       if (!provider) {
         break;
       }
 
       Maybe<wr::ImageKey> key =
-          aManager->CommandBuilder().CreateImageProviderKey(aItem, provider,
-                                                            aResources);
+          aManager->CommandBuilder().CreateImageProviderKey(
+              aItem, provider, drawResult, aResources);
       if (key.isNothing()) {
         break;
       }
 
       if (mFill) {
         float epsilon = 0.0001;
         bool noVerticalBorders = widths[0] <= epsilon && widths[2] < epsilon;
         bool noHorizontalBorders = widths[1] <= epsilon && widths[3] < epsilon;
--- a/layout/painting/nsImageRenderer.cpp
+++ b/layout/painting/nsImageRenderer.cpp
@@ -627,18 +627,18 @@ ImgDrawResult nsImageRenderer::BuildWebR
           aManager->LayerManager(), decodeSize, svgContext, region,
           containerFlags, getter_AddRefs(provider));
       if (!provider) {
         NS_WARNING("Failed to get image container");
         break;
       }
 
       Maybe<wr::ImageKey> key =
-          aManager->CommandBuilder().CreateImageProviderKey(aItem, provider,
-                                                            aResources);
+          aManager->CommandBuilder().CreateImageProviderKey(
+              aItem, provider, drawResult, aResources);
       if (key.isNothing()) {
         break;
       }
 
       auto rendering =
           wr::ToImageRendering(aItem->Frame()->UsedImageRendering());
       wr::LayoutRect clip = wr::ToLayoutRect(clipRect);
 
--- a/layout/svg/SVGImageFrame.cpp
+++ b/layout/svg/SVGImageFrame.cpp
@@ -644,18 +644,19 @@ bool SVGImageFrame::CreateWebRenderComma
 
   // Don't do any actual mutations to state if we're doing a dry run
   // (used to decide if we're making this into an active layer)
   if (!aDryRun) {
     // If the image container is empty, we don't want to fallback. Any other
     // failure will be due to resource constraints and fallback is unlikely to
     // help us. Hence we can ignore the return value from PushImage.
     if (provider) {
-      aManager->CommandBuilder().PushImageProvider(
-          aItem, provider, aBuilder, aResources, destRect, clipRect);
+      aManager->CommandBuilder().PushImageProvider(aItem, provider, drawResult,
+                                                   aBuilder, aResources,
+                                                   destRect, clipRect);
     }
 
     nsDisplayItemGenericImageGeometry::UpdateDrawResult(aItem, drawResult);
   }
 
   return true;
 }
 
--- a/layout/xul/nsImageBoxFrame.cpp
+++ b/layout/xul/nsImageBoxFrame.cpp
@@ -437,17 +437,17 @@ ImgDrawResult nsImageBoxFrame::CreateWeb
     NS_WARNING("Failed to get image provider");
     return result;
   }
 
   auto rendering = wr::ToImageRendering(aItem->Frame()->UsedImageRendering());
   wr::LayoutRect fill = wr::ToLayoutRect(fillRect);
 
   Maybe<wr::ImageKey> key = aManager->CommandBuilder().CreateImageProviderKey(
-      aItem, provider, aResources);
+      aItem, provider, result, aResources);
   if (key.isNothing()) {
     return result;
   }
 
   aBuilder.PushImage(fill, fill, !BackfaceIsHidden(), rendering, key.value());
   return result;
 }