Drag and drop optimization
Lubos Lunak
kde-optimize@mail.kde.org
Tue, 4 Feb 2003 13:08:00 +0100
--Boundary-00=_g06P+JEqeULzfj8
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On Thursday 30 of January 2003 02:04, Wilco Greven wrote:
> Hello,
>
> The accompanying patch speeds up the part of Qt's dnd code which looks up
> the window under the cursor (*). The critical part of this code is the
> function findClientWindow. One call takes about 100 ms. Most of this time
> of this is spend in XGetWindowAttributes.
>
> The replacement avoids uses the NETWM client list stacking looks at the
> windows starting from the top of the client stack. Here are some
> testresults comparing the two methods:
>
> KCounterProf:findRealWindow took approx 117.79 ms
> findRealWindow was called 584 times
> Window returned: 22001b3
> KCounterProf:findRealWindowNETWM took approx 3.32 ms
> Window returned: 22001b3
>
> KCounterProf:findRealWindow took approx 113.16 ms
> findRealWindow was called 582 times
> Window returned: 1a00002
> KCounterProf:findRealWindowNETWM took approx 2.17 ms
> Window returned: 1a00002
>
> KCounterProf:findRealWindow took approx 114.25 ms
> findRealWindow was called 583 times
> Window returned: 2c02673
> KCounterProf:findRealWindowNETWM took approx 2.70 ms
> Window returned: 2c02673
Strange, I get quite different numbers. It's 850MHz CPU, so it's notably
faster, but also the improvements differ. When checking the whole
QDragManager::move() method, findRealWindow() takes about 66% of the whole
method. Your optimized version takes it down to 33%, thus saving half of the
time. Perhaps you have many more windows open than I did, or maybe your
testcase is wrong (at least, you shouldn't use random input, as that gives
unreproducable results). Does your patch make a difference also in reality,
e.g. when dragging in Konqy?
>
> So far the biggest problem I can see is that the code relies on a NETWM
> compliant window manager. For the rest it's hard to say because I don't
> know X11 that well.
In order to make your code really work, you'd have to go back to
XGetWindowAttributes and additionally check the map_state in the struct to be
IsMapped. To check if the window manager supports the stacking property, you
can use qt_net_supports().
However, perhaps I have something better. Please try the attached patch. It
should do two things: First, it makes a "hole" in the dragged pixmap, so
XTranslateCoordinates() always finds the window under it, thus making
findRealWindow() unnecessary. Second, if the app cannot catch up with the
XServer sending the mouse move events, it tries to discard multiple move
events. The second change doesn't actually help on my computer, because I
have to try really hard to get CPU usage up to 100%, but on slower computers
it should help (the CPU usage will be 100% too, but it won't demand more).
In general, I don't think there can be done much more. This is a tough
problem, actually. One more thing to do could be to have larger 'sameanswer'
area (see the XDND spec), but I'm afraid that wouldn't be that trivial in Qt.
I tried also some other things, like updating the icon position only every X
ms, but that didn't look nice. And, actually, if the discarding of
superfluous move events will help to make it visually acceptable without the
icon lagging behind the cursor, I think there no need for further
optimizations.
--
Lubos Lunak
KDE developer
---------------------------------------------------------------------
SuSE CR, s.r.o. e-mail: l.lunak@suse.cz , l.lunak@kde.org
Drahobejlova 27 tel: +420 2 9654 2373
190 00 Praha 9 fax: +420 2 9654 2374
Czech Republic http://www.suse.cz/
--Boundary-00=_g06P+JEqeULzfj8
Content-Type: text/x-diff;
charset="iso-8859-1";
name="qdnd_x11.cpp.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="qdnd_x11.cpp.patch"
--- qdnd_x11.cpp.sav 2003-01-02 12:22:59.000000000 +0100
+++ qdnd_x11.cpp 2003-02-04 13:01:13.000000000 +0100
@@ -54,8 +54,8 @@
// conflict resolution
-// unused, may be used again later: const int XKeyPress = KeyPress;
-// unused, may be used again later: const int XKeyRelease = KeyRelease;
+const int XKeyPress = KeyPress;
+const int XKeyRelease = KeyRelease;
#undef KeyPress
#undef KeyRelease
@@ -245,23 +245,40 @@ static const char* const default_pm[] =
"X X X X X X X"
};
+#include <qbitmap.h>
+#include <qpainter.h>
+
class QShapedPixmapWidget : public QWidget {
QPixmap pixmap;
public:
QShapedPixmapWidget(int screen = -1) :
QWidget(QApplication::desktop()->screen( screen ),
- 0, WStyle_Customize | WStyle_Tool | WStyle_NoBorder | WX11BypassWM )
+ 0, WStyle_Customize | WStyle_Tool | WStyle_NoBorder | WX11BypassWM ), oldpmser( 0 ), oldbmser( 0 )
{
}
- void setPixmap(QPixmap pm)
+ void setPixmap(QPixmap pm, QPoint hot)
{
- pixmap = pm;
- if ( pixmap.mask() ) {
- setMask( *pixmap.mask() );
- } else {
- clearMask();
+ int bmser = pm.mask() ? pm.mask()->serialNumber() : 0;
+ if( oldpmser == pm.serialNumber() && oldbmser == bmser
+ && oldhot == hot )
+ return;
+ oldpmser = pm.serialNumber();
+ oldbmser = bmser;
+ oldhot = hot;
+ bool hotspot_in = !(hot.x() < 0 || hot.y() < 0 || hot.x() >= pm.width() || hot.y() >= pm.height());
+ QBitmap mask = pixmap.mask() ? *pixmap.mask() : QBitmap( pm.width(), pm.height());
+ if( hotspot_in ) {
+ if( !pixmap.mask())
+ mask.fill( Qt::color1 );
+ QPainter p( &mask );
+ p.setPen( Qt::color0 );
+ p.drawPoint( hot.x(), hot.y());
+ p.end();
+ pm.setMask( mask );
}
+ setMask( mask );
+ pixmap = pm;
resize(pm.width(),pm.height());
}
@@ -269,6 +286,9 @@ public:
{
bitBlt(this,0,0,&pixmap);
}
+ int oldpmser;
+ int oldbmser;
+ QPoint oldhot;
};
QShapedPixmapWidget * qt_xdnd_deco = 0;
@@ -865,6 +885,38 @@ void QDragManager::timerEvent( QTimerEve
move( QCursor::pos() );
}
+static bool was_move = false;
+static bool found = false;
+static
+Bool predicate( Display*, XEvent* ev, XPointer )
+{
+ if( found )
+ return False;
+ if( ev->type == MotionNotify )
+ {
+ was_move = true;
+ found = true;
+ }
+ if( ev->type == ButtonPress || ev->type == ButtonRelease
+ || ev->type == XKeyPress || ev->type == XKeyRelease
+ || ev->type == ClientMessage )
+ {
+ was_move = false;
+ found = true;
+ }
+ return False;
+}
+
+static
+bool another_movement()
+{
+ was_move = false;
+ found = false;
+ XEvent dummy;
+ XCheckIfEvent( qt_xdisplay(), &dummy, predicate, NULL );
+ return was_move;
+}
+
bool QDragManager::eventFilter( QObject * o, QEvent * e)
{
if ( beingCancelled ) {
@@ -887,8 +939,14 @@ bool QDragManager::eventFilter( QObject
if ( e->type() == QEvent::MouseMove ) {
QMouseEvent* me = (QMouseEvent *)e;
- updateMode(me->stateAfter());
- move( me->globalPos() );
+ if( !another_movement()) {
+ updateMode(me->stateAfter());
+ move( me->globalPos() );
+ }
+ else {
+ static int i = 0;
+ qDebug( "AVOIDING %d", ++i );
+ }
return TRUE;
} else if ( e->type() == QEvent::MouseButtonRelease ) {
qApp->removeEventFilter( this );
@@ -1136,6 +1194,7 @@ void QDragManager::move( const QPoint &
Window targetW = qt_x11_findClientWindow( target, qt_wm_state, TRUE );
if (targetW)
target = targetW;
+ qDebug( "TAR: 0x%lx 0x%lx 0x%lx", target, targetW, qt_xdnd_deco->winId());
if ( qt_xdnd_deco && (!target || target == qt_xdnd_deco->winId()) ) {
target = findRealWindow(globalPos,rootwin,6);
}
@@ -1700,7 +1759,7 @@ void QDragManager::updatePixmap()
defaultPm = new QPixmap(default_pm);
pm = *defaultPm;
}
- qt_xdnd_deco->setPixmap(pm);
+ qt_xdnd_deco->setPixmap(pm, pm_hot);
qt_xdnd_deco->move(QCursor::pos()-pm_hot);
qt_xdnd_deco->repaint(FALSE);
//if ( willDrop ) {
--Boundary-00=_g06P+JEqeULzfj8--