Bug 1580922. Partially revert blob recoord fallback changes. r=nical
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Tue, 01 Oct 2019 22:23:45 +0000
changeset 495928 95114b9893afbbbaae7bb945b42b38d5c8ad01f3
parent 495927 46c2aeec240856f8f86678dc32ed9717ea2f41fd
child 495929 41f38d4062fc0385ce1dbfb7bf4dd91aa6dceb3b
push id114140
push userdvarga@mozilla.com
push dateWed, 02 Oct 2019 18:04:51 +0000
treeherdermozilla-inbound@32eb0ea893f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1580922, 1568227
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1580922. Partially revert blob recoord fallback changes. r=nical This moves the origin of fallback blobs back to the top left of their display item bounds. This is what they were before the patches in bug 1568227 and makes more sense because there's not necessarily a reference frame above the fallback frame which means that the coordinates of the display item can change without us wanting to invalidate the interior. Differential Revision: https://phabricator.services.mozilla.com/D47275
gfx/layers/wr/WebRenderCommandBuilder.cpp
layout/reftests/text-stroke/reftest.list
testing/web-platform/meta/css/css-fill-stroke/paint-order-001.tentative.html.ini
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -2098,16 +2098,24 @@ static bool PaintItemByDrawTarget(nsDisp
       aDT->FillRect(Rect(visibleRect),
                     gfx::ColorPattern(gfx::Color(r, g, b, 0.5)));
     }
   }
 
   return isInvalidated;
 }
 
+// When drawing fallback images we create either
+// a real image or a blob image that will contain the display item.
+// In the case of a blob image we paint the item at 0,0 instead
+// of trying to keep at aItem->GetBounds().TopLeft() like we do
+// with SVG. We do this because there's not necessarily a reference frame
+// between us and the rest of the world so the the coordinates
+// that we get for the bounds are not necessarily stable across scrolling
+// or other movement.
 already_AddRefed<WebRenderFallbackData>
 WebRenderCommandBuilder::GenerateFallbackData(
     nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder,
     wr::IpcResourceUpdateQueue& aResources, const StackingContextHelper& aSc,
     nsDisplayListBuilder* aDisplayListBuilder, LayoutDeviceRect& aImageRect) {
   bool useBlobImage = StaticPrefs::gfx_webrender_blob_images() &&
                       !aItem->MustPaintOnContentSide();
   Maybe<gfx::Color> highlight = Nothing();
@@ -2178,16 +2186,20 @@ WebRenderCommandBuilder::GenerateFallbac
 
   auto visibleSize = visibleRect.Size();
   if (visibleSize.IsEmpty()) {
     return nullptr;
   }
   // Display item bounds should be unscaled
   aImageRect = visibleRect / layerScale;
 
+  // We always paint items at 0,0 so the visibleRect that we use inside the blob
+  // is needs to be adjusted by the display item bounds top left.
+  visibleRect -= dtRect.TopLeft();
+
   nsDisplayItemGeometry* geometry = fallbackData->mGeometry;
 
   bool needPaint = true;
 
   // nsDisplayFilters is rendered via BasicLayerManager which means the
   // invalidate region is unknown until we traverse the displaylist contained by
   // it.
   if (geometry && !fallbackData->IsInvalid() &&
@@ -2251,36 +2263,37 @@ WebRenderCommandBuilder::GenerateFallbac
                   aStream.write((const char*)&font, sizeof(font));
                 }
                 fonts = std::move(aScaledFonts);
               },
               visibleRect.ToUnknownRect().TopLeft());
       RefPtr<gfx::DrawTarget> dummyDt = gfx::Factory::CreateDrawTarget(
           gfx::BackendType::SKIA, gfx::IntSize(1, 1), format);
       RefPtr<gfx::DrawTarget> dt = gfx::Factory::CreateRecordingDrawTarget(
-          recorder, dummyDt, dtRect.ToUnknownRect());
+          recorder, dummyDt, visibleRect.ToUnknownRect());
       if (!fallbackData->mBasicLayerManager) {
         fallbackData->mBasicLayerManager =
             new BasicLayerManager(BasicLayerManager::BLM_INACTIVE);
       }
-      // aOffset is (0, 0) because blobs don't want to normalize their
-      // coordinates
       bool isInvalidated = PaintItemByDrawTarget(
-          aItem, dt, LayoutDevicePoint(0, 0),
-          /*aVisibleRect: */ visibleRect.ToUnknownRect(), aDisplayListBuilder,
+          aItem, dt, (dtRect/layerScale).TopLeft(),
+          /*aVisibleRect: */ dt->GetRect(), aDisplayListBuilder,
           fallbackData->mBasicLayerManager, scale, highlight);
       if (!isInvalidated) {
         if (!aItem->GetBuildingRect().IsEqualInterior(
                 fallbackData->mBuildingRect)) {
           // The building rect has changed but we didn't see any invalidations.
           // We should still consider this an invalidation.
           isInvalidated = true;
         }
       }
-      recorder->FlushItem(visibleRect.ToUnknownRect());
+
+      // the item bounds are relative to the blob origin which is
+      // dtRect.TopLeft()
+      recorder->FlushItem((dtRect - dtRect.TopLeft()).ToUnknownRect());
       recorder->Finish();
 
       if (!validFonts) {
         gfxCriticalNote << "Failed serializing fonts for blob image";
         return nullptr;
       }
 
       if (isInvalidated) {
@@ -2329,18 +2342,16 @@ WebRenderCommandBuilder::GenerateFallbac
           RefPtr<gfx::DrawTarget> dt = helper.GetDrawTarget();
           if (!dt) {
             return nullptr;
           }
           if (!fallbackData->mBasicLayerManager) {
             fallbackData->mBasicLayerManager =
                 new BasicLayerManager(mManager->GetWidget());
           }
-          // aOffset is applied because this case is a "real" image and not a
-          // blob
           isInvalidated = PaintItemByDrawTarget(
               aItem, dt,
               /*aOffset: */ aImageRect.TopLeft(),
               /*aVisibleRect: */ dt->GetRect(), aDisplayListBuilder,
               fallbackData->mBasicLayerManager, scale, highlight);
         }
 
         if (isInvalidated) {
--- a/layout/reftests/text-stroke/reftest.list
+++ b/layout/reftests/text-stroke/reftest.list
@@ -1,10 +1,9 @@
 # basic tests for webkit-text-stroke
 # fuzzy is needed here for platform dependent backends
-
 # These fail on Linux without webrender due to lack of antialiasing of the HTML text stroke
-fuzzy(0-64,0-52) fails-if(gtkWidget&&!webrender) fuzzy-if(geckoview&&webrender,0-64,0-770) == webkit-text-stroke-property-001.html webkit-text-stroke-property-001-ref.html
-fuzzy(0-4,0-24) fails-if(gtkWidget&&!webrender) fuzzy-if(geckoview&&webrender,0-4,0-1476) == webkit-text-stroke-property-002.html webkit-text-stroke-property-002-ref.html
+fuzzy(0-64,0-52) fails-if(gtkWidget&&!webrender) fuzzy-if(webrender,0-64,0-61) fuzzy-if(geckoview&&webrender,0-64,0-770) == webkit-text-stroke-property-001.html webkit-text-stroke-property-001-ref.html
+fuzzy(0-4,0-24) fails-if(gtkWidget&&!webrender) fuzzy-if(webrender,0-4,0-27) fuzzy-if(geckoview&&webrender,0-4,0-1476) == webkit-text-stroke-property-002.html webkit-text-stroke-property-002-ref.html
 fuzzy(0-48,0-28) fails-if(gtkWidget&&!webrender) fuzzy-if(webrender,0-64,0-341) == webkit-text-stroke-property-003.html webkit-text-stroke-property-003-ref.html
-fuzzy(0-64,0-33) fails-if(gtkWidget&&!webrender) fuzzy-if(webrender,0-48,0-575) == webkit-text-stroke-property-004.html webkit-text-stroke-property-004-ref.html
+fuzzy(0-64,0-33) fails-if(gtkWidget&&!webrender) fuzzy-if(webrender,0-64,0-575) fuzzy-if(geckoview&&webrender,0-64,0-599) == webkit-text-stroke-property-004.html webkit-text-stroke-property-004-ref.html
 fuzzy(0-64,0-47) fails-if(gtkWidget&&!webrender) fuzzy-if(webrender,0-64,0-776) == webkit-text-stroke-property-005.html webkit-text-stroke-property-005-ref.html
 fuzzy(0-71,0-10) fails-if(gtkWidget&&!webrender) == webkit-text-stroke-property-006.html webkit-text-stroke-property-006-ref.html
--- a/testing/web-platform/meta/css/css-fill-stroke/paint-order-001.tentative.html.ini
+++ b/testing/web-platform/meta/css/css-fill-stroke/paint-order-001.tentative.html.ini
@@ -1,7 +1,5 @@
 [paint-order-001.tentative.html]
   disabled:
     if os == "android" and not e10s: https://bugzilla.mozilla.org/show_bug.cgi?id=1520847
   expected:
     if (os == "win"): FAIL
-  fuzzy:
-    if webrender: maxDifference=64;totalPixels=0-46