Grouping in the playlist is having problems for various artist albums when grouped by album. Songs from different artists get grouped together.
Is this still valid in Amarok 2.2.2 or current git?
yes
I guess the "Yes" is about Amarok 2.2.2, right?
Hello Lydia, My refactoring of the GroupingProxy has been merged, on 2010-02-22. That refactoring was mainly for performance, not for changes to the grouping algorithm, but it is useful if you can re-test this bug. If the problem is still present later than 2010-02-22, could you describe a specific example?
Created attachment 41795 [details] various artist album grouping bug I unfortunately can't test trunk atm. However the attached screenshot shows how it looks like in 2.3.0. There are two albums in the playlist. Both various artist albums. The group header just shows one artist of the whole album while the individual tracks have correct artists set.
Ah, I see what you mean. The *grouping* works exactly as requested: the setting is "Group by album", and grouped-together tracks 1..15 *are* all on the same album ("Divas of Soul"). What could be improved is the *display of the individual items*: - The layout "Head" should contain "Album artist" instead of "Artist". - The layout "Body" should contain something like "Artist, if different from Album artist". Right?
That makes sense I think, yes.
commit a0c2c3327c45cf06c92901210baac982ebdf505b Author: Nanno Langstraat <langstr@gmail.com> Date: Wed Mar 24 22:14:12 2010 +0100 Introduce a 4th playlist layout type: "Body (Various artists)". This applies to playlist items that: - Are part of a group ("same album", in the normal case) - Have a different track artist than album's artist. BUG:219618 As part of this change, the default 'Head' shows "Album artist", not "Artist" like it used to. diff --git a/src/data/DefaultPlaylistLayouts.xml b/src/data/DefaultPlaylistLayouts.xml index d6ebc59..52897e1 100644 --- a/src/data/DefaultPlaylistLayouts.xml +++ b/src/data/DefaultPlaylistLayouts.xml @@ -16,7 +16,7 @@ <element value="Album" size="1" bold="true" alignment="center"/> </row> <row> - <element value="Artist" size="1" bold="true" alignment="center"/> + <element value="Album artist" size="1" bold="true" alignment="center"/> </row> </group_head> <group_body show_cover="false"> @@ -25,6 +25,15 @@ <element value="Length" size="0.2" bold="false" alignment="right"/> </row> </group_body> + <group_variousArtistsBody show_cover="false"> + <row> + <element value="Title (with track number)" size="0.8" bold="false" alignment="left"/> + <element value="Length" size="0.2" bold="false" alignment="right"/> + </row> + <row> + <element value="Artist" prefix=" " size="0" bold="false" italic="true" alignment="left"/> + </row> + </group_variousArtistsBody> </layout> <layout name="No Grouping" group_by="None"> <single_track show_cover="true"> @@ -84,7 +93,7 @@ <element size="1" bold="true" alignment="left" value="Album" italic="false" /> </row> <row> - <element size="0.8" bold="true" alignment="left" value="Artist" italic="true" /> + <element size="0.8" bold="true" alignment="left" value="Album artist" italic="true" /> <element size="0" bold="false" alignment="right" value="Type" italic="false" /> </row> <row> @@ -103,5 +112,15 @@ <element size="0.15" bold="false" alignment="right" value="Length" italic="false" /> </row> </group_body> + <group_variousArtistsBody active_indicator_row="0" show_cover="false" > + <row> + <element size="0.7" bold="false" alignment="left" value="Title (with track number)" italic="true" /> + <element size="0.15" bold="false" alignment="center" value="Rating" italic="false" /> + <element size="0.15" bold="false" alignment="right" value="Length" italic="false" /> + </row> + <row> + <element value="Artist" prefix=" " size="0" bold="false" italic="true" alignment="left"/> + </row> + </group_variousArtistsBody> </layout> </playlist_layouts> diff --git a/src/playlist/layouts/LayoutItemConfig.cpp b/src/playlist/layouts/LayoutItemConfig.cpp index bf9acb3..5a3655b 100644 --- a/src/playlist/layouts/LayoutItemConfig.cpp +++ b/src/playlist/layouts/LayoutItemConfig.cpp @@ -1,6 +1,7 @@ /**************************************************************************************** * Copyright (c) 2008 Nikolaj Hald Nielsen <nhn@kde.org> * * Copyright (c) 2010 Oleksandr Khayrullin <saniokh@gmail.com> * + * Copyright (c) 2010 Nanno Langstraat <langstr@gmail.com> * * * * This program is free software; you can redistribute it and/or modify it under * * the terms of the GNU General Public License as published by the Free Software * @@ -14,9 +15,11 @@ * You should have received a copy of the GNU General Public License along with * * this program. If not, see <http://www.gnu.org/licenses/>. * ****************************************************************************************/ - + #include "LayoutItemConfig.h" +#include "playlist/proxymodels/GroupingProxy.h" // For 'GroupMode' + namespace Playlist { LayoutItemConfigRowElement::LayoutItemConfigRowElement( int value, qreal size, @@ -163,9 +166,14 @@ LayoutItemConfig Playlist::PlaylistLayout::head() const return m_head; } -LayoutItemConfig Playlist::PlaylistLayout::body() const +LayoutItemConfig Playlist::PlaylistLayout::standardBody() const { - return m_body; + return m_standardBody; +} + +LayoutItemConfig Playlist::PlaylistLayout::variousArtistsBody() const +{ + return m_variousArtistsBody; } LayoutItemConfig Playlist::PlaylistLayout::single() const @@ -173,14 +181,85 @@ LayoutItemConfig Playlist::PlaylistLayout::single() const return m_single; } +Playlist::PlaylistLayout::LayoutType +Playlist::PlaylistLayout::layoutTypeForItem( const QModelIndex &index ) const +{ + switch ( index.data( GroupRole ).toInt() ) + { + case Grouping::Head: // GroupMode + case Grouping::Body: + case Grouping::Tail: + { + Meta::TrackPtr track = index.data( TrackRole ).value<Meta::TrackPtr>(); + + if( !track->artist() || !track->album() || !track->album()->albumArtist() || ( track->artist()->name() != track->album()->albumArtist()->name() ) ) + return VariousArtistsBody; + else + return StandardBody; + } + + case Grouping::None: + default: + return Single; + } +} + +Playlist::LayoutItemConfig +Playlist::PlaylistLayout::layoutForItem( const QModelIndex &index ) const +{ + switch( layoutTypeForItem( index ) ) + { + case PlaylistLayout::Head: // LayoutType + return head(); + + case PlaylistLayout::StandardBody: + return standardBody(); + + case PlaylistLayout::VariousArtistsBody: + return variousArtistsBody(); + + case PlaylistLayout::Single: + default: + return single(); + } +} + +void +Playlist::PlaylistLayout::setLayout( LayoutType layoutType, LayoutItemConfig itemConfig ) +{ + switch( layoutType ) + { + case PlaylistLayout::Head: // LayoutType + setHead( itemConfig ); + break; + + case PlaylistLayout::StandardBody: + setStandardBody( itemConfig ); + break; + + case PlaylistLayout::VariousArtistsBody: + setVariousArtistsBody( itemConfig ); + break; + + case PlaylistLayout::Single: + setSingle( itemConfig ); + break; + } +} + void Playlist::PlaylistLayout::setHead( LayoutItemConfig head ) { m_head = head; } -void Playlist::PlaylistLayout::setBody( LayoutItemConfig body ) +void Playlist::PlaylistLayout::setStandardBody( LayoutItemConfig standardBody ) +{ + m_standardBody = standardBody; +} + +void Playlist::PlaylistLayout::setVariousArtistsBody( LayoutItemConfig variousArtistsBody ) { - m_body = body; + m_variousArtistsBody = variousArtistsBody; } void Playlist::PlaylistLayout::setSingle( LayoutItemConfig single ) diff --git a/src/playlist/layouts/LayoutItemConfig.h b/src/playlist/layouts/LayoutItemConfig.h index 1de164d..1aa052d 100644 --- a/src/playlist/layouts/LayoutItemConfig.h +++ b/src/playlist/layouts/LayoutItemConfig.h @@ -1,6 +1,7 @@ /**************************************************************************************** * Copyright (c) 2008 Nikolaj Hald Nielsen <nhn@kde.org> * * Copyright (c) 2010 Oleksandr Khayrullin <saniokh@gmail.com> * + * Copyright (c) 2010 Nanno Langstraat <langstr@gmail.com> * * * * This program is free software; you can redistribute it and/or modify it under * * the terms of the GNU General Public License as published by the Free Software * @@ -19,6 +20,7 @@ #define LAYOUTITEMCONFIG_H #include <QList> +#include <QModelIndex> #include <QString> namespace Playlist { @@ -226,6 +228,13 @@ class LayoutItemConfig class PlaylistLayout { public: + enum LayoutType { + Head, + StandardBody, + VariousArtistsBody, + Single + }; + /** * Default Constructor */ @@ -239,9 +248,15 @@ class PlaylistLayout /** * Get the config to use for painting group members. - * @return The config for group member tracks. + * @return The config for standard group member tracks. + */ + LayoutItemConfig standardBody() const; + + /** + * Get the config to use for painting group members. + * @return The config for group member tracks with a different artist than their album. */ - LayoutItemConfig body() const; + LayoutItemConfig variousArtistsBody() const; /** * Get the config to use for painting single (non grouped) tracks. @@ -263,6 +278,17 @@ class PlaylistLayout bool isDirty() const; /** + * Determine the layout type for an item in a QAbstractItemModel. + */ + LayoutType layoutTypeForItem( const QModelIndex &index ) const; + LayoutItemConfig layoutForItem( const QModelIndex &index ) const; + + /** + * Calls 'setHead()', 'setSingle()' etc. based on 'type'. + */ + void setLayout( LayoutType type, LayoutItemConfig itemConfig ); + + /** * Set the head config for this layout. * @param head The head config. */ @@ -272,7 +298,13 @@ class PlaylistLayout * Set the body config for this layout. * @param body The body config. */ - void setBody( LayoutItemConfig body ); + void setStandardBody( LayoutItemConfig body ); + + /** + * Set the body config for this layout. + * @param body The body config. + */ + void setVariousArtistsBody( LayoutItemConfig body ); /** * Set the single track config for this layout. @@ -303,7 +335,8 @@ class PlaylistLayout private: LayoutItemConfig m_head; - LayoutItemConfig m_body; + LayoutItemConfig m_standardBody; + LayoutItemConfig m_variousArtistsBody; LayoutItemConfig m_single; bool m_isEditable; bool m_isDirty; diff --git a/src/playlist/layouts/LayoutManager.cpp b/src/playlist/layouts/LayoutManager.cpp index 35973d9..1cd0651 100644 --- a/src/playlist/layouts/LayoutManager.cpp +++ b/src/playlist/layouts/LayoutManager.cpp @@ -205,7 +205,12 @@ void LayoutManager::loadLayouts( const QString &fileName, bool user ) currentLayout.setHead( parseItemConfig( layout.toElement().firstChildElement( "group_head" ) ) ); - currentLayout.setBody( parseItemConfig( layout.toElement().firstChildElement( "group_body" ) ) ); + currentLayout.setStandardBody( parseItemConfig( layout.toElement().firstChildElement( "group_body" ) ) ); + QDomElement variousArtistsXML = layout.toElement().firstChildElement( "group_variousArtistsBody" ); + if ( !variousArtistsXML.isNull() ) + currentLayout.setVariousArtistsBody( parseItemConfig( variousArtistsXML ) ); + else // Handle old custom layout XMLs + currentLayout.setVariousArtistsBody( parseItemConfig( layout.toElement().firstChildElement( "group_body" ) ) ); currentLayout.setSingle( parseItemConfig( layout.toElement().firstChildElement( "single_track" ) ) ); if ( !layoutName.isEmpty() ) @@ -299,7 +304,8 @@ void LayoutManager::addUserLayout( const QString &name, PlaylistLayout layout ) newLayout.appendChild( createItemElement( doc, "single_track", layout.single() ) ); newLayout.appendChild( createItemElement( doc, "group_head", layout.head() ) ); - newLayout.appendChild( createItemElement( doc, "group_body", layout.body() ) ); + newLayout.appendChild( createItemElement( doc, "group_body", layout.standardBody() ) ); + newLayout.appendChild( createItemElement( doc, "group_variousArtistsBody", layout.variousArtistsBody() ) ); if( layout.inlineControls() ) newLayout.setAttribute( "inline_controls", "true" ); diff --git a/src/playlist/layouts/PlaylistLayoutEditDialog.cpp b/src/playlist/layouts/PlaylistLayoutEditDialog.cpp index 7c5d661..abaf7e1 100644 --- a/src/playlist/layouts/PlaylistLayoutEditDialog.cpp +++ b/src/playlist/layouts/PlaylistLayoutEditDialog.cpp @@ -98,12 +98,14 @@ PlaylistLayoutEditDialog::PlaylistLayoutEditDialog( QWidget *parent ) //add an editor to each tab m_headEdit = new Playlist::LayoutEditWidget( this ); - m_bodyEdit = new Playlist::LayoutEditWidget( this ); + m_standardBodyEdit = new Playlist::LayoutEditWidget( this ); + m_variousArtistsBodyEdit = new Playlist::LayoutEditWidget( this ); m_singleEdit = new Playlist::LayoutEditWidget( this ); m_layoutsMap = new QMap<QString, PlaylistLayout>(); elementTabs->addTab( m_headEdit, i18n( "Head" ) ); - elementTabs->addTab( m_bodyEdit, i18n( "Body" ) ); + elementTabs->addTab( m_standardBodyEdit, i18n( "Body" ) ); + elementTabs->addTab( m_variousArtistsBodyEdit, i18n( "Body (Various artists)" ) ); elementTabs->addTab( m_singleEdit, i18n( "Single" ) ); QStringList layoutNames = LayoutManager::instance()->layouts(); @@ -160,7 +162,8 @@ PlaylistLayoutEditDialog::PlaylistLayoutEditDialog( QWidget *parent ) toggleUpDownButtons(); connect( m_headEdit, SIGNAL( changed() ), this, SLOT( setLayoutChanged() ) ); - connect( m_bodyEdit, SIGNAL( changed() ), this, SLOT( setLayoutChanged() ) ); + connect( m_standardBodyEdit, SIGNAL( changed() ), this, SLOT( setLayoutChanged() ) ); + connect( m_variousArtistsBodyEdit, SIGNAL( changed() ), this, SLOT( setLayoutChanged() ) ); connect( m_singleEdit, SIGNAL( changed() ), this, SLOT( setLayoutChanged() ) ); connect( inlineControlsChekbox, SIGNAL( stateChanged( int ) ), this, SLOT( setLayoutChanged() ) ); connect( tooltipsCheckbox, SIGNAL( stateChanged( int ) ), this, SLOT( setLayoutChanged() ) ); @@ -202,10 +205,12 @@ void PlaylistLayoutEditDialog::newLayout() //SLOT layoutListWidget->addItem( layoutName ); layoutListWidget->setCurrentItem( (layoutListWidget->findItems( layoutName, Qt::MatchExactly ) ).first() ); m_headEdit->clear(); - m_bodyEdit->clear(); + m_standardBodyEdit->clear(); + m_variousArtistsBodyEdit->clear(); m_singleEdit->clear(); layout.setHead( m_headEdit->config() ); - layout.setBody( m_bodyEdit->config() ); + layout.setStandardBody( m_standardBodyEdit->config() ); + layout.setVariousArtistsBody( m_variousArtistsBodyEdit->config() ); layout.setSingle( m_singleEdit->config() ); m_layoutsMap->insert( layoutName, layout ); @@ -217,7 +222,8 @@ void PlaylistLayoutEditDialog::newLayout() //SLOT void PlaylistLayoutEditDialog::copyLayout() { LayoutItemConfig headConfig = m_headEdit->config(); - LayoutItemConfig bodyConfig = m_bodyEdit->config(); + LayoutItemConfig standardBodyConfig = m_standardBodyEdit->config(); + LayoutItemConfig variousArtistsBodyConfig = m_variousArtistsBodyEdit->config(); LayoutItemConfig singleConfig = m_singleEdit->config(); QString layoutName = layoutListWidget->currentItem()->text(); @@ -246,7 +252,8 @@ void PlaylistLayoutEditDialog::copyLayout() headConfig.setActiveIndicatorRow( -1 ); layout.setHead( headConfig ); - layout.setBody( bodyConfig ); + layout.setStandardBody( standardBodyConfig ); + layout.setVariousArtistsBody( variousArtistsBodyConfig ); layout.setSingle( singleConfig ); layout.setInlineControls( inlineControlsChekbox->isChecked() ); @@ -316,7 +323,8 @@ void PlaylistLayoutEditDialog::setLayout( const QString &layoutName ) //SLOT { PlaylistLayout layout = m_layoutsMap->value( layoutName ); m_headEdit->readLayout( layout.head() ); - m_bodyEdit->readLayout( layout.body() ); + m_standardBodyEdit->readLayout( layout.standardBody() ); + m_variousArtistsBodyEdit->readLayout( layout.variousArtistsBody() ); m_singleEdit->readLayout( layout.single() ); inlineControlsChekbox->setChecked( layout.inlineControls() ); tooltipsCheckbox->setChecked( layout.tooltips() ); @@ -330,7 +338,8 @@ void PlaylistLayoutEditDialog::setLayout( const QString &layoutName ) //SLOT else { m_headEdit->clear(); - m_bodyEdit->clear(); + m_standardBodyEdit->clear(); + m_variousArtistsBodyEdit->clear(); m_singleEdit->clear(); } } @@ -343,7 +352,8 @@ void PlaylistLayoutEditDialog::preview() headConfig.setActiveIndicatorRow( -1 ); layout.setHead( headConfig ); - layout.setBody( m_bodyEdit->config() ); + layout.setStandardBody( m_standardBodyEdit->config() ); + layout.setVariousArtistsBody( m_variousArtistsBodyEdit->config() ); layout.setSingle( m_singleEdit->config() ); layout.setInlineControls( inlineControlsChekbox->isChecked() ); layout.setTooltips( tooltipsCheckbox->isChecked() ); @@ -485,13 +495,15 @@ void PlaylistLayoutEditDialog::setEnabledTabs() { //Grouping allowed - enable all tabs elementTabs->setTabEnabled(elementTabs->indexOf(m_headEdit), true); - elementTabs->setTabEnabled(elementTabs->indexOf(m_bodyEdit), true); + elementTabs->setTabEnabled(elementTabs->indexOf(m_standardBodyEdit), true); + elementTabs->setTabEnabled(elementTabs->indexOf(m_variousArtistsBodyEdit), true); } else { //Disable the head and body tabs, leaving only the single tab elementTabs->setTabEnabled(elementTabs->indexOf(m_headEdit), false); - elementTabs->setTabEnabled(elementTabs->indexOf(m_bodyEdit), false); + elementTabs->setTabEnabled(elementTabs->indexOf(m_standardBodyEdit), false); + elementTabs->setTabEnabled(elementTabs->indexOf(m_variousArtistsBodyEdit), false); elementTabs->setCurrentWidget(m_singleEdit); } } @@ -520,7 +532,8 @@ void PlaylistLayoutEditDialog::setLayoutChanged() setEnabledTabs(); (*m_layoutsMap)[m_layoutName].setHead( m_headEdit->config() ); - (*m_layoutsMap)[m_layoutName].setBody( m_bodyEdit->config() ); + (*m_layoutsMap)[m_layoutName].setStandardBody( m_standardBodyEdit->config() ); + (*m_layoutsMap)[m_layoutName].setVariousArtistsBody( m_variousArtistsBodyEdit->config() ); (*m_layoutsMap)[m_layoutName].setSingle( m_singleEdit->config() ); (*m_layoutsMap)[m_layoutName].setInlineControls( inlineControlsChekbox->isChecked() ); diff --git a/src/playlist/layouts/PlaylistLayoutEditDialog.h b/src/playlist/layouts/PlaylistLayoutEditDialog.h index 362713a..b89b991 100644 --- a/src/playlist/layouts/PlaylistLayoutEditDialog.h +++ b/src/playlist/layouts/PlaylistLayoutEditDialog.h @@ -14,7 +14,7 @@ * You should have received a copy of the GNU General Public License along with * * this program. If not, see <http://www.gnu.org/licenses/>. * ****************************************************************************************/ - + #ifndef PLAYLISTLAYOUTEDITDIALOG_H #define PLAYLISTLAYOUTEDITDIALOG_H @@ -130,11 +130,12 @@ class PlaylistLayoutEditDialog : public QDialog, private Ui::PlaylistLayoutEditD /** * Populates the grouping mode combo box with options */ - void setupGroupByCombo(); - - + void setupGroupByCombo(); + + Playlist::LayoutEditWidget *m_headEdit; - Playlist::LayoutEditWidget *m_bodyEdit; + Playlist::LayoutEditWidget *m_standardBodyEdit; + Playlist::LayoutEditWidget *m_variousArtistsBodyEdit; Playlist::LayoutEditWidget *m_singleEdit; QMap<QString, Playlist::PlaylistLayout> *m_layoutsMap; diff --git a/src/playlist/proxymodels/GroupingProxy.cpp b/src/playlist/proxymodels/GroupingProxy.cpp index 7ed304b..1a97766 100644 --- a/src/playlist/proxymodels/GroupingProxy.cpp +++ b/src/playlist/proxymodels/GroupingProxy.cpp @@ -102,15 +102,15 @@ Playlist::GroupingProxy::setGroupingCategory( const QString &groupingCategory ) bool Playlist::GroupingProxy::isFirstInGroup( const QModelIndex & index ) { - GroupMode mode = groupModeForIndex( index ); - return ( (mode == Head) || (mode == None) ); + Grouping::GroupMode mode = groupModeForIndex( index ); + return ( (mode == Grouping::Head) || (mode == Grouping::None) ); } bool Playlist::GroupingProxy::isLastInGroup( const QModelIndex & index ) { - GroupMode mode = groupModeForIndex( index ); - return ( (mode == Tail) || (mode == None) ); + Grouping::GroupMode mode = groupModeForIndex( index ); + return ( (mode == Grouping::Tail) || (mode == Grouping::None) ); } QModelIndex @@ -243,14 +243,14 @@ Playlist::GroupingProxy::proxyRowsRemoved( const QModelIndex& parent, int proxyS } -Playlist::GroupMode +Playlist::Grouping::GroupMode Playlist::GroupingProxy::groupModeForIndex( const QModelIndex & thisIndex ) { - GroupMode groupMode; + Grouping::GroupMode groupMode; - groupMode = m_cachedGroupModeForRow.value( thisIndex.row(), Invalid ); // Try to get from cache + groupMode = m_cachedGroupModeForRow.value( thisIndex.row(), Grouping::Invalid ); // Try to get from cache - if ( groupMode == Invalid ) + if ( groupMode == Grouping::Invalid ) { // Not in our cache QModelIndex prevIndex = thisIndex.sibling( thisIndex.row() - 1, thisIndex.column() ); // May be invalid, if 'thisIndex' is the first playlist item. QModelIndex nextIndex = thisIndex.sibling( thisIndex.row() + 1, thisIndex.column() ); // May be invalid, if 'thisIndex' is the last playlist item. @@ -263,13 +263,13 @@ Playlist::GroupingProxy::groupModeForIndex( const QModelIndex & thisIndex ) bool matchAfter = shouldBeGrouped( thisTrack, nextTrack ); // if ( !matchBefore && matchAfter ) - groupMode = Head; + groupMode = Grouping::Head; else if ( matchBefore && matchAfter ) - groupMode = Body; + groupMode = Grouping::Body; else if ( matchBefore && !matchAfter ) - groupMode = Tail; + groupMode = Grouping::Tail; else - groupMode = None; + groupMode = Grouping::None; m_cachedGroupModeForRow.insert( thisIndex.row(), groupMode ); // Cache our decision } diff --git a/src/playlist/proxymodels/GroupingProxy.h b/src/playlist/proxymodels/GroupingProxy.h index fb8e8f3..a21fbdd 100644 --- a/src/playlist/proxymodels/GroupingProxy.h +++ b/src/playlist/proxymodels/GroupingProxy.h @@ -40,6 +40,8 @@ enum GroupDataRoles GroupedTracksRole, // deprecated }; +namespace Grouping +{ enum GroupMode { None = 1, @@ -50,6 +52,7 @@ enum GroupMode Collapsed, // deprecated Invalid }; +} class GroupingProxy : public ProxyBase { @@ -126,7 +129,7 @@ private: /** * This function determines the "Status within the group" of a model row. */ - GroupMode groupModeForIndex( const QModelIndex & index ); + Grouping::GroupMode groupModeForIndex( const QModelIndex & index ); /** * This function is used to determine if 2 tracks belong in the same group. @@ -145,7 +148,7 @@ private: QString m_groupingCategory; int m_groupingCategoryIndex; - QHash<int, GroupMode> m_cachedGroupModeForRow; + QHash<int, Grouping::GroupMode> m_cachedGroupModeForRow; }; } // namespace Playlist diff --git a/src/playlist/view/listview/InlineEditorWidget.cpp b/src/playlist/view/listview/InlineEditorWidget.cpp index 48798eb..8fb1c4e 100644 --- a/src/playlist/view/listview/InlineEditorWidget.cpp +++ b/src/playlist/view/listview/InlineEditorWidget.cpp @@ -18,10 +18,10 @@ #include "Debug.h" #include "moodbar/MoodbarManager.h" +#include "playlist/PlaylistDefines.h" #include "playlist/layouts/LayoutManager.h" #include "PrettyItemDelegate.h" #include "SvgHandler.h" -#include "playlist/proxymodels/GroupingProxy.h" #include <kratingwidget.h> #include <KHBox> @@ -43,11 +43,10 @@ const qreal Playlist::MARGINH = 6.0; const qreal Playlist::MARGINBODY = 1.0; const qreal Playlist::PADDING = 1.0; -InlineEditorWidget::InlineEditorWidget( QWidget * parent, const QModelIndex &index, PlaylistLayout layout, int groupMode ) +InlineEditorWidget::InlineEditorWidget( QWidget * parent, const QModelIndex &index, PlaylistLayout layout, bool hasHeader ) : KVBox( parent ) , m_index( index ) , m_layout( layout ) - , m_groupMode( groupMode ) , m_layoutChanged( false ) { setContentsMargins( 0, 0, 0, 0 ); @@ -62,35 +61,15 @@ InlineEditorWidget::InlineEditorWidget( QWidget * parent, const QModelIndex &ind int s_fontHeight = bfm.height(); - int rowCount = 1; + int rowCount = PrettyItemDelegate::rowsForItem( m_index ); m_headerHeight = 0; - switch ( groupMode ) - { - case Head: - rowCount = layout.head().rows() + layout.body().rows(); - break; - - case Body: - rowCount = layout.body().rows(); - break; - - case Tail: - rowCount = layout.body().rows(); - break; - - case None: - default: - rowCount = layout.single().rows(); - break; - } - height = MARGIN * 2 + rowCount * s_fontHeight + ( rowCount - 1 ) * PADDING; setFixedHeight( height ); setFixedWidth( parent->width() ); - if( groupMode == Head ) + if( hasHeader ) m_headerHeight = ( height * layout.head().rows() ) / rowCount - 1; //prevent editor closing when cliking a rating widget or pressing return in a line edit. @@ -108,7 +87,7 @@ void InlineEditorWidget::createChildWidgets() DEBUG_BLOCK debug() << "width: " << width(); - + //if we are a head item, create a blank widget to make space for the head info if ( m_headerHeight != 0 ) @@ -119,20 +98,8 @@ void InlineEditorWidget::createChildWidgets() KHBox * trackBox = new KHBox( this ); - LayoutItemConfig config; - switch(m_groupMode) - { - //For now, we don't allow editing of the "head" data, just the body - case Head: - case Body: - case Tail: - config = m_layout.body(); - break; - case None: - default: - config = m_layout.single(); - break; - } + //For now, we don't allow editing of the "head" data, just the body + LayoutItemConfig config = m_layout.layoutForItem( m_index ); int rowCount = config.rows(); @@ -148,7 +115,7 @@ void InlineEditorWidget::createChildWidgets() //add a small "spacer" widget to offset the cover a little. QWidget * coverSpacer = new QWidget( trackBox ); coverSpacer->setFixedWidth( MARGINH - MARGIN ); - + QModelIndex coverIndex = m_index.model()->index( m_index.row(), CoverImage ); QPixmap albumPixmap = coverIndex.data( Qt::DisplayRole ).value<QPixmap>(); @@ -182,7 +149,7 @@ void InlineEditorWidget::createChildWidgets() QSplitter *rowWidget = new QSplitter( rowsWidget ); connect( rowWidget, SIGNAL( splitterMoved ( int, int ) ), this, SLOT( splitterMoved( int, int ) ) ); - + m_splitterRowMap.insert( rowWidget, i ); // the Oxygen widget style causes the frame around items too squashed @@ -380,8 +347,8 @@ void InlineEditorWidget::editValueChanged() debug() << "Storing changed value: " << edit->text(); m_changedValues.insert( role, edit->text() ); } - - + + } void InlineEditorWidget::ratingValueChanged() @@ -413,14 +380,14 @@ void InlineEditorWidget::splitterMoved( int pos, int index ) Q_UNUSED( pos ) Q_UNUSED( index ) - + QSplitter * splitter = dynamic_cast<QSplitter *>( sender() ); if ( !splitter ) return; int row = m_splitterRowMap.value( splitter ); debug() << "on row: " << row; - + //first, get total size of all items; QList<int> sizes = splitter->sizes(); @@ -437,21 +404,7 @@ void InlineEditorWidget::splitterMoved( int pos, int index ) newSizes << newSize; } - LayoutItemConfig itemConfig; - - switch ( m_groupMode ) - { - case Head: - case Body: - case Tail: - itemConfig = m_layout.body(); - break; - - case None: - default: - itemConfig = m_layout.single(); - break; - } + LayoutItemConfig itemConfig = m_layout.layoutForItem( m_index ); LayoutItemConfigRow rowConfig = itemConfig.row( row ); @@ -459,7 +412,7 @@ void InlineEditorWidget::splitterMoved( int pos, int index ) LayoutItemConfigRow newRowConfig; for( int i = 0; i<rowConfig.count(); i++ ) - { + { LayoutItemConfigRowElement element = rowConfig.element( i ); debug() << "item " << i << " old/new: " << element.size() << "/" << newSizes.at( i ); element.setSize( newSizes.at( i ) ); @@ -478,20 +431,8 @@ void InlineEditorWidget::splitterMoved( int pos, int index ) newItemConfig.addRow( itemConfig.row( i ) ); } - switch ( m_groupMode ) - { - case Head: - case Body: - case Tail: - m_layout.setBody( newItemConfig ); - break; - - case None: - default: - m_layout.setSingle( newItemConfig ); - break; - } - + m_layout.setLayout( m_layout.layoutTypeForItem( m_index ), newItemConfig ); + m_layoutChanged = true; } @@ -516,7 +457,7 @@ InlineEditorWidget::eventFilter( QObject *obj, QEvent *event ) debug() << "emitting editingDone!"; emit editingDone( this ); } - + return true; } else diff --git a/src/playlist/view/listview/InlineEditorWidget.h b/src/playlist/view/listview/InlineEditorWidget.h index fac3ac6..f14d6df 100644 --- a/src/playlist/view/listview/InlineEditorWidget.h +++ b/src/playlist/view/listview/InlineEditorWidget.h @@ -32,7 +32,7 @@ class InlineEditorWidget : public KVBox Q_OBJECT public: - InlineEditorWidget( QWidget * parent, const QModelIndex &index, Playlist::PlaylistLayout layout, int groupMode ); + InlineEditorWidget( QWidget * parent, const QModelIndex &index, Playlist::PlaylistLayout layout, bool hasHeader ); ~InlineEditorWidget(); QMap<int, QString> changedValues(); @@ -56,7 +56,6 @@ private: QPersistentModelIndex m_index; Playlist::PlaylistLayout m_layout; - int m_groupMode; QMap<QWidget *, int> m_editorRoleMap; QMap<int, QString> m_changedValues; diff --git a/src/playlist/view/listview/PrettyItemDelegate.cpp b/src/playlist/view/listview/PrettyItemDelegate.cpp index caac234..091cb97 100644 --- a/src/playlist/view/listview/PrettyItemDelegate.cpp +++ b/src/playlist/view/listview/PrettyItemDelegate.cpp @@ -2,6 +2,7 @@ * Copyright (c) 2007 Ian Monroe <ian@monroe.nu> * * Copyright (c) 2008-2009 Nikolaj Hald Nielsen <nhn@kde.org> * * Copyright (c) 2008 Soren Harward <stharward@gmail.com> * + * Copyright (c) 2010 Nanno Langstraat <langstr@gmail.com> * * * * This program is free software; you can redistribute it and/or modify it under * * the terms of the GNU General Public License as published by the Free Software * @@ -58,38 +59,21 @@ Playlist::PrettyItemDelegate::PrettyItemDelegate( QObject* parent ) PrettyItemDelegate::~PrettyItemDelegate() { } -int PrettyItemDelegate::getGroupMode( const QModelIndex &index) const +int PrettyItemDelegate::getGroupMode( const QModelIndex &index) { return index.data( GroupRole ).toInt(); } -int PrettyItemDelegate::rowsForItem( const QModelIndex &index ) const +int +PrettyItemDelegate::rowsForItem( const QModelIndex &index ) { - PlaylistLayout layout = LayoutManager::instance()->activeLayout(); + int rowCount = 0; - const int groupMode = getGroupMode(index); - int rowCount = 1; + if( getGroupMode( index ) == Grouping::Head ) + rowCount += layout.head().rows(); - switch ( groupMode ) - { - case Head: - rowCount = layout.head().rows() + layout.body().rows(); - break; - - case Body: - rowCount = layout.body().rows(); - break; - - case Tail: - rowCount = layout.body().rows(); - break; - - case None: - default: - rowCount = layout.single().rows(); - break; - } + rowCount += layout.layoutForItem( index ).rows(); return rowCount; } @@ -137,9 +121,8 @@ PrettyItemDelegate::paint( QPainter* painter, const QStyleOptionViewItem& option int rowCount = rowsForItem( index ); bool paintInlineControls = LayoutManager::instance()->activeLayout().inlineControls() && index.data( ActiveTrackRole ).toBool(); - if ( groupMode == None || groupMode == Body || groupMode == Tail ) + if ( groupMode == Grouping::None || groupMode == Grouping::Body || groupMode == Grouping::Tail ) { - int trackHeight = 0; int extraHeight = 0; QStyleOptionViewItem trackOption( option ); @@ -151,12 +134,7 @@ PrettyItemDelegate::paint( QPainter* painter, const QStyleOptionViewItem& option trackOption.rect = QRect( 0, 0, option.rect.width(), trackHeight ); } - if ( groupMode == None ) - paintItem( layout.single(), painter, trackOption, index ); - else if ( groupMode == Body ) - paintItem( layout.body(), painter, trackOption, index ); - else - paintItem( layout.body(), painter, trackOption, index ); + paintItem( layout.layoutForItem( index ), painter, trackOption, index ); if (paintInlineControls ) { @@ -165,7 +143,7 @@ PrettyItemDelegate::paint( QPainter* painter, const QStyleOptionViewItem& option } } - else if ( groupMode == Head ) + else if ( groupMode == Grouping::Head ) { //we need to split up the options for the actual header and the included first track @@ -177,12 +155,15 @@ PrettyItemDelegate::paint( QPainter* painter, const QStyleOptionViewItem& option QStyleOptionViewItem trackOption( option ); int headRows = layout.head().rows(); - int trackRows = layout.body().rows(); + int trackRows = layout.layoutForItem( index ).rows(); int totalRows = headRows + trackRows; //if this layout is completely empty, bail out or we will get in divide-by-zero trouble if ( totalRows == 0 ) + { + painter->restore(); return; + } if ( paintInlineControls ) { @@ -200,7 +181,7 @@ PrettyItemDelegate::paint( QPainter* painter, const QStyleOptionViewItem& option } trackOption.rect = QRect( 0, 0, option.rect.width(), trackHeight ); - paintItem( layout.body(), painter, trackOption, index ); + paintItem( layout.layoutForItem( index ), painter, trackOption, index ); if ( paintInlineControls ) { @@ -665,10 +646,10 @@ bool Playlist::PrettyItemDelegate::clicked( const QPoint &pos, const QRect &item QWidget * Playlist::PrettyItemDelegate::createEditor ( QWidget * parent, const QStyleOptionViewItem & option, const QModelIndex & index ) const { Q_UNUSED( option ); - DEBUG_BLOCK - const int groupMode = getGroupMode(index); - InlineEditorWidget * editor = new InlineEditorWidget( parent, index, LayoutManager::instance()->activeLayout(), groupMode ); + + bool hasHeader = ( getGroupMode( index ) == Grouping::Head ); + InlineEditorWidget * editor = new InlineEditorWidget( parent, index, LayoutManager::instance()->activeLayout(), hasHeader ); connect( editor, SIGNAL( editingDone( InlineEditorWidget *) ), this, SLOT( editorDone( InlineEditorWidget *) ) ); return editor; } diff --git a/src/playlist/view/listview/PrettyItemDelegate.h b/src/playlist/view/listview/PrettyItemDelegate.h index b653089..a929ecb 100644 --- a/src/playlist/view/listview/PrettyItemDelegate.h +++ b/src/playlist/view/listview/PrettyItemDelegate.h @@ -34,6 +34,8 @@ class PrettyItemDelegate : public QStyledItemDelegate { Q_OBJECT public: + static int rowsForItem( const QModelIndex &index ); + PrettyItemDelegate( QObject* parent = 0 ); ~PrettyItemDelegate(); @@ -81,9 +83,7 @@ private: QPointF centerImage( const QPixmap&, const QRectF& ) const; - int rowsForItem( const QModelIndex &index ) const; - - int getGroupMode( const QModelIndex &index) const; + static int getGroupMode( const QModelIndex &index); static QFontMetricsF* s_nfm; //normal static QFontMetricsF* s_ufm; //underline diff --git a/src/playlist/view/listview/PrettyListView.cpp b/src/playlist/view/listview/PrettyListView.cpp index 1012647..386a98f 100644 --- a/src/playlist/view/listview/PrettyListView.cpp +++ b/src/playlist/view/listview/PrettyListView.cpp @@ -119,20 +119,12 @@ Playlist::PrettyListView::PrettyListView( QWidget* parent ) connect( m_animationTimer, SIGNAL( timeout() ), this, SLOT( redrawActive() ) ); m_animationTimer->setInterval( 250 ); - if ( LayoutManager::instance()->activeLayout().inlineControls() ) - m_animationTimer->start(); - // Tooltips m_toolTipManager = new ToolTipManager(this); - // Indicate to the tooltip manager what to display based on the layout - // That way, we avoid doing it every time we want to display the tooltip - // This will be done every time the layout will be changed - m_toolTipManager->cancelExclusions(); - excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().head(), false); - excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().body(), false); - excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().single(), true); + + playlistLayoutChanged(); } Playlist::PrettyListView::~PrettyListView() @@ -565,7 +557,7 @@ bool Playlist::PrettyListView::mouseEventInHeader( const QMouseEvent* event ) const { QModelIndex index = indexAt( event->pos() ); - if ( index.data( GroupRole ).toInt() == Head ) + if ( index.data( GroupRole ).toInt() == Grouping::Head ) { QPoint mousePressPos = event->pos(); mousePressPos.rx() += horizontalOffset(); @@ -893,7 +885,8 @@ void Playlist::PrettyListView::playlistLayoutChanged() // This will be done every time the layout will be changed m_toolTipManager->cancelExclusions(); excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().head(), false); - excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().body(), false); + excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().standardBody(), false); + excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().variousArtistsBody(), false); excludeFieldsFromTooltip(LayoutManager::instance()->activeLayout().single(), true); update(); diff --git a/src/playlist/view/tooltips/ToolTipManager.cpp b/src/playlist/view/tooltips/ToolTipManager.cpp index 29b428f..4bb1a1e 100644 --- a/src/playlist/view/tooltips/ToolTipManager.cpp +++ b/src/playlist/view/tooltips/ToolTipManager.cpp @@ -98,7 +98,7 @@ void ToolTipManager::requestToolTip(const QModelIndex& index) m_track = track; KToolTip::hideTip(); - m_singleItem = (index.data( Playlist::GroupRole ).toInt() == Playlist::None); + m_singleItem = (index.data( Playlist::GroupRole ).toInt() == Playlist::Grouping::None); m_itemRect = m_view->visualRect(index); const QPoint pos = m_view->viewport()->mapToGlobal(m_itemRect.topLeft()); @@ -189,11 +189,11 @@ void ToolTipManager::prepareToolTip() text = QString( i18n( "No extra information available" ) ); } else - { + { text = QString("<table>"+ text +"</table>"); } showToolTip(image, text); - + } void ToolTipManager::showToolTip(const QIcon& icon, const QString& text) @@ -308,7 +308,7 @@ QString ToolTipManager::breakLongLinesHTML(const QString& text) else { QString textInLines; - + QStringList words = text.trimmed().split(' '); int lineLength = 0; while(words.size() > 0)
*** Bug 186864 has been marked as a duplicate of this bug. ***