From fcec47e7bdd82cadef9a7ba518b747a8ab0f1605 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Tue, 13 Mar 2018 09:16:04 -0400 Subject: [PATCH] Bug 1444387 - Part 1. Avoid using fallback if an image is not ready. r=jrmuizel If an image container is empty, it will not produce an image key for use with WebRender. This is generally not a sign of failure because the producer likely has yet to populate the container with data. As such, we should not immediately attempt to fallback. In fact, fallback can make things worse in this situation, as we will create an image client to send over the data, but then find that there is no data to share (or find that there is, due to a race with the producer thread, and use image clients when we could use shared surfaces). --- layout/generic/nsImageFrame.cpp | 6 +++++- layout/generic/nsPluginFrame.cpp | 6 +++++- layout/generic/nsVideoFrame.cpp | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp index fc80b9f7e032..e5e8b9ec4379 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -1744,7 +1744,11 @@ nsDisplayImage::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilde return false; } - return aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, destRect); + // 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. + aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, destRect); + return true; } ImgDrawResult diff --git a/layout/generic/nsPluginFrame.cpp b/layout/generic/nsPluginFrame.cpp index 7032e3e2ddd5..1f8526f38cc1 100644 --- a/layout/generic/nsPluginFrame.cpp +++ b/layout/generic/nsPluginFrame.cpp @@ -1442,8 +1442,12 @@ nsPluginFrame::CreateWebRenderCommands(nsDisplayItem* aItem, } lm->AddDidCompositeObserver(mDidCompositeObserver.get()); + // 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. LayoutDeviceRect dest(r.x, r.y, size.width, size.height); - return aManager->CommandBuilder().PushImage(aItem, container, aBuilder, aResources, aSc, dest); + aManager->CommandBuilder().PushImage(aItem, container, aBuilder, aResources, aSc, dest); + return true; } diff --git a/layout/generic/nsVideoFrame.cpp b/layout/generic/nsVideoFrame.cpp index 645202a2ee81..471dc380719e 100644 --- a/layout/generic/nsVideoFrame.cpp +++ b/layout/generic/nsVideoFrame.cpp @@ -494,8 +494,12 @@ public: SwapScaleWidthHeightForRotation(scaleHint, rotationDeg); container->SetScaleHint(scaleHint); + // 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. LayoutDeviceRect rect(destGFXRect.x, destGFXRect.y, destGFXRect.width, destGFXRect.height); - return aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, rect); + aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, rect); + return true; } // It would be great if we could override GetOpaqueRegion to return nonempty here,