Bug 138057. Eliminate nsBlockFrame::RememberFloaterDamage, instead recording the damage when we reflow the floater. This allows us to catch several cases that we weren't catching before, so we no longer need to dirty all the line boxes when an incremental reflow targeted at a float arrives. r=dbaron, sr=attinasi.

2002-04-26 19:19:39 +00:00 · 2002-04-26 19:19:39 +00:00 · d572531e5b
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@ -1561,9 +1561,7 @@ nsresult
 nsBlockFrame::PrepareChildIncrementalReflow(nsBlockReflowState& aState)
 {
  // Determine the line being impacted
-  PRBool isFloater;
-  line_iterator line;
-  FindLineFor(aState.mNextRCFrame, &isFloater, &line);
+  line_iterator line = FindLineFor(aState.mNextRCFrame);
  if (line == end_lines()) {
    // This assertion actually fires on lots of pages
    // (e.g., bugzilla, bugzilla query page), so limit it
@ -1581,12 +1579,6 @@ nsBlockFrame::PrepareChildIncrementalReflow(nsBlockReflowState& aState)
    return PrepareResizeReflow(aState);
  }

-  // XXX: temporary: If the child frame is a floater then punt
-  // XXX_perf XXXldb How big a perf problem is this?
-  if (isFloater) {
-    return PrepareResizeReflow(aState);
-  }
-
  if (line->IsInline()) {
    if (aState.GetFlag(BRS_COMPUTEMAXWIDTH)) {
      // We've been asked to compute the maximum width of the block
@ -1653,9 +1645,7 @@ nsBlockFrame::RetargetInlineIncrementalReflow(nsBlockReflowState &aState,
 #ifdef DEBUG
    // Paranoia. Ensure that the prev-in-flow is really in the
    // previous line.
-    PRBool dummy;
-    line_iterator check;
-    FindLineFor(aState.mNextRCFrame, &dummy, &check);
+    line_iterator check = FindLineFor(aState.mNextRCFrame);
    NS_ASSERTION(check == aLine, "prev-in-flow not in previous linebox");
 #endif

@ -1947,10 +1937,8 @@ nsBlockFrame::PrepareResizeReflow(nsBlockReflowState& aState)

 //----------------------------------------

-PRBool
-nsBlockFrame::FindLineFor(nsIFrame* aFrame,
-                          PRBool* aIsFloaterResult,
-                          line_iterator* aFoundLine)
+nsBlockFrame::line_iterator
+nsBlockFrame::FindLineFor(nsIFrame* aFrame)
 {
  // This assertion actually fires on lots of pages (e.g., bugzilla,
  // bugzilla query page), so limit it to a few people until we fix the
@ -1960,30 +1948,27 @@ nsBlockFrame::FindLineFor(nsIFrame* aFrame,
  NS_PRECONDITION(aFrame, "why pass a null frame?");
 #endif

-  PRBool isFloater = PR_FALSE;
  line_iterator line = begin_lines(),
                line_end = end_lines();
  for ( ; line != line_end; ++line) {
-    if (line->Contains(aFrame)) {
-      break;
-    }
-    // XXXldb what does this do?
+    // If the target frame is in-flow, and this line contains the it,
+    // then we've found our line.
+    if (line->Contains(aFrame))
+      return line;
+
+    // If the target frame is floated, and this line contains the
+    // floater's placeholder, then we've found our line.
    if (line->HasFloaters()) {
-      nsFloaterCache* fc = line->GetFirstFloater();
-      while (fc) {
-        if (aFrame == fc->mPlaceholder->GetOutOfFlowFrame()) {
-          isFloater = PR_TRUE;
-          goto done;
-        }
-        fc = fc->Next();
+      for (nsFloaterCache *fc = line->GetFirstFloater();
+           fc != nsnull;
+           fc = fc->Next()) {
+        if (aFrame == fc->mPlaceholder->GetOutOfFlowFrame())
+          return line;
      }
-    }
+    }      
  }

- done:
-  *aIsFloaterResult = isFloater;
-  *aFoundLine = line;
-  return (line != line_end);
+  return line_end;
 }

 // SEC: added GetCurrentLine() for bug 45152
@ -1997,46 +1982,10 @@ nsBlockFrame::GetCurrentLine(nsBlockReflowState *aState, nsLineBox ** aOutCurren
  return NS_OK;  
 }

-/**
- * Remember regions of reflow damage from floaters that changed size (so
- * far, only vertically, which is a bug) in the space manager so that we
- * can mark any impacted lines dirty in |PropagateFloaterDamage|.
- */
-void
-nsBlockFrame::RememberFloaterDamage(nsBlockReflowState& aState,
-                                    nsLineBox* aLine,
-                                    const nsRect& aOldCombinedArea)
-{
-  nsRect lineCombinedArea;
-  aLine->GetCombinedArea(&lineCombinedArea);
-  if (lineCombinedArea != aLine->mBounds &&
-      lineCombinedArea != aOldCombinedArea) {
-    // The line's combined-area changed. Therefore we need to damage
-    // the lines below that were previously (or are now) impacted by
-    // the change. It's possible that a floater shrunk or grew so
-    // use the larger of the impacted area.
-
-    // XXXldb If just the width of the floater changed, then this code
-    // won't be triggered and the code below (in |PropagateFloaterDamage|)
-    // won't kick in for "non-Block lines".  See
-    // "XXX: Maybe the floater itself changed size?" below.
-    //
-    // This is a major flaw in this code.
-    //
-    nscoord newYMost = lineCombinedArea.YMost();
-    nscoord oldYMost = aOldCombinedArea.YMost();
-    nscoord impactYB = newYMost < oldYMost ? oldYMost : newYMost;
-    nscoord impactYA = lineCombinedArea.y;
-    nsSpaceManager *spaceManager = aState.mReflowState.mSpaceManager;
-    spaceManager->IncludeInDamage(impactYA, impactYB);
-  }
-}
-
 /**
 * Propagate reflow "damage" from from earlier lines to the current
 * line.  The reflow damage comes from the following sources:
- *  1. The regions of floater damage remembered in
- *     |RememberFloaterDamage|.
+ *  1. The regions of floater damage remembered during reflow.
 *  2. The combination of nonzero |aDeltaY| and any impact by a floater,
 *     either the previous reflow or now.
 *
@ -2279,11 +2228,6 @@ nsBlockFrame::ReflowDirtyLines(nsBlockReflowState& aState)
      } else {
        deltaY = line->mBounds.YMost() - oldYMost;
      }
-
-      // Remember any things that could potentially be changes in the
-      // positions of floaters in this line so that we later damage the
-      // lines adjacent to those changes.
-      RememberFloaterDamage(aState, line, oldCombinedArea);
    } else {
      if (deltaY != 0)
        SlideLine(aState, line, deltaY);
@ -5898,22 +5842,9 @@ nsBlockFrame::ReflowDirtyChild(nsIPresShell* aPresShell, nsIFrame* aChild)
    }

    // Mark the line containing the child frame dirty.
-    PRBool isFloater;
-    line_iterator fline;
-    FindLineFor(aChild, &isFloater, &fline);
-    
-    if (!isFloater) {
-      if (fline != end_lines())
-        MarkLineDirty(fline);
-    }
-    else {
-      // XXXldb Could we do this more efficiently?
-      for (line_iterator line = begin_lines(), line_end = end_lines();
-           line != line_end;
-           ++line) {
-        line->MarkDirty();
-      }
-    }
+    line_iterator fline = FindLineFor(aChild);
+    if (fline != end_lines())
+      MarkLineDirty(fline);
  }

  // Either generate a reflow command to reflow the dirty child or 
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@ -80,7 +80,6 @@ public:
  typedef nsLineList::reverse_iterator          reverse_line_iterator;
  typedef nsLineList::const_reverse_iterator    const_reverse_line_iterator;

-protected:
  line_iterator begin_lines() { return mLines.begin(); }
  line_iterator end_lines() { return mLines.end(); }
  const_line_iterator begin_lines() const { return mLines.begin(); }
@ -90,7 +89,6 @@ protected:
  const_reverse_line_iterator rbegin_lines() const { return mLines.rbegin(); }
  const_reverse_line_iterator rend_lines() const { return mLines.rend(); }

-public:
  friend nsresult NS_NewBlockFrame(nsIPresShell* aPresShell, nsIFrame** aNewFrame, PRUint32 aFlags);

  // nsISupports
@ -188,10 +186,9 @@ public:
    */
  nsIFrame* GetTopBlockChild();

-  // returns true on success and false if aFoundLine is set to end_lines()
-  PRBool FindLineFor(nsIFrame* aFrame,
-                     PRBool* aIsFloaterResult,
-                     line_iterator* aFoundLine);
+  // Returns the line containing aFrame, or end_lines() if the frame
+  // isn't in the block.
+  line_iterator FindLineFor(nsIFrame* aFrame);

  static nsresult GetCurrentLine(nsBlockReflowState *aState, nsLineBox **aOutCurrentLine);

@ -450,10 +447,6 @@ protected:
                     nsIRenderingContext& aRenderingContext,
                     const nsRect& aDirtyRect);

-  void RememberFloaterDamage(nsBlockReflowState& aState,
-                             nsLineBox* aLine,
-                             const nsRect& aOldCombinedArea);
-
  void PropagateFloaterDamage(nsBlockReflowState& aState,
                              nsLineBox* aLine,
                              nscoord aDeltaY);
--- a/layout/generic/nsBlockReflowState.cpp
+++ b/layout/generic/nsBlockReflowState.cpp
@ -846,6 +846,11 @@ nsBlockReflowState::FlowAndPlaceFloater(nsFloaterCache* aFloaterCache,
  // for it to live.
  nsRect region;

+  // The floater's old region, so we can propagate damage.
+  nsRect oldRegion;
+  floater->GetRect(oldRegion);
+  oldRegion.Inflate(aFloaterCache->mMargins);
+
  // Advance mY to mLastFloaterY (if it's not past it already) to
  // enforce 9.5.1 rule [2]; i.e., make sure that a float isn't
  // ``above'' another float that preceded it in the flow.
@ -1003,6 +1008,18 @@ nsBlockReflowState::FlowAndPlaceFloater(nsFloaterCache* aFloaterCache,
    NS_ABORT_IF_FALSE(NS_SUCCEEDED(rv), "bad floater placement");
  }

+  // If the floater's dimensions have changed, note the damage in the
+  // space manager.
+  if (region != oldRegion) {
+    // XXXwaterson conservative: we could probably get away with noting
+    // less damage; e.g., if only height has changed, then only note the
+    // area into which the float has grown or from which the float has
+    // shrunk.
+    nscoord top = NS_MIN(region.y, oldRegion.y);
+    nscoord bottom = NS_MAX(region.YMost(), oldRegion.YMost());
+    mSpaceManager->IncludeInDamage(top, bottom);
+  }
+
  // Save away the floaters region in the spacemanager, after making
  // it relative to the containing block's frame instead of relative
  // to the spacemanager translation (which is inset by the
--- a/layout/generic/nsHTMLReflowState.cpp
+++ b/layout/generic/nsHTMLReflowState.cpp
@ -855,15 +855,13 @@ nsHTMLReflowState::CalculateHypotheticalBox(nsIPresContext*    aPresContext,
    // The element would have been block-level which means it would be below
    // the line containing the placeholder frame
    if (aBlockFrame) {
-      nsIFrame*   blockChild;
-      nsBlockFrame::line_iterator lineBox;
-      PRBool      isFloater;
      nsBlockFrame* blockFrame = NS_STATIC_CAST(nsBlockFrame*, aBlockFrame);

      // We need the immediate child of the block frame, and that may not be
      // the placeholder frame
-      blockChild = FindImmediateChildOf(aBlockFrame, aPlaceholderFrame);
-      if (blockFrame->FindLineFor(blockChild, &isFloater, &lineBox)) {
+      nsIFrame *blockChild = FindImmediateChildOf(aBlockFrame, aPlaceholderFrame);
+      nsBlockFrame::line_iterator lineBox = blockFrame->FindLineFor(blockChild);
+      if (lineBox != blockFrame->end_lines()) {
        // The top of the hypothetical box is just below the line containing
        // the placeholder
        aHypotheticalBox.mTop = lineBox->mBounds.YMost();
--- a/layout/html/base/src/nsBlockFrame.cpp
+++ b/layout/html/base/src/nsBlockFrame.cpp
@ -1561,9 +1561,7 @@ nsresult
 nsBlockFrame::PrepareChildIncrementalReflow(nsBlockReflowState& aState)
 {
  // Determine the line being impacted
-  PRBool isFloater;
-  line_iterator line;
-  FindLineFor(aState.mNextRCFrame, &isFloater, &line);
+  line_iterator line = FindLineFor(aState.mNextRCFrame);
  if (line == end_lines()) {
    // This assertion actually fires on lots of pages
    // (e.g., bugzilla, bugzilla query page), so limit it
@ -1581,12 +1579,6 @@ nsBlockFrame::PrepareChildIncrementalReflow(nsBlockReflowState& aState)
    return PrepareResizeReflow(aState);
  }

-  // XXX: temporary: If the child frame is a floater then punt
-  // XXX_perf XXXldb How big a perf problem is this?
-  if (isFloater) {
-    return PrepareResizeReflow(aState);
-  }
-
  if (line->IsInline()) {
    if (aState.GetFlag(BRS_COMPUTEMAXWIDTH)) {
      // We've been asked to compute the maximum width of the block
@ -1653,9 +1645,7 @@ nsBlockFrame::RetargetInlineIncrementalReflow(nsBlockReflowState &aState,
 #ifdef DEBUG
    // Paranoia. Ensure that the prev-in-flow is really in the
    // previous line.
-    PRBool dummy;
-    line_iterator check;
-    FindLineFor(aState.mNextRCFrame, &dummy, &check);
+    line_iterator check = FindLineFor(aState.mNextRCFrame);
    NS_ASSERTION(check == aLine, "prev-in-flow not in previous linebox");
 #endif

@ -1947,10 +1937,8 @@ nsBlockFrame::PrepareResizeReflow(nsBlockReflowState& aState)

 //----------------------------------------

-PRBool
-nsBlockFrame::FindLineFor(nsIFrame* aFrame,
-                          PRBool* aIsFloaterResult,
-                          line_iterator* aFoundLine)
+nsBlockFrame::line_iterator
+nsBlockFrame::FindLineFor(nsIFrame* aFrame)
 {
  // This assertion actually fires on lots of pages (e.g., bugzilla,
  // bugzilla query page), so limit it to a few people until we fix the
@ -1960,30 +1948,27 @@ nsBlockFrame::FindLineFor(nsIFrame* aFrame,
  NS_PRECONDITION(aFrame, "why pass a null frame?");
 #endif

-  PRBool isFloater = PR_FALSE;
  line_iterator line = begin_lines(),
                line_end = end_lines();
  for ( ; line != line_end; ++line) {
-    if (line->Contains(aFrame)) {
-      break;
-    }
-    // XXXldb what does this do?
+    // If the target frame is in-flow, and this line contains the it,
+    // then we've found our line.
+    if (line->Contains(aFrame))
+      return line;
+
+    // If the target frame is floated, and this line contains the
+    // floater's placeholder, then we've found our line.
    if (line->HasFloaters()) {
-      nsFloaterCache* fc = line->GetFirstFloater();
-      while (fc) {
-        if (aFrame == fc->mPlaceholder->GetOutOfFlowFrame()) {
-          isFloater = PR_TRUE;
-          goto done;
-        }
-        fc = fc->Next();
+      for (nsFloaterCache *fc = line->GetFirstFloater();
+           fc != nsnull;
+           fc = fc->Next()) {
+        if (aFrame == fc->mPlaceholder->GetOutOfFlowFrame())
+          return line;
      }
-    }
+    }      
  }

- done:
-  *aIsFloaterResult = isFloater;
-  *aFoundLine = line;
-  return (line != line_end);
+  return line_end;
 }

 // SEC: added GetCurrentLine() for bug 45152
@ -1997,46 +1982,10 @@ nsBlockFrame::GetCurrentLine(nsBlockReflowState *aState, nsLineBox ** aOutCurren
  return NS_OK;  
 }

-/**
- * Remember regions of reflow damage from floaters that changed size (so
- * far, only vertically, which is a bug) in the space manager so that we
- * can mark any impacted lines dirty in |PropagateFloaterDamage|.
- */
-void
-nsBlockFrame::RememberFloaterDamage(nsBlockReflowState& aState,
-                                    nsLineBox* aLine,
-                                    const nsRect& aOldCombinedArea)
-{
-  nsRect lineCombinedArea;
-  aLine->GetCombinedArea(&lineCombinedArea);
-  if (lineCombinedArea != aLine->mBounds &&
-      lineCombinedArea != aOldCombinedArea) {
-    // The line's combined-area changed. Therefore we need to damage
-    // the lines below that were previously (or are now) impacted by
-    // the change. It's possible that a floater shrunk or grew so
-    // use the larger of the impacted area.
-
-    // XXXldb If just the width of the floater changed, then this code
-    // won't be triggered and the code below (in |PropagateFloaterDamage|)
-    // won't kick in for "non-Block lines".  See
-    // "XXX: Maybe the floater itself changed size?" below.
-    //
-    // This is a major flaw in this code.
-    //
-    nscoord newYMost = lineCombinedArea.YMost();
-    nscoord oldYMost = aOldCombinedArea.YMost();
-    nscoord impactYB = newYMost < oldYMost ? oldYMost : newYMost;
-    nscoord impactYA = lineCombinedArea.y;
-    nsSpaceManager *spaceManager = aState.mReflowState.mSpaceManager;
-    spaceManager->IncludeInDamage(impactYA, impactYB);
-  }
-}
-
 /**
 * Propagate reflow "damage" from from earlier lines to the current
 * line.  The reflow damage comes from the following sources:
- *  1. The regions of floater damage remembered in
- *     |RememberFloaterDamage|.
+ *  1. The regions of floater damage remembered during reflow.
 *  2. The combination of nonzero |aDeltaY| and any impact by a floater,
 *     either the previous reflow or now.
 *
@ -2279,11 +2228,6 @@ nsBlockFrame::ReflowDirtyLines(nsBlockReflowState& aState)
      } else {
        deltaY = line->mBounds.YMost() - oldYMost;
      }
-
-      // Remember any things that could potentially be changes in the
-      // positions of floaters in this line so that we later damage the
-      // lines adjacent to those changes.
-      RememberFloaterDamage(aState, line, oldCombinedArea);
    } else {
      if (deltaY != 0)
        SlideLine(aState, line, deltaY);
@ -5898,22 +5842,9 @@ nsBlockFrame::ReflowDirtyChild(nsIPresShell* aPresShell, nsIFrame* aChild)
    }

    // Mark the line containing the child frame dirty.
-    PRBool isFloater;
-    line_iterator fline;
-    FindLineFor(aChild, &isFloater, &fline);
-    
-    if (!isFloater) {
-      if (fline != end_lines())
-        MarkLineDirty(fline);
-    }
-    else {
-      // XXXldb Could we do this more efficiently?
-      for (line_iterator line = begin_lines(), line_end = end_lines();
-           line != line_end;
-           ++line) {
-        line->MarkDirty();
-      }
-    }
+    line_iterator fline = FindLineFor(aChild);
+    if (fline != end_lines())
+      MarkLineDirty(fline);
  }

  // Either generate a reflow command to reflow the dirty child or 
--- a/layout/html/base/src/nsBlockFrame.h
+++ b/layout/html/base/src/nsBlockFrame.h
@ -80,7 +80,6 @@ public:
  typedef nsLineList::reverse_iterator          reverse_line_iterator;
  typedef nsLineList::const_reverse_iterator    const_reverse_line_iterator;

-protected:
  line_iterator begin_lines() { return mLines.begin(); }
  line_iterator end_lines() { return mLines.end(); }
  const_line_iterator begin_lines() const { return mLines.begin(); }
@ -90,7 +89,6 @@ protected:
  const_reverse_line_iterator rbegin_lines() const { return mLines.rbegin(); }
  const_reverse_line_iterator rend_lines() const { return mLines.rend(); }

-public:
  friend nsresult NS_NewBlockFrame(nsIPresShell* aPresShell, nsIFrame** aNewFrame, PRUint32 aFlags);

  // nsISupports
@ -188,10 +186,9 @@ public:
    */
  nsIFrame* GetTopBlockChild();

-  // returns true on success and false if aFoundLine is set to end_lines()
-  PRBool FindLineFor(nsIFrame* aFrame,
-                     PRBool* aIsFloaterResult,
-                     line_iterator* aFoundLine);
+  // Returns the line containing aFrame, or end_lines() if the frame
+  // isn't in the block.
+  line_iterator FindLineFor(nsIFrame* aFrame);

  static nsresult GetCurrentLine(nsBlockReflowState *aState, nsLineBox **aOutCurrentLine);

@ -450,10 +447,6 @@ protected:
                     nsIRenderingContext& aRenderingContext,
                     const nsRect& aDirtyRect);

-  void RememberFloaterDamage(nsBlockReflowState& aState,
-                             nsLineBox* aLine,
-                             const nsRect& aOldCombinedArea);
-
  void PropagateFloaterDamage(nsBlockReflowState& aState,
                              nsLineBox* aLine,
                              nscoord aDeltaY);
--- a/layout/html/base/src/nsBlockReflowState.cpp
+++ b/layout/html/base/src/nsBlockReflowState.cpp
@ -846,6 +846,11 @@ nsBlockReflowState::FlowAndPlaceFloater(nsFloaterCache* aFloaterCache,
  // for it to live.
  nsRect region;

+  // The floater's old region, so we can propagate damage.
+  nsRect oldRegion;
+  floater->GetRect(oldRegion);
+  oldRegion.Inflate(aFloaterCache->mMargins);
+
  // Advance mY to mLastFloaterY (if it's not past it already) to
  // enforce 9.5.1 rule [2]; i.e., make sure that a float isn't
  // ``above'' another float that preceded it in the flow.
@ -1003,6 +1008,18 @@ nsBlockReflowState::FlowAndPlaceFloater(nsFloaterCache* aFloaterCache,
    NS_ABORT_IF_FALSE(NS_SUCCEEDED(rv), "bad floater placement");
  }

+  // If the floater's dimensions have changed, note the damage in the
+  // space manager.
+  if (region != oldRegion) {
+    // XXXwaterson conservative: we could probably get away with noting
+    // less damage; e.g., if only height has changed, then only note the
+    // area into which the float has grown or from which the float has
+    // shrunk.
+    nscoord top = NS_MIN(region.y, oldRegion.y);
+    nscoord bottom = NS_MAX(region.YMost(), oldRegion.YMost());
+    mSpaceManager->IncludeInDamage(top, bottom);
+  }
+
  // Save away the floaters region in the spacemanager, after making
  // it relative to the containing block's frame instead of relative
  // to the spacemanager translation (which is inset by the
--- a/layout/html/base/src/nsHTMLReflowState.cpp
+++ b/layout/html/base/src/nsHTMLReflowState.cpp
@ -855,15 +855,13 @@ nsHTMLReflowState::CalculateHypotheticalBox(nsIPresContext*    aPresContext,
    // The element would have been block-level which means it would be below
    // the line containing the placeholder frame
    if (aBlockFrame) {
-      nsIFrame*   blockChild;
-      nsBlockFrame::line_iterator lineBox;
-      PRBool      isFloater;
      nsBlockFrame* blockFrame = NS_STATIC_CAST(nsBlockFrame*, aBlockFrame);

      // We need the immediate child of the block frame, and that may not be
      // the placeholder frame
-      blockChild = FindImmediateChildOf(aBlockFrame, aPlaceholderFrame);
-      if (blockFrame->FindLineFor(blockChild, &isFloater, &lineBox)) {
+      nsIFrame *blockChild = FindImmediateChildOf(aBlockFrame, aPlaceholderFrame);
+      nsBlockFrame::line_iterator lineBox = blockFrame->FindLineFor(blockChild);
+      if (lineBox != blockFrame->end_lines()) {
        // The top of the hypothetical box is just below the line containing
        // the placeholder
        aHypotheticalBox.mTop = lineBox->mBounds.YMost();