Bug 1580922. Partially revert blob recoord fallback changes. r=nical
☠☠ backed out by ffd50b38a549 ☠ ☠
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Tue, 01 Oct 2019 12:15:53 +0000
changeset 495812 88a32122785ba7f6269a82442a0a46f971313a0e
parent 495811 7d1f6ed3ccf56db2f558bd8aaf352543122e09a2
child 495813 10c7400edbc1d7754b44e090e5a7685991ba16a6
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-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-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
+fuzzy(0-64,0-52) 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) 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) 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) 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) 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) == 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