Improvements to playtree

* Don't expand nodes by default
* Distinguish notification types from document to view
  ( update of one item, global change, append of one item)
* Handle item-append 
   - Add a command/callback
   - TODO: Only rebuild the tree image if the newly appended item is 
     visible
* Rename m_lastPos to m_firstPos as it represents the first position
* Beginning of a general ensureVisible function (not working yet)

modules/gui/skins2/commands/cmd_vars.cpp

@@ -48,7 +48,7 @@ void CmdPlaytreeUpdate::execute()
 {
     // Notify  the playtree variable
     Playtree &rVar = VlcProc::instance( getIntf() )->getPlaytreeVar();
-    rVar.onUpdate( m_id );
+    rVar.onUpdateItem( m_id );
 }
 
 bool CmdPlaytreeUpdate::checkRemove( CmdGeneric *pQueuedCommand ) const
@@ -64,6 +64,13 @@ bool CmdPlaytreeUpdate::checkRemove( CmdGeneric *pQueuedCommand ) const
 }
 
 
+void CmdPlaytreeAppend::execute()
+{
+    // Notify  the playtree variable
+    Playtree &rVar = VlcProc::instance( getIntf() )->getPlaytreeVar();
+    rVar.onAppend( m_pAdd );
+}
+
 void CmdSetText::execute()
 {
     // Change the text variable

modules/gui/skins2/commands/cmd_vars.hpp

@@ -59,6 +59,24 @@ class CmdPlaytreeUpdate: public CmdGeneric
         int m_id;
 };
 
+/// Command to notify the playtree of an item append
+class CmdPlaytreeAppend: public CmdGeneric
+{
+    public:
+        CmdPlaytreeAppend( intf_thread_t *pIntf, playlist_add_t *p_add ) :
+            CmdGeneric( pIntf ), m_pAdd( p_add ) {}
+        virtual ~CmdPlaytreeAppend() {}
+
+        /// This method does the real job of the command
+        virtual void execute();
+
+        /// Return the type of the command
+        virtual string getType() const { return "playtree append"; }
+
+    private:
+        playlist_add_t * m_pAdd;
+};
+
 
 /// Command to set a text variable
 class CmdSetText: public CmdGeneric

modules/gui/skins2/controls/ctrl_tree.cpp

@@ -67,7 +67,7 @@ CtrlTree::CtrlTree( intf_thread_t *pIntf,
     m_rTree.addObserver( this );
     m_rTree.getPositionVar().addObserver( this );
 
-    m_lastPos = m_rTree.begin();
+    m_firstPos = m_rTree.begin();
 
     makeImage();
 }
@@ -130,10 +130,26 @@ int CtrlTree::maxItems()
 }
 
 
-void CtrlTree::onUpdate( Subject<VarTree, int> &rTree,  int arg )
+void CtrlTree::onUpdate( Subject<VarTree, tree_update*> &rTree,
+                         tree_update *arg )
 {
-    fprintf( stderr, "Doing update type %i\n", arg );
-    autoScroll();
+    if( arg->i_type == 0 ) // Item update
+    {
+        autoScroll();
+        makeImage();
+    }
+    else if ( arg->i_type == 1 ) // Global change or deletion
+    {
+        makeImage();
+    }
+    else if ( arg->i_type == 2 ) // Item-append
+    {
+        /* TODO: Check if the item should be visible. If it is, makeImage
+         * Else, do nothing
+         */
+        makeImage();
+    }
+    notifyLayout();
     m_pLastSelected = NULL;
 }
 
@@ -153,10 +169,10 @@ void CtrlTree::onUpdate( Subject<VarPercent, void*> &rPercent, void* arg)
 #endif
         it = m_rTree.getVisibleItem(lrint( (1.0 - rVarPos.get()) * (double)excessItems ) + 1);
     }
-    if( m_lastPos != it )
+    if( m_firstPos != it )
     {
         // Redraw the control if the position has changed
-        m_lastPos = it;
+        m_firstPos = it;
         makeImage();
         notifyLayout();
     }
@@ -164,7 +180,7 @@ void CtrlTree::onUpdate( Subject<VarPercent, void*> &rPercent, void* arg)
 
 void CtrlTree::onResize()
 {
-// FIXME : shouldn't be the same as the onUpdate function ... but i'm lazy
+    // FIXME : shouldn't be the same as the onUpdate function ... but i'm lazy
     // Determine what is the first item to display
     VarTree::Iterator it = m_rTree.begin();
 
@@ -180,34 +196,9 @@ void CtrlTree::onResize()
         it = m_rTree.getVisibleItem(lrint( (1.0 - rVarPos.get()) * (double)excessItems ) + 1);
     }
     // Redraw the control if the position has changed
-    m_lastPos = it;
+    m_firstPos = it;
     makeImage();
     notifyLayout();
-#if 0
-    // Determine what is the first item to display
-    VarTree::Iterator it = m_rTree.begin();
-
-    int excessItems = m_rTree.visibleItems() - maxItems();
-
-    if( excessItems > 0)
-    {
-        /* FIXME VarPercent &rVarPos = m_rTree.getPositionVar();
-        double newVal = 1.0 - (double)m_lastPos / excessItems;
-        if( newVal >= 0 )
-        {
-            // Change the position to keep the same first displayed item
-            rVarPos.set( 1.0 - (double)m_lastPos / excessItems );
-        }
-        else
-        {
-            // We cannot keep the current first item
-            m_lastPos = excessItems;
-        }*/
-        it = m_rTree.getVisibleItem( excessItems );
-    }
-    makeImage();
-    notifyLayout();
-#endif
 }
 
 void CtrlTree::onPositionChange()
@@ -241,6 +232,7 @@ void CtrlTree::handleEvent( EvtGeneric &rEvent )
                         m_pLastSelected = &*it;
                     }
                 }
+                //ensureVisible( it );
             }
             else if( key == KEY_DOWN )
             {
@@ -260,6 +252,7 @@ void CtrlTree::handleEvent( EvtGeneric &rEvent )
                 {
                     previousWasSelected = ( &*it == m_pLastSelected );
                 }
+                //ensureVisible( it );
             }
             else if( key == KEY_RIGHT )
             {
@@ -468,47 +461,66 @@ void CtrlTree::draw( OSGraphics &rImage, int xDest, int yDest )
     }
 }
 
-void CtrlTree::autoScroll()
+bool CtrlTree::ensureVisible( VarTree::Iterator item )
 {
-    // Find the current playing stream
-    int playIndex = 0;
+    // Find the item to focus
+    int focusItemIndex = 0;
     VarTree::Iterator it;
     for( it = m_rTree.begin(); it != m_rTree.end();
          it = m_rTree.getNextVisibleItem( it ) )
     {
         if( it->m_playing ) break;
-        playIndex++;
+        focusItemIndex++;
     }
+   return  ensureVisible( focusItemIndex );
+}
 
-    if( it == m_rTree.end() ) return;
-
-    // Find  m_lastPos
-    int lastPosIndex = 0;
+bool CtrlTree::ensureVisible( int focusItemIndex )
+{
+    // Find  m_firstPos
+    VarTree::Iterator it;
+    int firstPosIndex = 0;
     for( it = m_rTree.begin(); it != m_rTree.end();
          it = m_rTree.getNextVisibleItem( it ) )
     {
-        if( it == m_lastPos ) break;
-        lastPosIndex++;
+        if( it == m_firstPos ) break;
+        firstPosIndex++;
     }
 
-    if( it == m_rTree.end() ) return;
+    if( it == m_rTree.end() ) return false;
 
 
     if( it != m_rTree.end()
-        && ( playIndex < lastPosIndex
-           || playIndex > lastPosIndex + maxItems() ) )
+        && ( focusItemIndex < firstPosIndex
+           || focusItemIndex > firstPosIndex + maxItems() ) )
     {
         // Scroll to have the playing stream visible
         VarPercent &rVarPos = m_rTree.getPositionVar();
-        rVarPos.set( 1.0 - (double)playIndex / (double)m_rTree.visibleItems() );
+        rVarPos.set( 1.0 - (double)focusItemIndex /
+                           (double)m_rTree.visibleItems() );
+        return true;
     }
-    else
+    return false;
+}
+
+void CtrlTree::autoScroll()
+{
+    // Find the current playing stream
+    int playIndex = 0;
+    VarTree::Iterator it;
+    for( it = m_rTree.begin(); it != m_rTree.end();
+         it = m_rTree.getNextVisibleItem( it ) )
     {
-        makeImage();
-        notifyLayout();
+        if( it->m_playing ) break;
+        playIndex++;
     }
+
+    if( it == m_rTree.end() ) return;
+
+    ensureVisible( playIndex );
 }
 
+
 void CtrlTree::makeImage()
 {
     if( m_pImage )
@@ -531,7 +543,7 @@ void CtrlTree::makeImage()
     OSFactory *pOsFactory = OSFactory::instance( getIntf() );
     m_pImage = pOsFactory->createOSGraphics( width, height );
 
-    VarTree::Iterator it = m_lastPos;
+    VarTree::Iterator it = m_firstPos;
 
     if( m_pBgBitmap )
     {
@@ -579,7 +591,7 @@ void CtrlTree::makeImage()
     int bitmapWidth = itemImageWidth();
 
     int yPos = 0;
-    it = m_lastPos;
+    it = m_firstPos;
     while( it != m_rTree.end() && yPos < height )
     {
         const GenericBitmap *m_pCurBitmap;
@@ -628,14 +640,17 @@ void CtrlTree::makeImage()
         }
         it = m_rTree.getNextVisibleItem( it );
     }
+    /* TODO: Reposition percentage var to accomodate if it's not suitable anymore
+     * (if we expanded a node)
+     */
 }
 
 VarTree::Iterator CtrlTree::findItemAtPos( int pos )
 {
-    // The first item is m_lastPos.
+    // The first item is m_firstPos.
     // We decrement pos as we try the other items, until pos == 0.
     VarTree::Iterator it;
-    for( it = m_lastPos; it != m_rTree.end() && pos != 0;
+    for( it = m_firstPos; it != m_rTree.end() && pos != 0;
          it = m_rTree.getNextVisibleItem( it ) )
     {
         pos--;
@@ -643,4 +658,3 @@ VarTree::Iterator CtrlTree::findItemAtPos( int pos )
 
     return it;
 }
-

modules/gui/skins2/controls/ctrl_tree.hpp

@@ -33,7 +33,7 @@ class GenericFont;
 class GenericBitmap;
 
 /// Class for control tree
-class CtrlTree: public CtrlGeneric, public Observer<VarTree, int>,
+class CtrlTree: public CtrlGeneric, public Observer<VarTree, tree_update*>,
     public Observer<VarPercent, void*>
 {
     public:
@@ -71,6 +71,16 @@ class CtrlTree: public CtrlGeneric, public Observer<VarTree, int>,
         /// Get the type of control (custom RTTI)
         virtual string getType() const { return "tree"; }
 
+        /// Make sure an item is visible
+        /// \param item an iterator to a tree item
+        /// \return true if it changed the position
+        bool CtrlTree::ensureVisible( VarTree::Iterator item );
+
+        /// Make sure an item is visible
+        /// \param itemIndex the absolute index in the tree
+        /// \return true if it changed the position
+        bool CtrlTree::ensureVisible( int itemIndex );
+
     private:
         /// Tree associated to the control
         VarTree &m_rTree;
@@ -98,11 +108,12 @@ class CtrlTree: public CtrlGeneric, public Observer<VarTree, int>,
         VarTree *m_pLastSelected;
         /// Image of the control
         OSGraphics *m_pImage;
-        /// Last position
-        VarTree::Iterator m_lastPos;
+        /// First item in the visible area
+        VarTree::Iterator m_firstPos;
 
         /// Method called when the tree variable is modified
-        virtual void onUpdate( Subject<VarTree, int> &rTree , int);
+        virtual void onUpdate( Subject<VarTree, tree_update*> &rTree ,
+                               tree_update *);
 
         // Method called when the position variable of the tree is modified
         virtual void onUpdate( Subject<VarPercent, void *> &rPercent , void *);

modules/gui/skins2/src/vlcproc.cpp

@@ -122,9 +122,8 @@ VlcProc::VlcProc( intf_thread_t *pIntf ): SkinObject( pIntf ),
     var_AddCallback( pIntf->p_sys->p_playlist, "intf-change",
                      onIntfChange, this );
     // Called when a playlist item is added
-    // TODO: properly handle item-append
     var_AddCallback( pIntf->p_sys->p_playlist, "item-append",
-                     onIntfChange, this );
+                     onItemAppend, this );
     // Called when a playlist item is deleted
     // TODO: properly handle item-deleted
     var_AddCallback( pIntf->p_sys->p_playlist, "item-deleted",
@@ -167,7 +166,7 @@ VlcProc::~VlcProc()
     var_DelCallback( getIntf()->p_sys->p_playlist, "intf-change",
                      onIntfChange, this );
     var_DelCallback( getIntf()->p_sys->p_playlist, "item-append",
-                     onIntfChange, this );
+                     onItemAppend, this );
     var_DelCallback( getIntf()->p_sys->p_playlist, "item-deleted",
                      onIntfChange, this );
     var_DelCallback( getIntf()->p_sys->p_playlist, "intf-show",
@@ -411,6 +410,33 @@ int VlcProc::onItemChange( vlc_object_t *pObj, const char *pVariable,
     return VLC_SUCCESS;
 }
 
+int VlcProc::onItemAppend( vlc_object_t *pObj, const char *pVariable,
+                           vlc_value_t oldVal, vlc_value_t newVal,
+                           void *pParam )
+{
+    VlcProc *pThis = (VlcProc*)pParam;
+
+    playlist_add_t *p_add = (playlist_add_t*)malloc( sizeof(
+                                                playlist_add_t ) ) ;
+
+    memcpy( p_add, newVal.p_address, sizeof( playlist_add_t ) ) ;
+
+    // Create a playlist notify command (for old style playlist)
+    CmdNotifyPlaylist *pCmd = new CmdNotifyPlaylist( pThis->getIntf() );
+
+    // Create a playtree notify command (for new style playtree)
+    CmdPlaytreeAppend *pCmdTree = new CmdPlaytreeAppend( pThis->getIntf(),
+                                                         p_add );
+
+    // Push the command in the asynchronous command queue
+    AsyncQueue *pQueue = AsyncQueue::instance( pThis->getIntf() );
+    pQueue->push( CmdGenericPtr( pCmd ) );
+    pQueue->push( CmdGenericPtr( pCmdTree ), false );
+
+    return VLC_SUCCESS;
+}
+
+
 
 int VlcProc::onPlaylistChange( vlc_object_t *pObj, const char *pVariable,
                                vlc_value_t oldVal, vlc_value_t newVal,

modules/gui/skins2/src/vlcproc.hpp

@@ -168,6 +168,11 @@ class VlcProc: public SkinObject
                                  vlc_value_t oldVal, vlc_value_t newVal,
                                  void *pParam );
 
+        /// Callback for item-change variable
+        static int onItemAppend( vlc_object_t *pObj, const char *pVariable,
+                                 vlc_value_t oldVal, vlc_value_t newVal,
+                                 void *pParam );
+
         /// Callback for playlist-current variable
         static int onPlaylistChange( vlc_object_t *pObj, const char *pVariable,
                                      vlc_value_t oldVal, vlc_value_t newVal,

modules/gui/skins2/utils/var_tree.cpp

@@ -2,7 +2,7 @@
  * var_tree.cpp
  *****************************************************************************
  * Copyright (C) 2005 VideoLAN
- * $Id: var_bool.hpp 9934 2005-02-15 13:55:08Z courmisch $
+ * $Id$
  *
  * Authors: Antoine Cellerier <dionoea@videolan.org>
  *
@@ -28,7 +28,7 @@ const string VarTree::m_type = "tree";
 
 VarTree::VarTree( intf_thread_t *pIntf )
     : Variable( pIntf ), m_id( 0 ), m_selected( false ), m_playing( false ),
-    m_expanded( true ), m_pData( NULL ), m_pParent( NULL )
+    m_expanded( false ), m_pData( NULL ), m_pParent( NULL )
 {
     // Create the position variable
     m_cPosition = VariablePtr( new VarPercent( pIntf ) );
@@ -57,7 +57,6 @@ void VarTree::add( int id, const UStringPtr &rcString, bool selected,
 {
     m_children.push_back( VarTree( getIntf(), this, id, rcString, selected,
                                    playing, expanded, pData ) );
-    notify();
 }
 
 void VarTree::delSelected()
@@ -79,7 +78,6 @@ void VarTree::delSelected()
             it++;
         }
     }
-    notify();
 }
 
 void VarTree::clear()

modules/gui/skins2/utils/var_tree.hpp

@@ -31,8 +31,16 @@
 #include "ustring.hpp"
 #include "var_percent.hpp"
 
+/// Description of an update to the tree
+typedef struct tree_update
+{
+     int i_type;
+     int i_parent;
+     int i_id;
+} tree_update;
+
 /// Tree variable
-class VarTree: public Variable, public Subject<VarTree, int>
+class VarTree: public Variable, public Subject<VarTree, tree_update*>
 {
     public:
         VarTree( intf_thread_t *pIntf );

modules/gui/skins2/vars/playtree.cpp

@@ -78,7 +78,9 @@ void Playtree::delSelected()
         }
     }
     buildTree();
-    notify( 1 );
+    tree_update descr;
+    descr.i_type = 1;
+    notify( &descr );
 }
 
 void Playtree::action( VarTree *pItem )
@@ -106,10 +108,12 @@ void Playtree::action( VarTree *pItem )
 void Playtree::onChange()
 {
     buildTree();
-    notify( 1 );
+    tree_update descr;
+    descr.i_type = 1;
+    notify( &descr );
 }
 
-void Playtree::onUpdate( int id )
+void Playtree::onUpdateItem( int id )
 {
     Iterator it = findById( id );
     if( it != end() )
@@ -124,8 +128,32 @@ void Playtree::onUpdate( int id )
     {
         msg_Warn(getIntf(), "Cannot find node with id %d", id );
     }
-    // TODO update only the right node
-    notify( 0 );
+    tree_update descr;
+    descr.i_type = 0;
+    notify( &descr );
+}
+
+void Playtree::onAppend( playlist_add_t *p_add )
+{
+    Iterator node = findById( p_add->i_node );
+    if( node != end() )
+    {
+        Iterator item =  findById( p_add->i_item );
+        if( item == end() )
+        {
+            playlist_item_t *p_item = playlist_ItemGetById(
+                                        m_pPlaylist, p_add->i_item );
+            if( !p_item ) return;
+            UString *pName = new UString( getIntf(), p_item->input.psz_name );
+            node->add( p_add->i_item, UStringPtr( pName ),
+                      false,false, false, p_item );
+        }
+    }
+    tree_update descr;
+    descr.i_id = p_add->i_item;
+    descr.i_parent = p_add->i_node;
+    descr.i_type = 2;
+    notify( &descr );
 }
 
 void Playtree::buildNode( playlist_item_t *pNode, VarTree &rTree )
@@ -137,7 +165,7 @@ void Playtree::buildNode( playlist_item_t *pNode, VarTree &rTree )
         rTree.add( pNode->pp_children[i]->input.i_id, UStringPtr( pName ),
                      false,
                      m_pPlaylist->status.p_item == pNode->pp_children[i],
-                     true, pNode->pp_children[i] );
+                     false, pNode->pp_children[i] );
         if( pNode->pp_children[i]->i_children )
         {
             buildNode( pNode->pp_children[i], rTree.back() );

modules/gui/skins2/vars/playtree.hpp

@@ -2,7 +2,7 @@
  * playtree.hpp
  *****************************************************************************
  * Copyright (C) 2005 VideoLAN
- * $Id: playlist.hpp 8659 2004-09-07 21:16:49Z gbazin $
+ * $Id$
  *
  * Authors: Antoine Cellerier <dionoea@videolan.org>
  *
@@ -43,7 +43,10 @@ class Playtree: public VarTree
         void onChange();
 
         /// Function called to notify playlist item update
-        void onUpdate( int id );
+        void onUpdateItem( int id );
+
+        /// Function called to notify playlist item  append
+        void onAppend( playlist_add_t * );
 
     private:
         /// VLC playlist object