Bug 118658 - Incremental-repainting not incremental enough for marquees.
Summary: Incremental-repainting not incremental enough for marquees.
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml renderer (show other bugs)
Version: unspecified
Platform: Fedora RPMs Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
: 62313 107457 121543 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-12-19 19:54 UTC by kim Lux
Modified: 2006-09-26 04:05 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kim Lux 2005-12-19 19:54:28 UTC
Version:            (using KDE KDE 3.5.0)
Installed from:    Fedora RPMs
OS:                Linux

rpm -q kdebase
kdebase-3.5.0-1.2.fc4.kde

konqueror is Release 3.5.0-2.2.fc4.kde

This website make X go to 90% in Konqueror.  Firefox handles it fine. 

http://www.msnbc.msn.com/id/10532906/
Comment 1 Thiago Macieira 2005-12-20 01:36:02 UTC
It must be that news ticker they have on that page.

Both Konqueror and Firefox shoot the CPU to 100% here. The difference is that Konqueror is sharing the load with X, whereas Firefox claims 95% all for itself.

Tested Konqueror 3.5 r487700 and Firefox 1.0.4.
Comment 2 Maksim Orlovich 2005-12-20 01:51:40 UTC
It's a renderer/incremental repaint issue. hmm, I had a font patch that may make it slightly more efficient wrt to app's own load
Comment 3 Maksim Orlovich 2006-01-19 19:22:53 UTC
Let's make this a bit general. This is hardly the only website showing this.
Comment 4 Maksim Orlovich 2006-01-19 19:29:32 UTC
*** Bug 62313 has been marked as a duplicate of this bug. ***
Comment 5 Germain Garand 2006-02-28 05:16:33 UTC
SVN commit 514357 by ggarand:

Common-case style selection optimisation inspired from WebCore.
Avoids repetitive scanning of the class attribute for a list of classes.

CCBUG: 118658
(not the only issue there)



 M  +10 -0     ChangeLog  
 M  +3 -2      css/css_base.cpp  
 M  +1 -0      css/css_base.h  
 M  +25 -15    css/cssstyleselector.cpp  
 M  +1 -1      css/parser.cpp  
 M  +1 -1      css/parser.y  
 M  +10 -0     html/html_elementimpl.cpp  
 M  +1 -0      xml/dom_elementimpl.cpp  
 M  +8 -2      xml/dom_elementimpl.h  


--- branches/KDE/3.5/kdelibs/khtml/ChangeLog #514356:514357
@@ -1,5 +1,15 @@
 2006-02-28  Germain Garand  <germain@ebooksfrance.org>
+         
+        Common-case style selection optimisation inspired from WebCore.
 
+        * css/css_base.cpp/.h: add 'Class' to possible matching modes
+        * css/cssstyleselector.cpp: treat 'Class' extra, not anymore as an ordinary List.
+        * css/parser.cpp/y: use 'Class' matching mode 
+        * html/html_elementimpl.cpp: when parsing ATTR_CLASS, set a boolean to remember if it is or not a class list.
+        * xml/dom_elementimpl.{cpp,h}: add m_hasClassList boolean + setter/getter
+
+2006-02-28  Germain Garand  <germain@ebooksfrance.org>
+
         * rendering/render_object.cpp (dirtyFormattingContext): be more efficient with inlineflow objects.
         * rendering/render_block.cpp (layoutBlockChildren): be more efficient with regard to positioned objects relayout. 
         Merge WC margin-collapse regression fix (WC/#3508). 
--- branches/KDE/3.5/kdelibs/khtml/css/css_base.cpp #514356:514357
@@ -144,6 +144,7 @@
     case Exact:
     case Set:
     case List:
+    case Class:
     case Hyphen:
     case PseudoClass:
     case PseudoElement:
@@ -317,7 +318,7 @@
         str = "#";
         str += cs->value;
     }
-    else if ( tag == anyLocalName && cs->attr == ATTR_CLASS && cs->match == CSSSelector::List )
+    else if ( tag == anyLocalName && cs->match == CSSSelector::Class )
     {
         str = ".";
         str += cs->value;
@@ -343,7 +344,7 @@
             str += "#";
             str += cs->value;
         }
-        else if ( cs->attr == ATTR_CLASS && cs->match == CSSSelector::List )
+        else if ( cs->match == CSSSelector::Class )
         {
             str += ".";
             str += cs->value;
--- branches/KDE/3.5/kdelibs/khtml/css/css_base.h #514356:514357
@@ -107,6 +107,7 @@
 	    Id,
 	    Exact,
 	    Set,
+	    Class,
 	    List,
 	    Hyphen,
 	    PseudoClass,
--- branches/KDE/3.5/kdelibs/khtml/css/cssstyleselector.cpp #514356:514357
@@ -1068,25 +1068,35 @@
             break;
         case CSSSelector::Set:
             break;
+        case CSSSelector::Class:
+            if (!e->hasClassList()) {
+                if( (strictParsing && strcmp(sel->value, value) ) ||
+                    (!strictParsing && strcasecmp(sel->value, value)))
+                    return false;
+               return true;
+            }
+            // no break    
         case CSSSelector::List:
         {
-            const QChar* s = value.unicode();
-            int l = value.length();
-            while( l && !s->isSpace() )
-              l--,s++;
-	    if (!l) {
-		// There is no list, just a single item.  We can avoid
-		// allocing QStrings and just treat this as an exact
-		// match check.
-		if( (strictParsing && strcmp(sel->value, value) ) ||
-		    (!strictParsing && strcasecmp(sel->value, value)))
-		    return false;
-		break;
-	    }
+            if (sel->match != CSSSelector::Class) {
+                const QChar* s = value.unicode();
+                int l = value.length();
+                while( l && !s->isSpace() )
+                    l--,s++;
+                if (!l) {
+		    // There is no list, just a single item.  We can avoid
+		    // allocing QStrings and just treat this as an exact
+		    // match check.
+		    if( (strictParsing && strcmp(sel->value, value) ) ||
+		        (!strictParsing && strcasecmp(sel->value, value)))
+		        return false;
+		    break;
+	        }
+            }
 
             // The selector's value can't contain a space, or it's totally bogus.
-            l = sel->value.find(' ');
-            if (l != -1)
+            // ### check if this can still happen
+            if (sel->value.find(' ') != -1)
                 return false;
 
             QString str = value.string();
--- branches/KDE/3.5/kdelibs/khtml/css/parser.cpp #514356:514357
@@ -2072,7 +2072,7 @@
 
     {
 	yyval.selector = new CSSSelector();
-	yyval.selector->match = CSSSelector::List;
+	yyval.selector->match = CSSSelector::Class;
 	yyval.selector->attr = ATTR_CLASS;
 	yyval.selector->value = domString(yyvsp[0].string);
     ;}
--- branches/KDE/3.5/kdelibs/khtml/css/parser.y #514356:514357
@@ -691,7 +691,7 @@
 class:
     '.' IDENT {
 	$$ = new CSSSelector();
-	$$->match = CSSSelector::List;
+	$$->match = CSSSelector::Class;
 	$$->attr = ATTR_CLASS;
 	$$->value = domString($2);
     }
--- branches/KDE/3.5/kdelibs/khtml/html/html_elementimpl.cpp #514356:514357
@@ -175,6 +175,16 @@
         getDocument()->incDOMTreeVersion();
         break;
     case ATTR_CLASS:
+        if (attr->val()) {
+          DOMString v = attr->value();
+          const QChar* s = v.unicode();
+          int l = v.length();
+          while( l && !s->isSpace() )
+            l--,s++;
+          setHasClassList(l);
+        } else
+          setHasClassList(false);                 
+    // no break                                         
     case ATTR_NAME:
         setChanged(); // in case of a CSS selector on class/name
         getDocument()->incDOMTreeVersion();
--- branches/KDE/3.5/kdelibs/khtml/xml/dom_elementimpl.cpp #514356:514357
@@ -308,6 +308,7 @@
     m_restyleLate = false;
     m_restyleSelfLate = false;
     m_restyleChildrenLate = false;
+    m_hasClassList = false;
 }
 
 ElementImpl::~ElementImpl()
--- branches/KDE/3.5/kdelibs/khtml/xml/dom_elementimpl.h #514356:514357
@@ -141,7 +141,7 @@
 public:
     ElementImpl(DocumentPtr *doc);
     ~ElementImpl();
-
+    
     DOMString getAttribute( NodeImpl::Id id, bool nsAware = 0, const DOMString& qName = DOMString() ) const;
     void setAttribute( NodeImpl::Id id, const DOMString &value, const DOMString &qName,
                        int &exceptioncode );
@@ -224,16 +224,21 @@
      */
     DOMString openTagStartToString(bool expandurls = false) const;
     
-    bool restyleLate() { return m_restyleLate; };
+    bool restyleLate() const { return m_restyleLate; };
     void setRestyleLate(bool b=true) { m_restyleLate = b; };
     void setRestyleSelfLate() { m_restyleSelfLate = true; };
     void setRestyleChildrenLate() { m_restyleChildrenLate = true; };
 
+    // for style selection performance: whether the element matches several CSS Classes
+    bool hasClassList() const { return m_hasClassList; }
+    void setHasClassList(bool b) { m_hasClassList = b; }
+
     void updateId(DOMStringImpl* oldId, DOMStringImpl* newId);
     //Called when mapping from id to this node in document should be removed
     virtual void removeId(const QString& id);
     //Called when mapping from id to this node in document should be added
     virtual void addId   (const QString& id);
+
 protected:
     void createAttributeMap() const;
     void createDecl();
@@ -253,6 +258,7 @@
     bool m_restyleLate;
     bool m_restyleSelfLate;
     bool m_restyleChildrenLate;
+    bool m_hasClassList;
 };
 
 
Comment 6 Tommi Tervo 2006-03-01 19:28:44 UTC
*** Bug 107457 has been marked as a duplicate of this bug. ***
Comment 7 Maksim Orlovich 2006-09-05 00:45:57 UTC
*** Bug 121543 has been marked as a duplicate of this bug. ***
Comment 8 Germain Garand 2006-09-18 14:40:37 UTC
Mandriva wiki has some nasty moving rel pos of the same vein
e.g:
http://qa.mandriva.com/twiki/bin/view/Main/MandrivaLinux2007Sunna
Comment 9 Germain Garand 2006-09-26 04:05:48 UTC
SVN commit 588455 by ggarand:

Heavy DHTML optimizations.
Basically avoid to do any layouting work when the style difference only implies 
the translation of a layer and nothing more, which is very common.
Makes KHTML fly on a lot of dynamical pages!

Introduce some priority levels when repainting, so we can have rapid repaints
when needed.

Don't use anymore the overflow properties for storing the layers scroll overflows
as that was terminally boken for any nestig level. 
Admittedly leftmost/rightmost and friends can be a tad more 
expensive at times, but they do provide correct results which is very much valuable. 
Rebutals welcome.

BUG: 118658




 M  +44 -14    render_block.cpp  
 M  +4 -0      render_block.h  
 M  +2 -2      render_body.cpp  
 M  +1 -1      render_body.h  
 M  +11 -15    render_box.cpp  
 M  +3 -2      render_box.h  
 M  +71 -34    render_canvas.cpp  
 M  +15 -3     render_canvas.h  
 M  +44 -5     render_flow.cpp  
 M  +2 -1      render_flow.h  
 M  +11 -6     render_layer.cpp  
 M  +1 -1      render_layer.h  
 M  +70 -13    render_object.cpp  
 M  +12 -3     render_object.h  
 M  +13 -1     render_style.cpp  
 M  +22 -5     render_table.cpp  
 M  +2 -1      render_table.h  
 M  +2 -2      render_text.cpp  
 M  +1 -1      render_text.h