Bug 1343147 - Part 1. Do not double applying transform vector of the root frame in a glyph mask into the target context. r=mstange, a=ritu
authorcku <cku@mozilla.com>
Tue, 03 Oct 2017 11:29:19 +0800
changeset 434668 2374254401686aca9372ee4bac6a746bcd7b5aed
parent 434667 cff708b944d7ca498ec9396dae7578fa7f32194a
child 434669 25fd3441407d9b9246f11ac854007fb17f0ffce6
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange, ritu
bugs1343147, 1299715
milestone57.0
Bug 1343147 - Part 1. Do not double applying transform vector of the root frame in a glyph mask into the target context. r=mstange, a=ritu When we generate the glyph mask for a transformed frame in GenerateAndPushTextMask, the transform vector had been applied into aContext[1], so we should find a way to prevent applying the vector again when painting the glyph mask. In bug 1299715, I tried to prevent double apply at [2], it caused two problems: 1. We only skip generating nsDisplayTransform, but we may still create a nsDisplayPerspactive bellow. Since the parent of a nsDisplayPerspective must be a nsDisplayTransform, which have been ignored, so we hit this assertion. 2. We skip all transform for all frames while painting the glyph mask, which is not correct. We should only skip double applying transform vector of the root frame. This patch fixes both of these issues: a. We will still create a nsDisplayTransform for the root frame if need. But the transform matrix we apply into the target context will be an identity matrix, so we fix #1 above. b. In #a, we change the transform matrix to an identity matrix only for the root frame of the glyph mask, so we fix #2. [1] https://hg.mozilla.org/mozilla-central/file/59e5ec5729db/layout/painting/nsDisplayList.cpp#l752 [2] https://hg.mozilla.org/mozilla-central/file/ce2c129f0a87/layout/generic/nsFrame.cpp#l2806 MozReview-Commit-ID: 973lkQQxLB6
layout/generic/nsFrame.cpp
layout/painting/nsDisplayList.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2798,24 +2798,21 @@ nsIFrame::BuildDisplayListForStackingCon
     const nsIFrame* outerReferenceFrame = this;
     if (this != aBuilder->RootReferenceFrame()) {
       outerReferenceFrame =
         aBuilder->FindReferenceFrameFor(GetParent(), &toOuterReferenceFrame);
     }
     buildingDisplayList.SetReferenceFrameAndCurrentOffset(outerReferenceFrame,
       GetOffsetToCrossDoc(outerReferenceFrame));
 
-    if (!aBuilder->IsForGenerateGlyphMask() &&
-        !aBuilder->IsForPaintingSelectionBG()) {
-      nsDisplayTransform *transformItem =
-        new (aBuilder) nsDisplayTransform(aBuilder, this,
-                                          &resultList, dirtyRect, 0,
-                                          allowAsyncAnimation);
-      resultList.AppendNewToTop(transformItem);
-    }
+    nsDisplayTransform *transformItem =
+      new (aBuilder) nsDisplayTransform(aBuilder, this,
+                                        &resultList, dirtyRect, 0,
+                                        allowAsyncAnimation);
+    resultList.AppendNewToTop(transformItem);
 
     if (hasPerspective) {
       if (clipCapturedBy == ContainerItemType::ePerspective) {
         clipState.Restore();
       }
       resultList.AppendNewToTop(
         new (aBuilder) nsDisplayPerspective(
           aBuilder, this,
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7971,21 +7971,28 @@ nsDisplayTransform::UpdateScrollData(moz
   }
   return true;
 }
 
 already_AddRefed<Layer> nsDisplayTransform::BuildLayer(nsDisplayListBuilder *aBuilder,
                                                        LayerManager *aManager,
                                                        const ContainerLayerParameters& aContainerParameters)
 {
+  // While generating a glyph mask, the transform vector of the root frame had
+  // been applied into the target context, so stop applying it again here.
+  const bool shouldSkipTransform =
+    (aBuilder->RootReferenceFrame() == mFrame) &&
+    (aBuilder->IsForGenerateGlyphMask() || aBuilder->IsForPaintingSelectionBG());
+
   /* For frames without transform, it would not be removed for
    * backface hidden here.  But, it would be removed by the init
    * function of nsDisplayTransform.
    */
-  const Matrix4x4& newTransformMatrix = GetTransformForRendering();
+  const Matrix4x4 newTransformMatrix =
+    shouldSkipTransform ? Matrix4x4(): GetTransformForRendering();
 
   uint32_t flags = FrameLayerBuilder::CONTAINER_ALLOW_PULL_BACKGROUND_COLOR;
   RefPtr<ContainerLayer> container = aManager->GetLayerBuilder()->
     BuildContainerLayerFor(aBuilder, aManager, mFrame, this, mStoredList.GetChildren(),
                            aContainerParameters, &newTransformMatrix, flags);
 
   if (!container) {
     return nullptr;