Differential D11167

[WIP] move all the real models into a thread distinct from the UI thread to not block
AbandonedPublic
Actions

Authored by mgallien on Mar 9 2018, 6:33 AM.

Details

Reviewers

None

Group Reviewers

Elisa

Summary

move all the real models into a thread distinct from the UI thread to not block

mostly works like before but without blocking the UI thread

not

Test Plan

Manually test and did not notice too much problems.
Need more tests.

Diff Detail

Repository

R255 Elisa

Branch

threadedMediaPlayList

Lint

No Linters Available

Unit

No Unit Test Coverage

mgallien requested review of this revision.Mar 9 2018, 6:33 AM

mgallien created this revision.

Someone once said to me that large diffs are hard to review :)

Just from reading the code, the idea is to put the mediaplaylist to the worker thread the same as all the models, and the proxy model is basically used for interacting with the gui? If so, I'm not so sure abut this approach. Basically we are not using the functionality of qsortilterproxymodel at all, it's just an additional layer between interface and model. Or do you plan to use some of the sorting/filter options?
Can't this also be achieved by moving the playlist to the different thread and connect it by signals?

I haven't tested yet, but some comments/questions inline

src/elisaapplication.cpp
270	Does the proxy model really have to know about the ensurePlay signal?
src/qml/ElisaMainWindow.qml
877	You are reverting changes I made here. Did you do this on purpose or is this a leftover from rebase? I didn't like the extra variable just for one view, that's why I removed it and pushed the loading of the album data to the models.

In D11167#222095, @astippich wrote:

Someone once said to me that large diffs are hard to review :)

Just from reading the code, the idea is to put the mediaplaylist to the worker thread the same as all the models, and the proxy model is basically used for interacting with the gui? If so, I'm not so sure abut this approach. Basically we are not using the functionality of qsortilterproxymodel at all, it's just an additional layer between interface and model. Or do you plan to use some of the sorting/filter options?
Can't this also be achieved by moving the playlist to the different thread and connect it by signals?

I haven't tested yet, but some comments/questions inline

I am working on splitting this diff to only include the fast restore of big playlist on start. This one will already be quite big due to changes to the automatic tests.

astippich mentioned this in D12317: implement and enable sorting in the views.Apr 26 2018, 6:43 AM

mgallien mentioned this in D12785: [WIP] introduce GenericDataModel and ModelDataCache to have a generic model.May 12 2018, 7:55 PM

mgallien abandoned this revision.Jun 6 2018, 6:38 PM

Revision Contents
Changeset List

		Path
M		autotests/CMakeLists.txt (9 lines)
M		autotests/allalbumsmodeltest.cpp (2 lines)
M		autotests/manageheaderbartest.cpp (62 lines)
M		autotests/mediaplaylisttest.cpp (240 lines)
M		autotests/trackslistenertest.cpp (58 lines)
M		src/CMakeLists.txt (4 lines)
M		src/elisaapplication.h (6 lines)
M		src/elisaapplication.cpp (81 lines)
M		src/main.cpp (5 lines)
M		src/mediaplaylist.h (33 lines)
M		src/mediaplaylist.cpp (636 lines)
M		src/models/allalbumsmodel.cpp (97 lines)
M		src/models/allartistsmodel.cpp (45 lines)
M		src/models/alltracksmodel.cpp (59 lines)
A	M	src/models/mediaplaylistproxymodel.h (175 lines)
A	M	src/models/mediaplaylistproxymodel.cpp (188 lines)
M		src/mpris2/mediaplayer2player.h (6 lines)
M		src/mpris2/mediaplayer2player.cpp (4 lines)
M		src/mpris2/mpris2.h (10 lines)
M		src/mpris2/mpris2.cpp (4 lines)
M		src/musiclistenersmanager.h (6 lines)
M		src/musiclistenersmanager.cpp (32 lines)
M		src/qml/ElisaMainWindow.qml (11 lines)
M		src/qml/GridBrowserView.qml (8 lines)
M		src/qml/MediaAlbumView.qml (17 lines)
M		src/qml/MediaAllTracksView.qml (4 lines)
M		src/qml/MediaPlayListView.qml (2 lines)
M		src/trackslistener.h (3 lines)
M		src/trackslistener.cpp (8 lines)

Commit	Tree	Parents	Author	Summary	Date
5281a823c502	f21bb9d005a3	c6e928ffbae6	Matthieu Gallien	[WIP] move all the real models into a thread distinct from the UI thread to not… (Show More…)	Mar 7 2018, 7:05 PM

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	29058	c6e928f		Mar 9 2018, 6:33 AM	★	★

[WIP] move all the real models into a thread distinct from the UI thread to not blockAbandonedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 29058

autotests/CMakeLists.txt

autotests/allalbumsmodeltest.cpp

autotests/manageheaderbartest.cpp

autotests/mediaplaylisttest.cpp

autotests/trackslistenertest.cpp

src/CMakeLists.txt

src/elisaapplication.h

src/elisaapplication.cpp

src/main.cpp

src/mediaplaylist.h

src/mediaplaylist.cpp

src/models/allalbumsmodel.cpp

src/models/allartistsmodel.cpp

src/models/alltracksmodel.cpp

src/models/mediaplaylistproxymodel.h

src/models/mediaplaylistproxymodel.cpp

src/mpris2/mediaplayer2player.h

src/mpris2/mediaplayer2player.cpp

src/mpris2/mpris2.h

src/mpris2/mpris2.cpp

src/musiclistenersmanager.h

src/musiclistenersmanager.cpp

src/qml/ElisaMainWindow.qml

src/qml/GridBrowserView.qml

src/qml/MediaAlbumView.qml

src/qml/MediaAllTracksView.qml

src/qml/MediaPlayListView.qml

src/trackslistener.h

src/trackslistener.cpp

[WIP] move all the real models into a thread distinct from the UI thread to not block
AbandonedPublic
Actions

Revision Contents
Changeset List