Bug 1428670 - Part 3: Store the effective container ISize within the FontInflationData. r=dbaron
authorJan Henning <jh+bugzilla@buttercookie.de>
Tue, 02 Oct 2018 15:23:17 +0000
changeset 487557 57e3c8d09acea5e0bff83ddbb78d385f71dd352d
parent 487556 87971e291470b0d0ed5be19f323312ae39ac5308
child 487558 ec51b039eef15fd55908f1fa1051b0624c4b4a46
push id246
push userfmarier@mozilla.com
push dateSat, 13 Oct 2018 00:15:40 +0000
reviewersdbaron
bugs1428670
milestone64.0a1
Bug 1428670 - Part 3: Store the effective container ISize within the FontInflationData. r=dbaron When one of the two factors governing the effective container width for font inflation changes, we need to mark all affected frames as dirty. While the visible area commonly changes because of viewport changes and we can catch those through the check for ISize resizes of the top-level frame ("!mFrame->GetParent() && isIResize"), it still seems nicer to move calculation of the effective container width into the FontInflationData itself, especially since the effective container width calculation in nsLayoutUtils is the only consumer of the NCAISize currently stored by the FontInflationData. That way - we can be sure that really all changes of the visible width will correctly mark all affected frames as dirty - can avoid repeatedly recalculating the effective container width - can also detect the cases where the effective container width actually remains the same after a change in one of its input factors and skip forcing a full dirty reflow for all descendants just because of font inflation. While the code in nsLayoutUtils was technically always using the writing mode (horizontal/vertical) of each individual frame for determining which dimension of the visible size should be used for clamping, just using the writing mode of the respective flow root should be enough, since each change in the writing mode should create a new flow root. This assumption should already hold today because 1. as per the Writing Modes CSS spec, a change in writing mode compared to its parent means that the affected block cannot be purely "inline", but at most "inline-block". 2. Generally, any non-inline frame will be marked as a font inflation container. 3. Any block frame whose writing direction doesn't match its parent will be a block formatting context, which implies NS_BLOCK_FLOAT_MGR. 4. Any block frame that has both NS_BLOCK_FLOAT_MGR set and is a font inflation container will also become a font inflation flow root. but because this chain of reasoning is not the most direct, we also add a corresponding assertion to better catch any potential bugs here. Differential Revision: https://phabricator.services.mozilla.com/D5578
layout/base/nsLayoutUtils.cpp
layout/generic/nsFontInflationData.cpp
layout/generic/nsFontInflationData.h
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -8283,33 +8283,27 @@ MinimumFontSizeFor(nsPresContext* aPresC
   nsIPresShell* presShell = aPresContext->PresShell();
 
   uint32_t emPerLine = presShell->FontSizeInflationEmPerLine();
   uint32_t minTwips = presShell->FontSizeInflationMinTwips();
   if (emPerLine == 0 && minTwips == 0) {
     return 0;
   }
 
-  // Clamp the container width to the device dimensions
-  nscoord iFrameISize = aWritingMode.IsVertical()
-    ? aPresContext->GetVisibleArea().height
-    : aPresContext->GetVisibleArea().width;
-  nscoord effectiveContainerISize = std::min(iFrameISize, aContainerISize);
-
   nscoord byLine = 0, byInch = 0;
   if (emPerLine != 0) {
-    byLine = effectiveContainerISize / emPerLine;
+    byLine = aContainerISize / emPerLine;
   }
   if (minTwips != 0) {
     // REVIEW: Is this giving us app units and sizes *not* counting
     // viewport scaling?
     gfxSize screenSize = aPresContext->ScreenSizeInchesForFontInflation();
     float deviceISizeInches = aWritingMode.IsVertical()
       ? screenSize.height : screenSize.width;
-    byInch = NSToCoordRound(effectiveContainerISize /
+    byInch = NSToCoordRound(aContainerISize /
                             (deviceISizeInches * 1440 /
                              minTwips ));
   }
   return std::max(byLine, byInch);
 }
 
 /* static */ float
 nsLayoutUtils::FontSizeInflationInner(const nsIFrame *aFrame,
@@ -8447,17 +8441,17 @@ nsLayoutUtils::InflationMinFontSizeFor(c
       // FIXME: The need to null-check here is sort of a bug, and might
       // lead to incorrect results.
       if (!data || !data->InflationEnabled()) {
         return 0;
       }
 
       return MinimumFontSizeFor(aFrame->PresContext(),
                                 aFrame->GetWritingMode(),
-                                data->EffectiveISize());
+                                data->UsableISize());
     }
   }
 
   MOZ_ASSERT(false, "root should always be container");
 
   return 0;
 }
 
--- a/layout/generic/nsFontInflationData.cpp
+++ b/layout/generic/nsFontInflationData.cpp
@@ -22,63 +22,65 @@ NS_DECLARE_FRAME_PROPERTY_DELETABLE(Font
 
 /* static */ nsFontInflationData*
 nsFontInflationData::FindFontInflationDataFor(const nsIFrame *aFrame)
 {
   // We have one set of font inflation data per block formatting context.
   const nsIFrame *bfc = FlowRootFor(aFrame);
   NS_ASSERTION(bfc->GetStateBits() & NS_FRAME_FONT_INFLATION_FLOW_ROOT,
                "should have found a flow root");
+  MOZ_ASSERT(aFrame->GetWritingMode().IsVertical() ==
+               bfc->GetWritingMode().IsVertical(),
+             "current writing mode should match that of our flow root");
 
   return bfc->GetProperty(FontInflationDataProperty());
 }
 
 /* static */ bool
 nsFontInflationData::UpdateFontInflationDataISizeFor(const ReflowInput& aReflowInput)
 {
   nsIFrame *bfc = aReflowInput.mFrame;
   NS_ASSERTION(bfc->GetStateBits() & NS_FRAME_FONT_INFLATION_FLOW_ROOT,
                "should have been given a flow root");
   nsFontInflationData *data = bfc->GetProperty(FontInflationDataProperty());
   bool oldInflationEnabled;
-  nscoord oldNCAISize;
+  nscoord oldUsableISize;
   if (data) {
-    oldNCAISize = data->mNCAISize;
+    oldUsableISize = data->mUsableISize;
     oldInflationEnabled = data->mInflationEnabled;
   } else {
     data = new nsFontInflationData(bfc);
     bfc->SetProperty(FontInflationDataProperty(), data);
-    oldNCAISize = -1;
+    oldUsableISize = -1;
     oldInflationEnabled = true; /* not relevant */
   }
 
   data->UpdateISize(aReflowInput);
 
   if (oldInflationEnabled != data->mInflationEnabled)
     return true;
 
-  return oldInflationEnabled &&
-         oldNCAISize != data->mNCAISize;
+  return oldInflationEnabled && oldUsableISize != data->mUsableISize;
 }
 
 /* static */ void
 nsFontInflationData::MarkFontInflationDataTextDirty(nsIFrame *aBFCFrame)
 {
   NS_ASSERTION(aBFCFrame->GetStateBits() & NS_FRAME_FONT_INFLATION_FLOW_ROOT,
                "should have been given a flow root");
 
   nsFontInflationData *data = aBFCFrame->GetProperty(FontInflationDataProperty());
   if (data) {
     data->MarkTextDirty();
   }
 }
 
 nsFontInflationData::nsFontInflationData(nsIFrame *aBFCFrame)
   : mBFCFrame(aBFCFrame)
-  , mNCAISize(0)
+  , mUsableISize(0)
   , mTextAmount(0)
   , mTextThreshold(0)
   , mInflationEnabled(false)
   , mTextDirty(true)
 {
 }
 
 /**
@@ -209,17 +211,33 @@ nsFontInflationData::UpdateISize(const R
   nscoord newTextThreshold = (newNCAISize * lineThreshold) / 100;
 
   if (mTextThreshold <= mTextAmount && mTextAmount < newTextThreshold) {
     // Because we truncate our scan when we hit sufficient text, we now
     // need to rescan.
     mTextDirty = true;
   }
 
-  mNCAISize = newNCAISize;
+  // Font inflation increases the font size for a given flow root so that the
+  // text is legible when we've zoomed such that the respective nearest common
+  // ancestor's (NCA) full inline-size (ISize) fills the screen. We assume how-
+  // ever that we don't want to zoom out further than the root iframe's ISize
+  // (i.e. the viewport for a top-level document, or the containing iframe
+  // otherwise), since in some cases zooming out further might not even be
+  // possible or make sense.
+  // Hence the ISize assumed to be usable for displaying text is limited to the
+  // visible area.
+  nsPresContext* presContext = bfc->PresContext();
+  MOZ_ASSERT(bfc->GetWritingMode().IsVertical() ==
+               nca->GetWritingMode().IsVertical(),
+             "writing mode of NCA should match that of its flow root");
+  nscoord iFrameISize = bfc->GetWritingMode().IsVertical()
+                          ? presContext->GetVisibleArea().height
+                          : presContext->GetVisibleArea().width;
+  mUsableISize = std::min(iFrameISize, newNCAISize);
   mTextThreshold = newTextThreshold;
   mInflationEnabled = mTextAmount >= mTextThreshold;
 }
 
 /* static */ nsIFrame*
 nsFontInflationData::FindEdgeInflatableFrameIn(nsIFrame* aFrame,
                                                SearchDirection aDirection)
 {
--- a/layout/generic/nsFontInflationData.h
+++ b/layout/generic/nsFontInflationData.h
@@ -14,32 +14,32 @@
 class nsFontInflationData
 {
   using ReflowInput = mozilla::ReflowInput;
 
 public:
 
   static nsFontInflationData* FindFontInflationDataFor(const nsIFrame *aFrame);
 
-  // Returns whether the effective width changed (which requires the
-  // caller to mark its descendants dirty
+  // Returns whether the usable width changed (which requires the
+  // caller to mark its descendants dirty)
   static bool
     UpdateFontInflationDataISizeFor(const ReflowInput& aReflowInput);
 
   static void MarkFontInflationDataTextDirty(nsIFrame *aFrame);
 
   bool InflationEnabled() {
     if (mTextDirty) {
       ScanText();
     }
     return mInflationEnabled;
   }
 
-  nscoord EffectiveISize() const {
-    return mNCAISize;
+  nscoord UsableISize() const {
+    return mUsableISize;
   }
 
 private:
 
   explicit nsFontInflationData(nsIFrame* aBFCFrame);
 
   nsFontInflationData(const nsFontInflationData&) = delete;
   void operator=(const nsFontInflationData&) = delete;
@@ -62,15 +62,15 @@ private:
   {
     while (!(aFrame->GetStateBits() & NS_FRAME_FONT_INFLATION_FLOW_ROOT)) {
       aFrame = aFrame->GetParent();
     }
     return aFrame;
   }
 
   nsIFrame *mBFCFrame;
-  nscoord mNCAISize;
+  nscoord mUsableISize;
   nscoord mTextAmount, mTextThreshold;
   bool mInflationEnabled; // for this BFC
   bool mTextDirty;
 };
 
 #endif /* !defined(nsFontInflationData_h_) */