fixes for border-collapsing

David Hyatt hyatt at apple.com
Tue Feb 24 00:45:43 CET 2004


The border-collapsing code contains a pretty terrible bug that can 
cause it to build an insane # of borders (making pages hang).  
Basically because css only specifies a partial order rather than a 
total order when comparing borders, the old code was getting confused 
and not recognizing two styles were the same when collecting the unique 
borders.

I also should have used QValueList instead of QPtrList, so I made that 
change too.

You can also see my code here that puts in border collapsing for this 
new PaintInfo struct Dirk added, so you can see how I merged that in.

Two examples of the problem are:

http://californiaracquetball.org/ranking_info.htm (really bad, hangs 
for multiple minutes)
http://www.skating.nf.ca/events_e.htm (not as bad, but should 
scroll/paint/load somewhat sluggishly)

Here's the fix:

-------------- next part --------------
Index: khtml/rendering/render_object.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_object.cpp,v
retrieving revision 1.133
diff -u -p -r1.133 khtml/rendering/render_object.cpp
--- khtml/rendering/render_object.cpp	2004/02/23 21:26:26	1.133
+++ khtml/rendering/render_object.cpp	2004/02/23 23:28:49
@@ -2116,7 +2116,7 @@ void RenderObject::updateWidgetPositions
 }
 #endif
 
-void RenderObject::collectBorders(QPtrList<CollapsedBorderValue>& borderStyles)
+void RenderObject::collectBorders(QValueList<CollapsedBorderValue>& borderStyles)
 {
     for (RenderObject* curr = firstChild(); curr; curr = curr->nextSibling())
         curr->collectBorders(borderStyles);
Index: khtml/rendering/render_object.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_object.h,v
retrieving revision 1.105
diff -u -p -r1.105 khtml/rendering/render_object.h
--- khtml/rendering/render_object.h	2004/02/23 21:26:26	1.105
+++ khtml/rendering/render_object.h	2004/02/23 23:28:49
@@ -614,7 +614,7 @@ public:
     virtual void setTable(RenderTable*) {};
 
     // Used by collapsed border tables.
-    virtual void collectBorders(QPtrList<CollapsedBorderValue>& borderStyles);
+    virtual void collectBorders(QValueList<CollapsedBorderValue>& borderStyles);
 
     // Repaint the entire object.  Called when, e.g., the color of a border changes, or when a border
     // style changes.
Index: khtml/rendering/render_table.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_table.cpp,v
retrieving revision 1.95
diff -u -p -r1.95 khtml/rendering/render_table.cpp
--- khtml/rendering/render_table.cpp	2004/02/23 21:26:26	1.95
+++ khtml/rendering/render_table.cpp	2004/02/23 23:28:49
@@ -442,18 +442,17 @@ void RenderTable::paint(PaintInfo& i, in
         if (child->isTableSection() || child == tCaption)
 	    child->paint(paintInfo, _tx, _ty);
 
-    if (collapseBorders() && 
-        (paintAction == PaintActionElementBackground || paintAction == PaintActionChildBackground)
-        && style()->visibility() == VISIBLE) {
+    if (collapseBorders() && paintAction == PaintActionChildBackground && style()->visibility() == VISIBLE) {
         // Collect all the unique border styles that we want to paint in a sorted list.  Once we
         // have all the styles sorted, we then do individual passes, painting each style of border
         // from lowest precedence to highest precedence.
-        QPtrList<CollapsedBorderValue> borderStyles;
-        borderStyles.setAutoDelete(true);
-        collectBorders(borderStyles);
         paintInfo.phase = PaintActionCollapsedTableBorders;
-        for (uint i = 0; i < borderStyles.count(); i++) {
-            m_currentBorder = borderStyles.at(i);
+        QValueList<CollapsedBorderValue> borderStyles;
+        collectBorders(borderStyles);
+        QValueListIterator<CollapsedBorderValue> it = borderStyles.begin();
+        QValueListIterator<CollapsedBorderValue> end = borderStyles.end();
+        for (; it != end; ++it) {
+            m_currentBorder = &(*it);
             for (RenderObject* child = firstChild(); child; child = child->nextSibling())
                 if (child->isTableSection())
                     child->paint(paintInfo, _tx, _ty);
@@ -1756,8 +1755,8 @@ void RenderTableCell::setStyle( RenderSt
 // (4) If border styles differ only in color, then a style set on a cell wins over one on a row, 
 // which wins over a row group, column, column group and, lastly, table. It is undefined which color 
 // is used when two elements of the same type disagree.
-static const CollapsedBorderValue compareBorders(const CollapsedBorderValue& border1, 
-                                                 const CollapsedBorderValue& border2)
+static CollapsedBorderValue compareBorders(const CollapsedBorderValue& border1, 
+                                           const CollapsedBorderValue& border2)
 {
     // Sanity check the values passed in.  If either is null, return the other.
     if (!border2.exists()) return border1;
@@ -2069,16 +2068,23 @@ void RenderTableCell::paint(PaintInfo& i
 #endif
 
     _tx += m_x;
-    _ty += m_y + _topExtra;
+    _ty += m_y;
 
     // check if we need to do anything at all...
     int os = 2*maximalOutlineSize(i.phase);
-    if (!overhangingContents() && ((_ty - _topExtra >= i.r.y() + i.r.height() + os)
-                                   || (_ty + m_height + _bottomExtra <= i.r.y() - os))) return;
-    RenderBlock::paintObject(i, _tx, _ty);
+    if (!overhangingContents() && ((_ty >= i.r.y() + i.r.height() + os)
+                                   || (_ty + _topExtra + m_height + _bottomExtra <= i.r.y() - os))) return;
+    
+    if (i.phase == PaintActionCollapsedTableBorders && style()->visibility() == VISIBLE) {
+        int w = width();
+        int h = height() + borderTopExtra() + borderBottomExtra();
+        paintCollapsedBorder(i.p, _tx, _ty, w, h);
+    }
+    else
+        RenderBlock::paintObject(i, _tx, _ty + _topExtra);
 
 #ifdef BOX_DEBUG
-    ::outlineBox( i.p, _tx, _ty - _topExtra, width(), height() + borderTopExtra() + borderBottomExtra());
+    ::outlineBox( i.p, _tx, _ty, width(), height() + borderTopExtra() + borderBottomExtra());
 #endif
 }
 
@@ -2141,31 +2147,25 @@ public:
     int count;
 };
 
-static void addBorderStyle(QPtrList<CollapsedBorderValue>& borderStyles, CollapsedBorderValue borderValue)
+static void addBorderStyle(QValueList<CollapsedBorderValue>& borderStyles, CollapsedBorderValue borderValue)
 {
-    if (!borderValue.exists())
+    if (!borderValue.exists() || borderStyles.contains(borderValue))
         return;
     
-    uint count = borderStyles.count();
-    if (count == 0)
-        borderStyles.append(new CollapsedBorderValue(borderValue));
-    else {
-        for (uint i = 0; i < count; i++) {
-            CollapsedBorderValue* b = borderStyles.at(i);
-            if (*b == borderValue)
-                return;
-            CollapsedBorderValue result = compareBorders(*b, borderValue);
-            if (result == *b) {
-                borderStyles.insert(i, new CollapsedBorderValue(borderValue));
-                return;
-            }
+    QValueListIterator<CollapsedBorderValue> it = borderStyles.begin();
+    QValueListIterator<CollapsedBorderValue> end = borderStyles.end();
+    for (; it != end; ++it) {
+        CollapsedBorderValue result = compareBorders(*it, borderValue);
+        if (result == *it) {
+            borderStyles.insert(it, borderValue);
+            return;
         }
-        
-        borderStyles.append(new CollapsedBorderValue(borderValue));
     }
+
+    borderStyles.append(borderValue);
 }
 
-void RenderTableCell::collectBorders(QPtrList<CollapsedBorderValue>& borderStyles)
+void RenderTableCell::collectBorders(QValueList<CollapsedBorderValue>& borderStyles)
 {
     addBorderStyle(borderStyles, collapsedLeftBorder());
     addBorderStyle(borderStyles, collapsedRightBorder());
Index: khtml/rendering/render_table.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/rendering/render_table.h,v
retrieving revision 1.34
diff -u -p -r1.34 khtml/rendering/render_table.h
--- khtml/rendering/render_table.h	2004/02/11 19:52:38	1.34
+++ khtml/rendering/render_table.h	2004/02/23 23:28:49
@@ -29,7 +29,6 @@
 #define RENDER_TABLE_H
 
 #include <qcolor.h>
-#include <qptrvector.h>
 
 #include "render_box.h"
 #include "render_block.h"
@@ -350,7 +349,7 @@ public:
     CollapsedBorderValue collapsedRightBorder() const;
     CollapsedBorderValue collapsedTopBorder() const;
     CollapsedBorderValue collapsedBottomBorder() const;
-    virtual void collectBorders(QPtrList<CollapsedBorderValue>& borderStyles);
+    virtual void collectBorders(QValueList<CollapsedBorderValue>& borderStyles);
 
     virtual void updateFromElement();
 
-------------- next part --------------




More information about the Khtml-devel mailing list