Bug 664130 - Clarify some display list comments and code. r=roc.
authorJonathan Watt <jwatt@jwatt.org>
Thu, 08 Sep 2011 12:15:00 +0100
changeset 76721 0126d20e120fea346d42bd3faeb9407ab80ef80b
parent 76720 1067e5452e63a80ea1471bf45847c1c38080c3e4
child 76722 24ba516de8f32db9f882076784cf95933465052f
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersroc
bugs664130
milestone9.0a1
Bug 664130 - Clarify some display list comments and code. r=roc.
layout/base/nsDisplayList.cpp
layout/base/nsDisplayList.h
layout/generic/nsFrame.cpp
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -2619,16 +2619,17 @@ void nsDisplayTransform::HitTest(nsDispl
   /* We want to go from transformed-space to regular space.
    * Thus we have to invert the matrix, which normally does
    * the reverse operation (e.g. regular->transformed)
    */
 
   /* Now, apply the transform and pass it down the channel. */
   nsRect resultingRect;
   if (aRect.width == 1 && aRect.height == 1) {
+    // Magic width/height indicating we're hit testing a point, not a rect
     gfxPoint point = matrix.Inverse().ProjectPoint(
                        gfxPoint(NSAppUnitsToFloatPixels(aRect.x, factor),
                                 NSAppUnitsToFloatPixels(aRect.y, factor)));
 
     resultingRect = nsRect(NSFloatPixelsToAppUnits(float(point.x), factor),
                            NSFloatPixelsToAppUnits(float(point.y), factor),
                            1, 1);
 
--- a/layout/base/nsDisplayList.h
+++ b/layout/base/nsDisplayList.h
@@ -617,17 +617,20 @@ public:
    * display items to be individually wrapped --- currently nsDisplayClip
    * and nsDisplayClipRoundedRect only.
    */
   virtual PRUint32 GetPerFrameKey() { return PRUint32(GetType()); }
   /**
    * This is called after we've constructed a display list for event handling.
    * When this is called, we've already ensured that aRect intersects the
    * item's bounds.
-   * 
+   *
+   * @param aRect the point or rect being tested, relative to the reference
+   * frame. If the width and height are both 1 app unit, it indicates we're
+   * hit testing a point, not a rect.
    * @param aState must point to a HitTestState. If you don't have one,
    * just create one with the default constructor and pass it in.
    * @param aOutFrames each item appends the frame(s) in this display item that
    * the rect is considered over (if any) to aOutFrames.
    */
   virtual void HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect,
                        HitTestState* aState, nsTArray<nsIFrame*> *aOutFrames) {}
   /**
@@ -2059,17 +2062,17 @@ private:
 
 /* A display item that applies a transformation to all of its descendant
  * elements.  This wrapper should only be used if there is a transform applied
  * to the root element.
  *
  * The reason that a "bounds" rect is involved in transform calculations is
  * because CSS-transforms allow percentage values for the x and y components
  * of <translation-value>s, where percentages are percentages of the element's
- * content box.
+ * border box.
  *
  * INVARIANT: The wrapped frame is transformed.
  * INVARIANT: The wrapped frame is non-null.
  */ 
 class nsDisplayTransform: public nsDisplayItem
 {
 public:
   /* Constructor accepts a display list, empties it, and wraps it up.  It also
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1541,23 +1541,30 @@ nsIFrame::BuildDisplayListForStackingCon
   if (disp->mOpacity == 0.0 && aBuilder->IsForPainting())
     return NS_OK;
 
   PRBool applyAbsPosClipping =
       ApplyAbsPosClipping(aBuilder, disp, this, &absPosClip);
   nsRect dirtyRect = aDirtyRect;
 
   PRBool inTransform = aBuilder->IsInTransform();
-  /* If we're being transformed, we need to invert the matrix transform so that we don't 
-   * grab points in the wrong coordinate system!
-   */
   if ((mState & NS_FRAME_MAY_BE_TRANSFORMED) &&
       disp->HasTransform()) {
-    /* If we have a complex transform, just grab the entire overflow rect instead. */
+    // Transform dirtyRect into our frame's local coordinate space. Note that
+    // the new value is the bounds of the old value's transformed vertices, so
+    // the area covered by dirtyRect may increase here.
+    //
+    // Although we don't bother to check for and maintain the 1x1 size of the
+    // magic rect indicating a hit test point, in reality this is extremely
+    // unlikely to matter. The rect starts off with dimensions of 1x1 *app*
+    // units, and it would require a very large number of elements with
+    // transforms along a parent chain to noticably expand this by an entire
+    // device pixel.
     if (Preserves3DChildren() || !nsDisplayTransform::UntransformRect(dirtyRect, this, nsPoint(0, 0), &dirtyRect)) {
+      // we have a singular transform - just grab the entire overflow rect
       dirtyRect = GetVisualOverflowRectRelativeToSelf();
     }
     inTransform = PR_TRUE;
   }
 
   if (applyAbsPosClipping) {
     dirtyRect.IntersectRect(dirtyRect,
                             absPosClip - aBuilder->ToReferenceFrame(this));
@@ -1648,30 +1655,33 @@ nsIFrame::BuildDisplayListForStackingCon
   if (applyAbsPosClipping) {
     nsAbsPosClipWrapper wrapper(absPosClip);
     nsDisplayItem* item = wrapper.WrapList(aBuilder, this, &resultList);
     if (!item)
       return NS_ERROR_OUT_OF_MEMORY;
     // resultList was emptied
     resultList.AppendToTop(item);
   }
- 
-  /* If there are any SVG effects, wrap up the list in an effects list. */
+
+  /* If there are any SVG effects, wrap the list up in an SVG effects item
+   * (which also handles CSS group opacity). Note that we create an SVG effects
+   * item even if resultList is empty, since a filter can produce graphical
+   * output even if the element being filtered wouldn't otherwise do so.
+   */
   if (usingSVGEffects) {
     /* List now emptied, so add the new list to the top. */
     rv = resultList.AppendNewToTop(
         new (aBuilder) nsDisplaySVGEffects(aBuilder, this, &resultList));
     if (NS_FAILED(rv))
       return rv;
-  } else
-
-  /* If there is any opacity, wrap it up in an opacity list.
-   * If there's nothing in the list, don't add anything.
+  }
+  /* Else, if the list is non-empty and there is CSS group opacity without SVG
+   * effects, wrap it up in an opacity item.
    */
-  if (disp->mOpacity < 1.0f && !resultList.IsEmpty()) {
+  else if (disp->mOpacity < 1.0f && !resultList.IsEmpty()) {
     rv = resultList.AppendNewToTop(
         new (aBuilder) nsDisplayOpacity(aBuilder, this, &resultList));
     if (NS_FAILED(rv))
       return rv;
   }
 
   /* If we're going to apply a transformation and don't have preserve-3d set, wrap 
    * everything in an nsDisplayTransform. If there's nothing in the list, don't add 
@@ -1864,18 +1874,18 @@ nsIFrame::BuildDisplayListForChild(nsDis
     if (NS_SUCCEEDED(rv)) {
       rv = aBuilder->DisplayCaret(aChild, dirty, &list);
     }
   } else {
     nsRect clipRect;
     PRBool applyAbsPosClipping =
         ApplyAbsPosClipping(aBuilder, disp, aChild, &clipRect);
     // A pseudo-stacking context (e.g., a positioned element with z-index auto).
-    // we allow positioned descendants of this element to escape to our
-    // container's positioned descendant list, because they might be
+    // We allow positioned descendants of the child to escape to our parent
+    // stacking context's positioned descendant list, because they might be
     // z-index:non-auto
     nsDisplayListCollection pseudoStack;
     nsRect clippedDirtyRect = dirty;
     if (applyAbsPosClipping) {
       // clipRect is in builder-reference-frame coordinates,
       // dirty/clippedDirtyRect are in aChild coordinates
       clippedDirtyRect.IntersectRect(clippedDirtyRect,
                                      clipRect - aBuilder->ToReferenceFrame(aChild));