Bug 1109873 - Add some explanatory comments to the APZC tree building code. r=kats
authorBotond Ballo <botond@mozilla.com>
Thu, 08 Jan 2015 09:40:01 -0500
changeset 222769 3352803d949bf54210c0c1ea80b1efad280df12a
parent 222768 79d541cf6db233801f6f1714416f94b4c20ca844
child 222770 e92ec65eb66b4fb4d940e8bb55eefb47f23554fd
push id28073
push userkwierso@gmail.com
push dateFri, 09 Jan 2015 01:08:23 +0000
treeherdermozilla-central@b3f84cf78dc2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1109873
milestone37.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 1109873 - Add some explanatory comments to the APZC tree building code. r=kats
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -52,18 +52,32 @@ struct APZCTreeManager::TreeBuildingStat
 
   // State that doesn't change as we recurse in the tree building
   CompositorParent* const mCompositor;
   const bool mIsFirstPaint;
   const uint64_t mOriginatingLayersId;
   const APZPaintLogHelper mPaintLogger;
 
   // State that is updated as we perform the tree build
+
+  // A list of APZCs that need to be destroyed at the end of the tree building.
+  // This is initialized with all APZCs in the old tree, and APZCs are removed
+  // from it as we reuse them in the new tree.
   nsTArray< nsRefPtr<AsyncPanZoomController> > mApzcsToDestroy;
+
+  // This map is populated as we place APZCs into the new tree. Its purpose is
+  // to facilitate re-using the same APZC for different layers that scroll
+  // together (and thus have the same ScrollableLayerGuid).
   std::map<ScrollableLayerGuid, AsyncPanZoomController*> mApzcMap;
+
+  // A stack of event regions which corresponds to the call stack of recursive
+  // UpdatePanZoomController calls during tree-building, except that we don't
+  // start populating this stack until we first encounter an APZC. At each level
+  // we accumulate the event regions of non-APZC descendants of the layer at
+  // that level.
   nsTArray<EventRegions> mEventRegions;
 };
 
 /*static*/ const ScreenMargin
 APZCTreeManager::CalculatePendingDisplayPort(
   const FrameMetrics& aFrameMetrics,
   const ParentLayerPoint& aVelocity,
   double aEstimatedPaintDuration)
@@ -190,16 +204,18 @@ APZCTreeManager::UpdatePanZoomController
   MOZ_ASSERT(state.mEventRegions.Length() == 0);
 
   for (size_t i = 0; i < state.mApzcsToDestroy.Length(); i++) {
     APZCTM_LOG("Destroying APZC at %p\n", state.mApzcsToDestroy[i].get());
     state.mApzcsToDestroy[i]->Destroy();
   }
 }
 
+// Compute the touch-sensitive region of an APZC. This is used only when
+// event-regions are disabled.
 static nsIntRegion
 ComputeTouchSensitiveRegion(GeckoContentController* aController,
                             const FrameMetrics& aMetrics,
                             const nsIntRegion& aObscured)
 {
   // Use the composition bounds as the hit test region.
   // Optionally, the GeckoContentController can provide a touch-sensitive
   // region that constrains all frames associated with the controller.
@@ -334,16 +350,20 @@ APZCTreeManager::PrepareAPZCForLayer(con
 
     apzc->NotifyLayersUpdated(aMetrics,
         aState.mIsFirstPaint && (aLayersId == aState.mOriginatingLayersId));
 
     nsIntRegion unobscured;
     if (!gfxPrefs::LayoutEventRegionsEnabled()) {
       unobscured = ComputeTouchSensitiveRegion(state->mController, aMetrics, aObscured);
     }
+    // This initializes, among other things, the APZC's hit-region.
+    // If event-regions are disabled, this will initialize it to the empty
+    // region, but UpdatePanZoomControllerTree will add the hit-region from
+    // the event regions later.
     apzc->SetLayerHitTestData(EventRegions(unobscured), aAncestorTransform);
     APZCTM_LOG("Setting region %s as visible region for APZC %p\n",
         Stringify(unobscured).c_str(), apzc);
 
     PrintAPZCInfo(aLayer, apzc);
 
     // Bind the APZC instance into the tree of APZCs
     if (aNextSibling) {
@@ -380,30 +400,31 @@ APZCTreeManager::PrepareAPZCForLayer(con
         // For an apzc that is not the root for its layers ID, we give it the
         // same zoom constraints as its parent. This ensures that if e.g.
         // user-scalable=no was specified, none of the APZCs allow double-tap
         // to zoom.
         apzc->UpdateZoomConstraints(apzc->GetParent()->GetZoomConstraints());
       }
     }
 
+    // Add a guid -> APZC mapping for the newly created APZC.
     insertResult.first->second = apzc;
   } else {
     // We already built an APZC earlier in this tree walk, but we have another layer
     // now that will also be using that APZC. The hit-test region on the APZC needs
     // to be updated to deal with the new layer's hit region.
     // FIXME: Combining this hit test region to the existing hit test region has a bit
     // of a problem, because it assumes the z-index of this new region is the same as
     // the z-index of the old region (from the previous layer with the same scrollid)
     // when in fact that may not be the case.
     // Consider the case where we have three layers: A, B, and C. A is at the top in
     // z-order and C is at the bottom. A and C share a scrollid and scroll together; but
     // B has a different scrollid and scrolls independently. Depending on how B moves
     // and the async transform on it, a larger/smaller area of C may be unobscured.
-    // However, when we combine the hit regions of A and C here we are ignoring the async
+    // However, when we combine the hit regions of A and C here we are ignoring the
     // async transform and so we basically assume the same amount of C is always visible
     // on top of B. Fixing this doesn't appear to be very easy so I'm leaving it for
     // now in the hopes that we won't run into this problem a lot.
     if (!gfxPrefs::LayoutEventRegionsEnabled()) {
       nsIntRegion unobscured = ComputeTouchSensitiveRegion(state->mController, aMetrics, aObscured);
       apzc->AddHitTestRegions(EventRegions(unobscured));
       APZCTM_LOG("Adding region %s to visible region of APZC %p\n", Stringify(unobscured).c_str(), apzc);
     }
@@ -464,17 +485,23 @@ APZCTreeManager::UpdatePanZoomController
     obscured.Transform(To3DMatrix(transform).Inverse());
   }
 
   // If there's no APZC at this level, any APZCs for our child layers will
   // have our siblings as their siblings, and our parent as their parent.
   AsyncPanZoomController* next = aNextSibling;
   if (apzc) {
     // Otherwise, use this APZC as the parent going downwards, and start off
-    // with its first child as the next sibling
+    // with its first child as the next sibling.
+    // Note that |apzc| at this point will not have children corresponding to
+    // the subtree of aLayer (those children will be populated in the loop
+    // below). However, it might have children corresponding to the subtree of
+    // another layer that shares the same APZC and that has already been
+    // visited; the children added at this level will become prev-siblings
+    // of those.
     aParent = apzc;
     next = apzc->GetFirstChild();
   }
 
   // In our recursive downward traversal, track event regions for layers once
   // we encounter an APZC. Push a new empty region on the mEventRegions stack
   // which will accumulate the hit area of descendants of aLayer. In general,
   // the mEventRegions stack is used to accumulate event regions from descendant
@@ -514,17 +541,17 @@ APZCTreeManager::UpdatePanZoomController
     // if a layer has an APZC, we simply store the regions from that subtree on
     // that APZC and don't propagate them upwards in the tree. Because of the
     // way we do hit-testing (where the deepest matching APZC is used) it should
     // still be ok if we did propagate those regions upwards and included them
     // in all the ancestor APZCs.
     //
     // Also at this point in the code the |obscured| region includes the hit
     // regions of children of |aLayer| as well as the hit regions of |aLayer|'s
-    // younger uncles (i.e. the next-sibling chain of |aLayer|'s parent).
+    // younger uncles (i.e. the next-sibling chains of |aLayer|'s ancestors).
     // When we compute the unobscured regions below, we subtract off the
     // |obscured| region, but it would also be ok to do this before the above
     // loop. At that point |obscured| would only have the uncles' hit regions
     // and not the children. The reason this is ok is again because of the way
     // we do hit-testing (where the deepest APZC is used) it doesn't matter if
     // we count the children as obscuring the parent or not.
 
     EventRegions unobscured;
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -447,16 +447,35 @@ private:
   /**
    * Recursive helper function to build the APZC tree. The tree of APZC instances has
    * the same shape as the layer tree, but excludes all the layers that are not scrollable.
    * Note that this means APZCs corresponding to layers at different depths in the tree
    * may end up becoming siblings. It also means that the "root" APZC may have siblings.
    * This function walks the layer tree backwards through siblings and constructs the APZC
    * tree also as a last-child-prev-sibling tree because that simplifies the hit detection
    * code.
+   *
+   * @param aState             The current tree building state.
+   * @param aLayer             The (layer, metrics) pair which is the current
+   *                           position in the recursive walk of the layer tree.
+   *                           This calls builds an APZC subtree corresponding
+   *                           to the layer subtree rooted at aLayer.
+   * @param aLayersId          The layers id of the layer in aLayer.
+   * @param aAncestorTransform The accumulated CSS transforms of all the
+   *                           layers from aLayer up (via the parent chain)
+   *                           to the next APZC-bearing layer.
+   * @param aParent            The parent of any APZC built at this level.
+   * @param aNextSibling       The next sibling any APZC built at this level.
+   * @param aObscured          The region that is obscured by APZCs above
+   *                           the one at this level (in practice, the
+   *                           next-siblings of this one), in the ParentLayer
+   *                           pixels of the APZC at this level.
+   * @return                   The root of the APZC subtree at this level, or
+   *                           aNextSibling if no APZCs were built for the
+   *                           layer subtree at this level.
    */
   AsyncPanZoomController* UpdatePanZoomControllerTree(TreeBuildingState& aState,
                                                       const LayerMetricsWrapper& aLayer,
                                                       uint64_t aLayersId,
                                                       const gfx::Matrix4x4& aAncestorTransform,
                                                       AsyncPanZoomController* aParent,
                                                       AsyncPanZoomController* aNextSibling,
                                                       const nsIntRegion& aObscured);