Optimize packing of RootInlineBox
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2018 05:09:21 +0000 (05:09 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2018 05:09:21 +0000 (05:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187430

Reviewed by Zalan Bujtas.

In InlineBox, move the m_logicalWidth float up next to the m_expansion float with m_topLeft next; this
avoids padding of 4 bytes after this float.

In InlineFlowBox, move the bitfields before the pointers so they can snug up into the
4 bytes after m_expansion in the base class.

The comment about m_lineBreakPos's padding in RootInlineBox is wrong; just move it to the end
to avoid padding before the m_lineBreakObj pointer.

Make m_logicalWidth private and have derived classes use the accessor.

Make EllipsisBox 4 bytes smaller too.

* rendering/EllipsisBox.cpp:
(WebCore::EllipsisBox::EllipsisBox):
(WebCore::EllipsisBox::paintMarkupBox):
(WebCore::EllipsisBox::nodeAtPoint):
* rendering/EllipsisBox.h:
* rendering/InlineBox.h:
(WebCore::InlineBox::InlineBox):
* rendering/InlineFlowBox.cpp:
* rendering/InlineFlowBox.h:
(WebCore::InlineFlowBox::InlineFlowBox):
(WebCore::InlineFlowBox::frameRectIncludingLineHeight const):
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::localSelectionRect const):
(WebCore::InlineTextBox::paint):
(WebCore::InlineTextBox::paintPlatformDocumentMarker):
(WebCore::InlineTextBox::paintMarkedTextBackground):
(WebCore::InlineTextBox::paintCompositionUnderline const):
* rendering/RootInlineBox.cpp:
(WebCore::RootInlineBox::RootInlineBox):
* rendering/RootInlineBox.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233634 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/EllipsisBox.cpp
Source/WebCore/rendering/EllipsisBox.h
Source/WebCore/rendering/InlineBox.h
Source/WebCore/rendering/InlineFlowBox.cpp
Source/WebCore/rendering/InlineFlowBox.h
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/RootInlineBox.cpp
Source/WebCore/rendering/RootInlineBox.h

index bd69f13..33fce42 100644 (file)
@@ -1,3 +1,44 @@
+2018-07-08  Simon Fraser  <simon.fraser@apple.com>
+
+        Optimize packing of RootInlineBox
+        https://bugs.webkit.org/show_bug.cgi?id=187430
+
+        Reviewed by Zalan Bujtas.
+
+        In InlineBox, move the m_logicalWidth float up next to the m_expansion float with m_topLeft next; this
+        avoids padding of 4 bytes after this float.
+        
+        In InlineFlowBox, move the bitfields before the pointers so they can snug up into the
+        4 bytes after m_expansion in the base class.
+        
+        The comment about m_lineBreakPos's padding in RootInlineBox is wrong; just move it to the end
+        to avoid padding before the m_lineBreakObj pointer.
+        
+        Make m_logicalWidth private and have derived classes use the accessor.
+        
+        Make EllipsisBox 4 bytes smaller too.
+
+        * rendering/EllipsisBox.cpp:
+        (WebCore::EllipsisBox::EllipsisBox):
+        (WebCore::EllipsisBox::paintMarkupBox):
+        (WebCore::EllipsisBox::nodeAtPoint):
+        * rendering/EllipsisBox.h:
+        * rendering/InlineBox.h:
+        (WebCore::InlineBox::InlineBox):
+        * rendering/InlineFlowBox.cpp:
+        * rendering/InlineFlowBox.h:
+        (WebCore::InlineFlowBox::InlineFlowBox):
+        (WebCore::InlineFlowBox::frameRectIncludingLineHeight const):
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::localSelectionRect const):
+        (WebCore::InlineTextBox::paint):
+        (WebCore::InlineTextBox::paintPlatformDocumentMarker):
+        (WebCore::InlineTextBox::paintMarkedTextBackground):
+        (WebCore::InlineTextBox::paintCompositionUnderline const):
+        * rendering/RootInlineBox.cpp:
+        (WebCore::RootInlineBox::RootInlineBox):
+        * rendering/RootInlineBox.h:
+
 2018-07-08  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] A number of tests report an incorrect computed offset
index 9524479..81054a3 100644 (file)
@@ -39,7 +39,6 @@ EllipsisBox::EllipsisBox(RenderBlockFlow& renderer, const AtomicString& ellipsis
     , m_shouldPaintMarkupBox(markupBox)
     , m_height(height)
     , m_str(ellipsisStr)
-    , m_selectionState(RenderObject::SelectionNone)
 {
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
     m_isEverInChildList = false;
@@ -108,7 +107,7 @@ void EllipsisBox::paintMarkupBox(PaintInfo& paintInfo, const LayoutPoint& paintO
         return;
 
     LayoutPoint adjustedPaintOffset = paintOffset;
-    adjustedPaintOffset.move(x() + m_logicalWidth - markupBox->x(),
+    adjustedPaintOffset.move(x() + logicalWidth() - markupBox->x(),
         y() + style.fontMetrics().ascent() - (markupBox->y() + markupBox->lineStyle().fontMetrics().ascent()));
     markupBox->paint(paintInfo, adjustedPaintOffset, lineTop, lineBottom);
 }
@@ -153,7 +152,7 @@ bool EllipsisBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& resu
     // Hit test the markup box.
     if (InlineBox* markupBox = this->markupBox()) {
         const RenderStyle& lineStyle = this->lineStyle();
-        LayoutUnit mtx = adjustedLocation.x() + m_logicalWidth - markupBox->x();
+        LayoutUnit mtx = adjustedLocation.x() + logicalWidth() - markupBox->x();
         LayoutUnit mty = adjustedLocation.y() + lineStyle.fontMetrics().ascent() - (markupBox->y() + markupBox->lineStyle().fontMetrics().ascent());
         if (markupBox->nodeAtPoint(request, result, locationInContainer, LayoutPoint(mtx, mty), lineTop, lineBottom, hitTestAction)) {
             blockFlow().updateHitTestResult(result, locationInContainer.point() - LayoutSize(mtx, mty));
@@ -161,7 +160,7 @@ bool EllipsisBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& resu
         }
     }
 
-    LayoutRect boundsRect(adjustedLocation, LayoutSize(m_logicalWidth, m_height));
+    LayoutRect boundsRect(adjustedLocation, LayoutSize(logicalWidth(), m_height));
     if (visibleToHitTesting() && boundsRect.intersects(HitTestLocation::rectForPoint(locationInContainer.point(), 0, 0, 0, 0))) {
         blockFlow().updateHitTestResult(result, locationInContainer.point() - toLayoutSize(adjustedLocation));
         if (result.addNodeToListBasedTestResult(blockFlow().element(), request, locationInContainer, boundsRect) == HitTestProgress::Stop)
index b61b84f..ebe1941 100644 (file)
@@ -46,9 +46,9 @@ private:
     InlineBox* markupBox() const;
 
     bool m_shouldPaintMarkupBox;
+    RenderObject::SelectionState m_selectionState { RenderObject::SelectionNone };
     int m_height;
     AtomicString m_str;
-    RenderObject::SelectionState m_selectionState;
 };
 
 } // namespace WebCore
index 77a6865..a05fe38 100644 (file)
@@ -294,7 +294,10 @@ private:
 
     RenderObject& m_renderer;
 
+private:
+    float m_logicalWidth { 0 };
     float m_expansion { 0 };
+    FloatPoint m_topLeft;
 
 #define ADD_BOOLEAN_BITFIELD(name, Name) \
     private:\
@@ -385,9 +388,9 @@ protected:
         , m_prev(prev)
         , m_parent(parent)
         , m_renderer(renderer)
-        , m_bitfields(firstLine, constructed, dirty, extracted, isHorizontal)
-        , m_topLeft(topLeft)
         , m_logicalWidth(logicalWidth)
+        , m_topLeft(topLeft)
+        , m_bitfields(firstLine, constructed, dirty, extracted, isHorizontal)
     {
     }
 
@@ -410,8 +413,6 @@ protected:
     bool extracted() const { return m_bitfields.extracted(); }
 
 protected:
-    FloatPoint m_topLeft;
-    float m_logicalWidth { 0 };
 
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
 private:
index 6bb9377..6adc6cb 100644 (file)
@@ -47,8 +47,8 @@ namespace WebCore {
 WTF_MAKE_ISO_ALLOCATED_IMPL(InlineFlowBox);
 
 struct SameSizeAsInlineFlowBox : public InlineBox {
-    void* pointers[5];
     uint32_t bitfields : 23;
+    void* pointers[5];
 };
 
 COMPILE_ASSERT(sizeof(InlineFlowBox) == sizeof(SameSizeAsInlineFlowBox), InlineFlowBox_should_stay_small);
index 6c4d58f..27c6229 100644 (file)
@@ -42,10 +42,6 @@ class InlineFlowBox : public InlineBox {
 public:
     explicit InlineFlowBox(RenderBoxModelObject& renderer)
         : InlineBox(renderer)
-        , m_firstChild(nullptr)
-        , m_lastChild(nullptr)
-        , m_prevLineBox(nullptr)
-        , m_nextLineBox(nullptr)
         , m_includeLogicalLeftEdge(false)
         , m_includeLogicalRightEdge(false)
         , m_descendantsHaveSameLineHeightAndBaseline(true)
@@ -56,6 +52,10 @@ public:
 #if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
         , m_hasBadChildList(false)
 #endif
+        , m_firstChild(nullptr)
+        , m_lastChild(nullptr)
+        , m_prevLineBox(nullptr)
+        , m_nextLineBox(nullptr)
     {
         // Internet Explorer and Firefox always create a marker for list items, even when the list-style-type is none.  We do not make a marker
         // in the list-style-type: none case, since it is wasteful to do so.  However, in order to match other browsers we have to pretend like
@@ -274,8 +274,8 @@ public:
     FloatRect frameRectIncludingLineHeight(LayoutUnit lineTop, LayoutUnit lineBottom) const
     {
         if (isHorizontal())
-            return FloatRect(m_topLeft.x(), lineTop, width(), lineBottom - lineTop);
-        return FloatRect(lineTop, m_topLeft.y(), lineBottom - lineTop, height());
+            return FloatRect(x(), lineTop, width(), lineBottom - lineTop);
+        return FloatRect(lineTop, y(), lineBottom - lineTop, height());
     }
     
     FloatRect logicalFrameRectIncludingLineHeight(LayoutUnit lineTop, LayoutUnit lineBottom) const
@@ -308,15 +308,6 @@ private:
     void addReplacedChildOverflow(const InlineBox*, LayoutRect& logicalLayoutOverflow, LayoutRect& logicalVisualOverflow);
     void constrainToLineTopAndBottomIfNeeded(LayoutRect&) const;
 
-protected:
-    RefPtr<RenderOverflow> m_overflow;
-
-    InlineBox* m_firstChild;
-    InlineBox* m_lastChild;
-    
-    InlineFlowBox* m_prevLineBox; // The previous box that also uses our RenderObject
-    InlineFlowBox* m_nextLineBox; // The next box that also uses our RenderObject
-
 private:
     unsigned m_includeLogicalLeftEdge : 1;
     unsigned m_includeLogicalRightEdge : 1;
@@ -346,6 +337,15 @@ protected:
 private:
     unsigned m_hasBadChildList : 1;
 #endif
+
+protected:
+    RefPtr<RenderOverflow> m_overflow;
+
+    InlineBox* m_firstChild;
+    InlineBox* m_lastChild;
+    
+    InlineFlowBox* m_prevLineBox; // The previous box that also uses our RenderObject
+    InlineFlowBox* m_nextLineBox; // The next box that also uses our RenderObject
 };
 
 #ifdef NDEBUG
index a8c19a3..034afbc 100644 (file)
@@ -203,7 +203,7 @@ LayoutRect InlineTextBox::localSelectionRect(unsigned startPos, unsigned endPos)
 
     TextRun textRun = createTextRun();
 
-    LayoutRect selectionRect = LayoutRect(LayoutPoint(logicalLeft(), selectionTop), LayoutSize(m_logicalWidth, selectionHeight));
+    LayoutRect selectionRect = LayoutRect(LayoutPoint(logicalLeft(), selectionTop), LayoutSize(logicalWidth(), selectionHeight));
     // Avoid measuring the text when the entire line box is selected as an optimization.
     if (sPos || ePos != textRun.length())
         lineFont().adjustSelectionRectForText(textRun, selectionRect, sPos, ePos);
@@ -474,7 +474,7 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
             // NOTE: WebKit's behavior differs from that of IE which appears to just overlay the ellipsis on top of the
             // truncated string i.e.  |Hello|CBA| -> |...lo|CBA|
             LayoutUnit widthOfVisibleText = renderer().width(m_start, m_truncation, textPos(), isFirstLine());
-            LayoutUnit widthOfHiddenText = m_logicalWidth - widthOfVisibleText;
+            LayoutUnit widthOfHiddenText = logicalWidth() - widthOfVisibleText;
             LayoutSize truncationOffset(isLeftToRightDirection() ? widthOfHiddenText : -widthOfHiddenText, 0);
             localPaintOffset.move(isHorizontal() ? truncationOffset : truncationOffset.transposedSize());
         }
@@ -669,7 +669,7 @@ void InlineTextBox::paintPlatformDocumentMarker(GraphicsContext& context, const
         return;
 
     float start = 0; // start of line to draw, relative to tx
-    float width = m_logicalWidth; // how much line to draw
+    float width = logicalWidth(); // how much line to draw
 
     // Avoid measuring the text when the entire line box is selected as an optimization.
     if (markedText.startOffset || markedText.endOffset != clampedOffset(end() + 1)) {
@@ -997,7 +997,7 @@ void InlineTextBox::paintMarkedTextBackground(PaintInfo& paintInfo, const FloatP
     LayoutUnit deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom - logicalBottom() : logicalTop() - selectionTop;
     LayoutUnit selectionHeight = std::max<LayoutUnit>(0, selectionBottom - selectionTop);
 
-    LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, m_logicalWidth, selectionHeight);
+    LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, logicalWidth(), selectionHeight);
     lineFont().adjustSelectionRectForText(textRun, selectionRect, clampedStartOffset, clampedEndOffset);
 
     // FIXME: Support painting combined text. See <https://bugs.webkit.org/show_bug.cgi?id=180993>.
@@ -1144,7 +1144,7 @@ void InlineTextBox::paintCompositionUnderline(PaintInfo& paintInfo, const FloatP
         return;
     
     float start = 0; // start of line to draw, relative to tx
-    float width = m_logicalWidth; // how much line to draw
+    float width = logicalWidth(); // how much line to draw
     bool useWholeWidth = true;
     unsigned paintStart = m_start;
     unsigned paintEnd = end() + 1; // end points at the last char, not past it
@@ -1163,7 +1163,7 @@ void InlineTextBox::paintCompositionUnderline(PaintInfo& paintInfo, const FloatP
     }
     if (!useWholeWidth) {
         width = renderer().width(paintStart, paintEnd - paintStart, textPos() + start, isFirstLine());
-        mirrorRTLSegment(m_logicalWidth, direction(), start, width);
+        mirrorRTLSegment(logicalWidth(), direction(), start, width);
     }
 
     // Thick marked text underlines are 2px thick as long as there is room for the 2px line under the baseline.
index c8c0e54..5da5de7 100644 (file)
@@ -63,7 +63,6 @@ static ContainingFragmentMap& containingFragmentMap(RenderBlockFlow& block)
 
 RootInlineBox::RootInlineBox(RenderBlockFlow& block)
     : InlineFlowBox(block)
-    , m_lineBreakPos(0)
 {
     setIsHorizontal(block.isHorizontalWritingMode());
 }
index dd9594f..379021e 100644 (file)
@@ -211,9 +211,6 @@ private:
 
     LayoutUnit beforeAnnotationsAdjustment() const;
 
-    // This folds into the padding at the end of InlineFlowBox on 64-bit.
-    unsigned m_lineBreakPos;
-
     // Where this line ended.  The exact object and the position within that object are stored so that
     // we can create an InlineIterator beginning just after the end of this line.
     WeakPtr<RenderObject> m_lineBreakObj;
@@ -231,6 +228,8 @@ private:
     // Floats hanging off the line are pushed into this vector during layout. It is only
     // good for as long as the line has not been marked dirty.
     std::unique_ptr<CleanLineFloatList> m_floats;
+
+    unsigned m_lineBreakPos { 0 };
 };
 
 inline RootInlineBox* RootInlineBox::nextRootBox() const