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

List:       kde-core-devel
Subject:    [patch] Preload popup menus for the desktop (3.5)
From:       Scott Wheeler <wheeler () kde ! org>
Date:       2006-03-30 1:35:26
Message-ID: 200603300335.26933.wheeler () kde ! org
[Download RAW message or body]

Here's one that will no doubt be at least semi-controversial, but for me I 
believe solves a significant issue for first impressions in KDE.

The right menu button on the desktop is slow.  Really slow.  In my profiling 
here (on a fast machine) it took between 250 - 750 ms to pop up.  Waiting 3/4 
of a second for a RMB menu on a text file just seems silly.

The same issue applies in Konqueror and if this is accepted I'll probably 
address it there too.

The basics of what this does are:

- Use a QCache to store up to 50 menus in memory
- Create the menu as soon as the user is over the item with the mouse
- When the RMB is pressed, check the cache first for the menu

Of course in some cases, if the user very quickly uses the RMB button on an 
item that they've never hovered over before there can still be a delay, but 
in practice there's usually a short delay before the right mouse button is 
pressed and in the course of a desktop session most of the time menus are 
reused (i.e. trash, home, etc.)

This makes the desktop feel much more responsive in general, and I'd like 
permission to commit it to 3.5.

-Scott

-- 
If the answer is "more lawyers" then the question shouldn't have been asked.

["kdiconview.patch" (text/x-diff)]

Index: kdiconview.h
===================================================================
--- kdiconview.h	(revision 520575)
+++ kdiconview.h	(working copy)
@@ -25,12 +25,15 @@
 #include <kfileitem.h>
 #include <kdirnotify.h>
 
+#include <qcache.h>
+
 class KDirLister;
 class KonqSettings;
 class KSimpleConfig;
 class KAccel;
 class KShadowEngine;
 class KDesktopShadowSettings;
+class KonqPopupMenu;
 
 /**
  * This class is KDesktop's icon view.
@@ -177,11 +180,17 @@
 
 private:
     void refreshTrashIcon();
+    KonqPopupMenu *createPopupMenu( const KFileItemList &items );
 
     static QRect desktopRect();
     static void saveIconPosition(KSimpleConfig *config, int x, int y);
     static void readIconPosition(KSimpleConfig *config, int &x, int &y);
 
+private slots:
+    void slotOverItem( QIconViewItem *item );
+    
+private:
+
     /** Our action collection, parent of all our actions */
     KActionCollection m_actionCollection;
 
@@ -249,6 +258,8 @@
     // did we already get the correct desktopIconsArea (from kicker)
     // needed when we want to line up icons on a grid
     bool m_gotIconsArea;
+
+    QCache<KonqPopupMenu> m_popupMenus;
 };
 
 #endif
Index: kdiconview.cc
===================================================================
--- kdiconview.cc	(revision 520575)
+++ kdiconview.cc	(working copy)
@@ -59,6 +59,8 @@
 #include "kdesktopshadowsettings.h"
 #include "kfileividesktop.h"
 
+#define MAX_POPUP_MENUS 50
+
 // for multihead
 extern int kdesktop_screen_number;
 
@@ -127,6 +129,16 @@
   }
 }
 
+void KDIconView::slotOverItem( QIconViewItem *item )
+{
+    KFileItem *fileItem = static_cast<KFileIVI*>( item )->item();
+
+    KFileItemList list;
+    list.append( fileItem );
+
+    createPopupMenu( list );
+}
+
 // -----------------------------------------------------------------------------
 
 KDIconView::KDIconView( QWidget *parent, const char* name )
@@ -147,7 +159,8 @@
       m_eSortCriterion( NameCaseInsensitive ),
       m_bSortDirectoriesFirst( true ),
       m_itemsAlwaysFirst(),
-      m_gotIconsArea(false)
+      m_gotIconsArea(false),
+      m_popupMenus(MAX_POPUP_MENUS, 101)
 {
     setResizeMode( Fixed );
     setIconArea( desktopRect() );  // the default is the whole desktop
@@ -177,12 +190,17 @@
     connect( this, SIGNAL( enableAction( const char * , bool ) ),
              SLOT( slotEnableAction( const char * , bool ) ) );
 
+    connect( this, SIGNAL( onItem( QIconViewItem * ) ),
+             SLOT( slotOverItem( QIconViewItem * ) ) );
+
     // Hack: KonqIconViewWidget::slotItemRenamed is not virtual :-(
     disconnect( this, SIGNAL(itemRenamed(QIconViewItem *, const QString &)),
              this, SLOT(slotItemRenamed(QIconViewItem *, const QString &)) );
     connect( this, SIGNAL(itemRenamed(QIconViewItem *, const QString &)),
              this, SLOT(slotItemRenamed(QIconViewItem *, const QString &)) );
 
+    m_popupMenus.setAutoDelete( true );
+
     if (!m_bEditableDesktopIcons)
     {
        setItemsMovable(false);
@@ -799,16 +817,10 @@
     KParts::BrowserExtension::PopupFlags itemFlags = KParts::BrowserExtension::DefaultPopupItems;
     if ( hasMediaFiles )
         itemFlags |= KParts::BrowserExtension::NoDeletion;
-    KonqPopupMenu * popupMenu = new KonqPopupMenu( KonqBookmarkManager::self(), _items,
-                                                   url(),
-                                                   m_actionCollection,
-                                                   KRootWm::self()->newMenu(),
-                                                   this,
-                                                   KonqPopupMenu::ShowProperties | KonqPopupMenu::ShowNewWindow,
-                                                   itemFlags );
 
-    popupMenu->exec( _global );
-    delete popupMenu;
+    KonqPopupMenu *menu = createPopupMenu( _items );
+    menu->exec( _global );
+
     m_popupURL = KURL();
     if (pasteTo)
         pasteTo->setEnabled( false );
@@ -1130,6 +1142,31 @@
     }
 }
 
+KonqPopupMenu *KDIconView::createPopupMenu( const KFileItemList &items )
+{
+    QString itemUrl = items.getFirst()->url().url();
+
+    if ( items.count() == 1 && m_popupMenus[ itemUrl ])
+        return m_popupMenus[ itemUrl ];
+
+    KonqPopupMenu *menu = new KonqPopupMenu(
+        KonqBookmarkManager::self(), items, url(), m_actionCollection,
+        KRootWm::self()->newMenu(), this,
+        KonqPopupMenu::ShowProperties | KonqPopupMenu::ShowNewWindow,
+        KParts::BrowserExtension::DefaultPopupItems );
+
+    // If it's a multi-selection menu, just throw it into a special cache slot
+    // called "multi".  It's not actually used as part of the cached memory,
+    // but this ensures that it's deleted at some point.
+
+    if ( items.count() > 1 )
+        m_popupMenus.insert( "multi", menu );
+    else
+        m_popupMenus.insert( itemUrl, menu );
+
+    return menu;
+}
+
 // -----------------------------------------------------------------------------
 
 void KDIconView::slotDeleteItem( KFileItem * _fileitem )


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