Bug 1343147 - Part 1. Do not double applying transform vector of the root frame in a glyph mask into the target context. draft
authorcku <cku@mozilla.com>
Tue, 03 Oct 2017 11:29:19 +0800
changeset 675250 f91c227087b2521f929e0a06908e6c4809866a66
parent 673165 44643fce30b43a8981535c335aaccb45006e456b
child 675251 6a23fbdb6554a5e5298475d7edcd9b7f52e098d0
push id83080
push userbmo:cku@mozilla.com
push dateThu, 05 Oct 2017 02:23:09 +0000
bugs1343147, 1299715
milestone58.0a1
Bug 1343147 - Part 1. Do not double applying transform vector of the root frame in a glyph mask into the target context. 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
@@ -8105,21 +8105,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;