[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    branches/KDE/3.5/kdenetwork/kopete
From:       Martijn Klingens <klingens () kde ! org>
Date:       2007-02-24 0:14:45
Message-ID: 1172276085.958825.32240.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 636734 by mklingens:

Major optimization of the code used by formatStyleKeywords().

In practice this makes the following cases a LOT faster:
* Opening a chat with large amounts of 'old' messages from history
* Browsing history in the chat window with alt-left/right
* Updating the chat window someone changes avatar
* Updating the chat window when switching style

Performance improvement initially is about 30% due to optimized
Kopete::Message::plainBody() code for determining whether the message is
left-to-right or not.

Each subsequent call on the same messages will be even about 60% faster
because the LTR value is cached.

In practice the code is unfortunately still quite slow, but the massive
improvement is already quite noticable.

Reviewed and approved by Will, also tested against the unit tests.



 M  +1 -3      kopete/chatwindow/chatmessagepart.cpp  
 M  +88 -55    libkopete/kopetemessage.cpp  
 M  +17 -1     libkopete/kopetemessage.h  


--- branches/KDE/3.5/kdenetwork/kopete/kopete/chatwindow/chatmessagepart.cpp \
#636733:636734 @@ -1060,9 +1060,7 @@
 	}
 
 	// Set message direction("rtl"(Right-To-Left) or "ltr"(Left-to-right))
-	// FIXME: The conversion to plainBody() is extremely expensive and should not be \
                used
-	//        here. Use a cached value for isRightToLeft instead. -- Martijn, 20070218
-	resultHTML = resultHTML.replace( QString::fromUtf8("%messageDirection%"), \
message.plainBody().isRightToLeft() ? "rtl" : "ltr" ); +	resultHTML = \
resultHTML.replace( QString::fromUtf8("%messageDirection%"), message.isRightToLeft() \
? "rtl" : "ltr" );  
 	// These colors are used for coloring nicknames. I tried to use
 	// colors both visible on light and dark background.
--- branches/KDE/3.5/kdenetwork/kopete/libkopete/kopetemessage.cpp #636733:636734
@@ -47,7 +47,7 @@
 {
 public:
 	Private( const QDateTime &timeStamp, const Contact *from, const ContactPtrList &to,
-	         const QString &body, const QString &subject, MessageDirection direction, \
MessageFormat f, +	         const QString &subject, MessageDirection direction,
 	         const QString &requestedPlugin, MessageType type );
 
 	QGuardedPtr<const Contact> from;
@@ -62,6 +62,7 @@
 	bool bgOverride;
 	bool fgOverride;
 	bool rtfOverride;
+	bool isRightToLeft;
 	QDateTime timeStamp;
 	QFont font;
 
@@ -72,46 +73,26 @@
 };
 
 Message::Private::Private( const QDateTime &timeStamp, const Contact *from,
-             const ContactPtrList &to, const QString &body, const QString &subject,
-				 MessageDirection direction, MessageFormat f, const QString &requestedPlugin, \
                MessageType type )
-	: from(from), to(to), manager(0), direction(direction), format(f), type(type)
-	, requestedPlugin(requestedPlugin), importance( (to.count() <= 1) ? Normal : Low ), \
                bgOverride(false), fgOverride(false)
-	, rtfOverride(false), timeStamp(timeStamp), body(body), subject(subject)
+		const ContactPtrList &to, const QString &subject,
+		MessageDirection direction, const QString &requestedPlugin, MessageType type )
+: from( from ), to( to ), manager( 0 ), direction( direction ), format( PlainText ), \
type( type ), +	requestedPlugin( requestedPlugin ), importance( (to.count() <= 1) ? \
Normal : Low ), +	bgOverride( false ), fgOverride( false ), rtfOverride( false ), \
isRightToLeft( false ), +	timeStamp( timeStamp ), body( QString::null ), subject( \
subject )  {
-	
-	//TODO: move that in ChatTextEditPart::contents
-	if( format == RichText )
-	{
-		//This is coming from the RichTextEditor component.
-		//Strip off the containing HTML document
-		this->body.replace( QRegExp( \
QString::fromLatin1(".*<body.*>\\s+(.*)\\s+</body>.*") ), QString::fromLatin1("\\1") \
                );
-
-		//Strip <p> tags
-		this->body.replace( QString::fromLatin1("<p>"), QString::null );
-
-		//Replace </p> with a <br/>
-		this->body.replace( QString::fromLatin1("</p>") , QString::fromLatin1("<br/>") );
-
-		//Remove trailing <br/>
-		if ( this->body.endsWith( QString::fromLatin1("<br/>") ) )
-			this->body.truncate( this->body.length() - 5 );
-		this->body.remove(  QString::fromLatin1("\n") );
-		this->body.replace( QRegExp( QString::fromLatin1( "\\s\\s" ) ), \
                QString::fromLatin1( "&nbsp; " ) );
-
-	}
 }
 
-
 Message::Message()
-    : d( new Private( QDateTime::currentDateTime(), 0L, QPtrList<Contact>(), \
                QString::null, QString::null, Internal,
-	PlainText, QString::null, TypeNormal ) )
+: d( new Private( QDateTime::currentDateTime(), 0L, QPtrList<Contact>(), \
QString::null, Internal, +	QString::null, TypeNormal ) )
 {
 }
 
 Message::Message( const Contact *fromKC, const QPtrList<Contact> &toKC, const \
                QString &body,
 		  MessageDirection direction, MessageFormat f, const QString &requestedPlugin, \
                MessageType type )
-    : d( new Private( QDateTime::currentDateTime(), fromKC, toKC, body, \
QString::null, direction, f, requestedPlugin, type ) ) +: d( new Private( \
QDateTime::currentDateTime(), fromKC, toKC, QString::null, direction, \
requestedPlugin, type ) )  {
+	doSetBody( body, f );
 }
 
 Message::Message( const Contact *fromKC, const Contact *toKC, const QString &body,
@@ -119,26 +100,30 @@
 {
 	QPtrList<Contact> to;
 	to.append(toKC);
-	d = new Private( QDateTime::currentDateTime(), fromKC, to, body, QString::null, \
direction, f, requestedPlugin, type ); +	d = new Private( \
QDateTime::currentDateTime(), fromKC, to, QString::null, direction, requestedPlugin, \
type ); +	doSetBody( body, f );
 }
 
 Message::Message( const Contact *fromKC, const QPtrList<Contact> &toKC, const \
                QString &body,
 		  const QString &subject, MessageDirection direction, MessageFormat f, const \
                QString &requestedPlugin, MessageType type )
-    : d( new Private( QDateTime::currentDateTime(), fromKC, toKC, body, subject, \
direction, f, requestedPlugin, type ) ) +    : d( new Private( \
QDateTime::currentDateTime(), fromKC, toKC, subject, direction, requestedPlugin, type \
) )  {
+	doSetBody( body, f );
 }
 
 Message::Message( const QDateTime &timeStamp, const Contact *fromKC, const \
                QPtrList<Contact> &toKC,
 		  const QString &body, MessageDirection direction, MessageFormat f, const QString \
                &requestedPlugin, MessageType type )
-    : d( new Private( timeStamp, fromKC, toKC, body, QString::null, direction, f, \
requestedPlugin, type ) ) +    : d( new Private( timeStamp, fromKC, toKC, \
QString::null, direction, requestedPlugin, type ) )  {
+	doSetBody( body, f );
 }
 
 
 Message::Message( const QDateTime &timeStamp, const Contact *fromKC, const \
                QPtrList<Contact> &toKC,
 		  const QString &body, const QString &subject, MessageDirection direction, \
                MessageFormat f, const QString &requestedPlugin, MessageType type )
-    : d( new Private( timeStamp, fromKC, toKC, body, subject, direction, f, \
requestedPlugin, type ) ) +    : d( new Private( timeStamp, fromKC, toKC, subject, \
direction, requestedPlugin, type ) )  {
+	doSetBody( body, f );
 }
 
 Kopete::Message::Message( const Message &other )
@@ -205,40 +190,57 @@
 	d->font = font;
 }
 
-void Message::setBody( const QString &body, MessageFormat f )
+void Message::doSetBody( const QString &_body, Message::MessageFormat f )
 {
-	detach();
+	QString body = _body;
 
-	QString theBody = body;
 	//TODO: move that in ChatTextEditPart::contents
 	if( f == RichText )
 	{
 		//This is coming from the RichTextEditor component.
 		//Strip off the containing HTML document
-		theBody.replace( QRegExp( QString::fromLatin1(".*<body.*>\\s+(.*)\\s+</body>.*") \
), QString::fromLatin1("\\1") ); +		body.replace( QRegExp( \
QString::fromLatin1(".*<body[^>]*>(.*)</body>.*") ), QString::fromLatin1("\\1") );  
 		//Strip <p> tags
-		theBody.replace( QString::fromLatin1("<p>"), QString::null );
+		body.replace( QString::fromLatin1("<p>"), QString::null );
 
 		//Replace </p> with a <br/>
-		theBody.replace( QString::fromLatin1("</p>"), QString::fromLatin1("<br/>") );
+		body.replace( QString::fromLatin1("</p>"), QString::fromLatin1("<br/>") );
 
 		//Remove trailing </br>
-		if ( theBody.endsWith( QString::fromLatin1("<br/>") ) )
-			theBody.truncate( theBody.length() - 5 );
+		if ( body.endsWith( QString::fromLatin1("<br/>") ) )
+			body.truncate( body.length() - 5 );
 	
-		theBody.remove( QString::fromLatin1("\n") );
-		theBody.replace( QRegExp( QString::fromLatin1( "\\s\\s" ) ), QString::fromLatin1( \
"&nbsp; " ) ); +		body.remove( QString::fromLatin1("\n") );
+		body.replace( QRegExp( QString::fromLatin1( "\\s\\s" ) ), QString::fromLatin1( " \
&nbsp;" ) );  }
-	/*	else if( f == ParsedHTML )
+	/*
+	else if( f == ParsedHTML )
 	{
-	kdWarning( 14000 ) << k_funcinfo << "using ParsedHTML which is internal !   \
                message: " << body << kdBacktrace() << endl;
-	}*/
+		kdWarning( 14000 ) << k_funcinfo << "using ParsedHTML which is internal! Message: \
'" << +			body << "', Backtrace: " << kdBacktrace() << endl;
+	}
+	*/
 
-	d->body=theBody;
+	d->body = body;
 	d->format = f;
+
+	// unescaping is very expensive, do it only once and cache the result
+	d->isRightToLeft = ( f & RichText ? unescape( d->body ).isRightToLeft() : \
d->body.isRightToLeft() );  }
 
+void Message::setBody( const QString &body, MessageFormat f )
+{
+	detach();
+
+	doSetBody( body, f );
+}
+
+bool Message::isRightToLeft() const
+{
+	return d->isRightToLeft;
+}
+
 void Message::setImportance(Message::MessageImportance i)
 {
 	detach();
@@ -249,15 +251,46 @@
 {
 	QString data = xml;
 
-	//remove linebreak and multiple spaces
+	// Remove linebreak and multiple spaces. First return nbsp's to normal spaces :)
 	data.replace( QRegExp( QString::fromLatin1( "\\s*[\n\r\t]+\\s*" ) , false ), \
QString::fromLatin1(" " )) ;  
-	data.replace( QRegExp( QString::fromLatin1( "< *img[^>]*title=\"([^>\"]*)\"[^>]*>" \
) , false ), QString::fromLatin1( "\\1" ) );  //escape smeleys, return to the \
                original code
-	data.replace( QRegExp( QString::fromLatin1( "< */ *p[^>]*>" ) , false ), \
                QString::fromLatin1( "\n" ) );
-	data.replace( QRegExp( QString::fromLatin1( "< */ *div[^>]*>" ) , false ), \
                QString::fromLatin1( "\n" ) );
-	data.replace( QRegExp( QString::fromLatin1( "< *br */? *>" ) , false ), \
                QString::fromLatin1( "\n" ) );
-	data.replace( QRegExp( QString::fromLatin1( "<[^>]*>" ) ), QString::null );
+	// Loop over data, replacing all XML elements
+	int pos = 0;
+	QRegExp elementRX( QString::fromLatin1(
+		"< *"                                   // Elem start, whitespace
+		"/? *([^ >]*)"                          // Elem name with optional leading /, but \
no trailing / +		"(?: (?:[^>]* )?title=\"([^>\"]*)\")?"  // Optionally, a title \
attrib, used for <img> +		"[^>]*>" ), false );                    // Everything else \
up to the closing '>'  
+	while ( ( pos = elementRX.search( data, pos ) ) != -1 )
+	{
+		QString elem = elementRX.cap( 1 ).lower().stripWhiteSpace();
+		if ( elem == QString::fromLatin1( "img" ) )
+		{
+			// Replace smileys with their original text
+			// If this is an image that is not a smiley it will make the
+			// code effectively behave like the wildcard filter in the
+			// else {} below, which is intended
+			QString orig = elementRX.cap( 2 );
+			data.replace( pos, elementRX.matchedLength(), orig );
+			pos += orig.length();
+		}
+		else if ( elem == QString::fromLatin1( "/p" ) || elem == QString::fromLatin1( \
"/div" ) || +			elem == QString::fromLatin1( "br" ) )
+		{
+			// Replace paragraph, div and line breaks with a newline
+			data.replace( pos, elementRX.matchedLength(), '\n' );
+			pos++;
+		}
+		else
+		{
+			// Remove all other elements entirely
+			// Don't update pos, we restart at this position :)
+			data.remove( pos, elementRX.matchedLength() );
+		}
+	}
+
+	// Replace stuff starting with '&'
 	data.replace( QString::fromLatin1( "&gt;" ), QString::fromLatin1( ">" ) );
 	data.replace( QString::fromLatin1( "&lt;" ), QString::fromLatin1( "<" ) );
 	data.replace( QString::fromLatin1( "&quot;" ), QString::fromLatin1( "\"" ) );
--- branches/KDE/3.5/kdenetwork/kopete/libkopete/kopetemessage.h #636733:636734
@@ -361,13 +361,23 @@
 public:  /* static helpers */
 
 	/**
-	* Unescapes a string, removing XML entity references and return a plein text.
+	* Unescapes a string, removing XML entity references and returns a plain text.
 	*
+	* Note that this method is *VERY* expensive when called on rich text bodies,
+	* use with care!
+	*
 	* @param xml The string you want to unescape
 	*/
 	static QString unescape( const QString &xml );
 
 	/**
+	 * Indicate whether the string is right-to-left (Arabic or Hebrew are bidi locales)
+	 * or "normal" left-to-right. Calculating RTL on rich text is expensive, and
+	 * isRightToLeft() therefore uses a cached value.
+	 */
+	bool isRightToLeft() const;
+
+	/**
 	 * @brief Transform a pleintext message to an html.
 	 * it escape main entity like &gt; &lt; add some &lt;br /&gt; or &amp;nbsp;
 	 */
@@ -394,6 +404,12 @@
 	 */
 	void detach();
 
+	/**
+	 * Called internally by @ref setBody() and the constructor
+	 * Basically @ref setBody() without detach
+	 */
+	void doSetBody( const QString &body, MessageFormat format = PlainText );
+
 	class Private;
 	KSharedPtr<Private> d;
 


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic