[vlc-devel] [PATCH 2/3] qml: MultiCoverPreview: clean up repeater

[Pierre Lamot]

Hi,

On mardi 15 octobre 2019 15:15:45 CEST Romain Vimont wrote:
> On Wed, Oct 09, 2019 at 07:11:23PM +0200, Alexandre Janniaux wrote:
> > From: Pierre Lamot <pierre at videolabs.io>
> > 
> > albums.get should be removed and the count property is useless.
> > ---
> >  modules/gui/qt/qml/utils/MultiCoverPreview.qml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/modules/gui/qt/qml/utils/MultiCoverPreview.qml b/modules/gui/qt/qml/utils/MultiCoverPreview.qml
> > index 5fc3c345bd7..2dc9e3e2c61 100644
> > --- a/modules/gui/qt/qml/utils/MultiCoverPreview.qml
> > +++ b/modules/gui/qt/qml/utils/MultiCoverPreview.qml
> > @@ -33,6 +33,7 @@
> >  
> >  import QtQuick 2.11
> >  import QtQuick.Layouts 1.3
> > +import org.videolan.medialib 0.1
> >  import "qrc:///style"
> >  
> >  Item {
> > @@ -49,7 +50,6 @@ Item {
> >          rowSpacing: VLCStyle.margin_xxxsmall
> >  
> >          Repeater {
> > -            property int count: albums.rowCount()
> >              model: Math.min(albums.rowCount(), 4)
> 
> (not related to this patch)
> The model is just a number?

Yeah, models doesn't abractitemmodel native way to limit/filter the elements to be used/displayed. 
So we iterate only on the required index avoids iterating on the whole (potentially long) model.
limit was added to the medialibrary models, but this wasn't the case when this class was written.

> >  
> >              /* One cover */
> > @@ -60,7 +60,7 @@ Item {
> >                  Layout.columnSpan: albums.rowCount() === 1 ? 2 : 1
> >                  Layout.fillHeight: true
> >                  Layout.fillWidth: true
> > -                source: albums.get(index).cover || VLCStyle.noArtAlbum
> > +                source: albums.data(albums.index(index, 0), MLAlbumModel.ALBUM_COVER) || VLCStyle.noArtAlbum
> 
> (not related to this patch)
> IMO the QML should not call .data() directly like this (it won't react
> to all model changes), but use the role "cover" directly.

Actually it doesn't really matters here.
This class mainly exists because the medialibrary doesn't provide thumbnails for genres. the class is instantiated once and the result is grabbed offscreen and cached.

Though, I expect this class to be removed in a near future, as most recent version (>= 5.13  IIRC) of Qt no longer allows to grab widget with mixed (native + qml) applications.
I plan to generate the thumbnail offline (probably using a QPainter) and cache it in the medialibrary, rather than generating it every time.

regards.

--
Pierre Lamot