line-height patch

David Hyatt hyatt at apple.com
Fri Jan 30 20:15:09 CET 2004


This patch helps performance for us (about ~1.5% improvement), since 
our fontMetrics().lineSpacing() method is quite slow.  This patch moves 
m_lineHeight off RenderText and on to RenderFlow, and it makes sure 
that when you ask for the first-line lineHeight that you still use the 
cached value when no CSS value is given.

Hopefully it will yield similar beneficial results for you.

-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/ChangeLog,v
retrieving revision 1.2485
diff -u -p -r1.2485 ChangeLog
--- ChangeLog	2004/01/30 09:06:44	1.2485
+++ ChangeLog	2004/01/30 19:10:12
@@ -1,5 +1,32 @@
 2004-01-30  David Hyatt  <hyatt at apple.com>
 
+	Make m_lineHeight be cached on RenderFlow instead of RenderText and avoid recomputing it so much when it
+	is not set by CSS (since calls to fontMetrics().lineSpacing() are expensive).
+
+	Yields ~1.5% performance improvement.
+	
+        Reviewed by darin
+
+        * khtml/rendering/render_block.cpp:
+        (khtml::RenderBlock::setStyle):
+        * khtml/rendering/render_flow.cpp:
+        (RenderFlow::lineHeight):
+        * khtml/rendering/render_flow.h:
+        * khtml/rendering/render_inline.cpp:
+        (RenderInline::setStyle):
+        * khtml/rendering/render_object.cpp:
+        (RenderObject::verticalPositionHint):
+        (RenderObject::lineHeight):
+        * khtml/rendering/render_object.h:
+        * khtml/rendering/render_text.cpp:
+        (RenderText::setStyle):
+        (RenderText::checkSelectionPointIgnoringContinuations):
+        (RenderText::height):
+        (RenderText::lineHeight):
+        * khtml/rendering/render_text.h:
+
+2004-01-30  David Hyatt  <hyatt at apple.com>
+
 	Disable XBL.  The loadBindings call was taking 0.1-0.25%.  While I know how to get rid of this overhead,
 	it's easier for now to just disable all of XBL.
 
Index: khtml/rendering/render_block.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_block.cpp,v
retrieving revision 1.91
diff -u -p -r1.91 khtml/rendering/render_block.cpp
--- khtml/rendering/render_block.cpp	2004/01/26 21:56:41	1.91
+++ khtml/rendering/render_block.cpp	2004/01/30 19:10:28
@@ -90,6 +90,8 @@ void RenderBlock::setStyle(RenderStyle* 
         child = child->nextSibling();
     }
 
+    m_lineHeight = -1;
+
     // Update pseudos for :before and :after now.
     updatePseudoChild(RenderStyle::BEFORE, firstChild());
     updatePseudoChild(RenderStyle::AFTER, lastChild());
Index: khtml/rendering/render_flow.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_flow.cpp,v
retrieving revision 1.120
diff -u -p -r1.120 khtml/rendering/render_flow.cpp
--- khtml/rendering/render_flow.cpp	2004/01/26 21:56:41	1.120
+++ khtml/rendering/render_flow.cpp	2004/01/30 19:10:28
@@ -151,6 +151,26 @@ void RenderFlow::detach()
     RenderBox::detach();
 }
 
+short RenderFlow::lineHeight(bool firstLine, bool isRootLineBox) const
+{
+    if (firstLine) {
+        RenderStyle* s = style(firstLine);
+        Length lh = s->lineHeight();
+        if (lh.value < 0) {
+	    if (m_lineHeight == -1)
+	      m_lineHeight = RenderObject::lineHeight(false);
+	    return m_lineHeight;
+	}
+        if (lh.isPercent())
+            return lh.minWidth(s->font().pixelSize());
+        return lh.value;
+    }
+
+    if (m_lineHeight == -1)
+        m_lineHeight = RenderObject::lineHeight(false);
+    return m_lineHeight;
+}
+
 InlineBox* RenderFlow::createInlineBox(bool makePlaceHolderBox, bool isRootLineBox)
 {
     if (!isRootLineBox &&
Index: khtml/rendering/render_flow.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_flow.h,v
retrieving revision 1.47
diff -u -p -r1.47 khtml/rendering/render_flow.h
--- khtml/rendering/render_flow.h	2004/01/23 18:55:45	1.47
+++ khtml/rendering/render_flow.h	2004/01/30 19:10:29
@@ -59,6 +59,8 @@ public:
     void deleteLineBoxes();
     virtual void detach();
 
+    virtual short lineHeight(bool firstLine, bool isRootLineBox=false) const;
+    
     InlineFlowBox* firstLineBox() const { return m_firstLineBox; }
     InlineFlowBox* lastLineBox() const { return m_lastLineBox; }
 
@@ -89,6 +91,8 @@ protected:
     // For inline flows, each box represents a portion of that inline.
     InlineFlowBox* m_firstLineBox;
     InlineFlowBox* m_lastLineBox;
+    
+    mutable short m_lineHeight;
 };
 
     
Index: khtml/rendering/render_inline.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_inline.cpp,v
retrieving revision 1.29
diff -u -p -r1.29 khtml/rendering/render_inline.cpp
--- khtml/rendering/render_inline.cpp	2003/12/17 23:48:53	1.29
+++ khtml/rendering/render_inline.cpp	2004/01/30 19:10:29
@@ -59,6 +59,8 @@ void RenderInline::setStyle(RenderStyle*
         currCont = currCont->continuation();
     }
 
+    m_lineHeight = -1;
+    
     // Update pseudos for :before and :after now.
     updatePseudoChild(RenderStyle::BEFORE, firstChild());
     updatePseudoChild(RenderStyle::AFTER, lastChild());
Index: khtml/rendering/render_object.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_object.cpp,v
retrieving revision 1.128
diff -u -p -r1.128 khtml/rendering/render_object.cpp
--- khtml/rendering/render_object.cpp	2004/01/29 00:22:29	1.128
+++ khtml/rendering/render_object.cpp	2004/01/30 19:10:32
@@ -1844,7 +1844,7 @@ short RenderObject::verticalPositionHint
     if ( m_verticalPosition == PositionUndefined || firstLine ) {
 	vpos = getVerticalPosition( firstLine );
 	if ( !firstLine )
-	    const_cast<RenderObject *>(this)->m_verticalPosition = vpos;
+	    m_verticalPosition = vpos;
     }
     return vpos;
 
@@ -1909,14 +1909,16 @@ short RenderObject::getVerticalPosition(
 
 short RenderObject::lineHeight( bool firstLine, bool ) const
 {
-    Length lh = style(firstLine)->lineHeight();
+    RenderStyle* s = style(firstLine);
+    
+    Length lh = s->lineHeight();
 
     // its "unset", choose nice default
-    if ( lh.value < 0 )
-        return style(firstLine)->fontMetrics().lineSpacing();
+    if (lh.value < 0)
+        return s->fontMetrics().lineSpacing();
 
-    if ( lh.isPercent() )
-        return lh.minWidth( style(firstLine)->font().pixelSize() );
+    if (lh.isPercent())
+        return lh.minWidth(s->font().pixelSize());
 
     // its fixed
     return lh.value;
Index: khtml/rendering/render_object.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_object.h,v
retrieving revision 1.101
diff -u -p -r1.101 khtml/rendering/render_object.h
--- khtml/rendering/render_object.h	2004/01/29 00:22:29	1.101
+++ khtml/rendering/render_object.h	2004/01/30 19:10:33
@@ -733,7 +733,7 @@ private:
     RenderObject *m_previous;
     RenderObject *m_next;
 
-    short m_verticalPosition;
+    mutable short m_verticalPosition;
 
     bool m_needsLayout               : 1;
     bool m_normalChildNeedsLayout    : 1;
Index: khtml/rendering/render_text.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_text.cpp,v
retrieving revision 1.106
diff -u -p -r1.106 khtml/rendering/render_text.cpp
--- khtml/rendering/render_text.cpp	2004/01/29 02:16:29	1.106
+++ khtml/rendering/render_text.cpp	2004/01/30 19:10:35
@@ -275,7 +275,6 @@ void RenderText::setStyle(RenderStyle *_
                                    (style() && style()->textTransform() != _style->textTransform());
 
         RenderObject::setStyle( _style );
-        m_lineHeight = RenderObject::lineHeight(false);
 
         if (needToTransformText) {
             DOM::DOMStringImpl* textToTransform = originalString();
@@ -403,7 +402,7 @@ FindSelectionResult RenderText::checkSel
     for (InlineTextBox* s = firstTextBox(); s; s = s->nextTextBox()) {
         int result;
         const Font *f = htmlFont(s == firstTextBox());
-        result = s->checkSelectionPoint(_x, _y, _tx, _ty, f, this, offset, m_lineHeight);
+        result = s->checkSelectionPoint(_x, _y, _tx, _ty, f, this, offset, lineHeight(false));
 
 //         kdDebug(6040) << "RenderText::checkSelectionPoint " << this << " line " << si << " result=" << result << " offset=" << offset << endl;
         if ( result == SelectionPointInside ) // x,y is inside the InlineTextBox
@@ -1171,16 +1170,13 @@ int RenderText::height() const
     // FIXME: Why use line-height? Shouldn't we be adding in the height of the last text box? -dwh
     int retval = 0;
     if (firstTextBox())
-        retval = lastTextBox()->m_y + m_lineHeight - firstTextBox()->m_y;
+        retval = lastTextBox()->m_y + lineHeight(false) - firstTextBox()->m_y;
     return retval;
 }
 
-short RenderText::lineHeight( bool firstLine, bool ) const
+short RenderText::lineHeight( bool firstLine, bool isRootLineBox) const
 {
-    if ( firstLine )
- 	return RenderObject::lineHeight( firstLine );
-
-    return m_lineHeight;
+    return parent()->lineHeight(firstLine, isRootLineBox);
 }
 
 short RenderText::baselinePosition( bool firstLine, bool ) const
Index: khtml/rendering/render_text.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_text.h,v
retrieving revision 1.46
diff -u -p -r1.46 khtml/rendering/render_text.h
--- khtml/rendering/render_text.h	2004/01/29 00:22:29	1.46
+++ khtml/rendering/render_text.h	2004/01/30 19:10:36
@@ -222,7 +222,6 @@ protected: // members
     InlineTextBox* m_firstTextBox;
     InlineTextBox* m_lastTextBox;
     
-    short m_lineHeight;
     short m_minWidth;
     short m_maxWidth;
     short m_beginMinWidth;
-------------- next part --------------


dave


More information about the Khtml-devel mailing list