D22256: [Image Wallpaper Slideshow] Allow setting of different sorting orders POC

msdobrescu updated this revision to Diff 61201. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22256?vs=61191=61201 REVISION DETAIL https://phabricator.kde.org/D22256 AFFECTED FILES wallpapers/image/backgroundlistmodel.cpp wallpapers/image/backgroundlistmodel.h

D22256: [Image Wallpaper Slideshow] Allow setting of different sorting orders POC

msdobrescu updated this revision to Diff 61191. msdobrescu edited the summary of this revision. msdobrescu added a comment. Added Reversed, DirsFirst/DirsLast, IgnoreCase, LocaleAware options. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22256?vs=6=61191 REVISION DETAIL

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. Indeed, at should crash, but when is that string empty? I can't crash mine, although inherits the same code. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22121 To: davidre, #plasma, davidedmundson Cc: davidedmundson,

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. IMHO, for lines like these, 'm_currentSlide = m_slideFilterModel->indexOf(path) - 1;', checks must be added because it is transparent to the QML code and let room for failure. Besides that, did you know that if there are no wallpapers, could be added one or

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. Ha Ha Ha! I know! I have asked you about, remember? Sometimes the actual build goes bananas and needs a reset. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22121 To: davidre, #plasma Cc: davidedmundson, msdobrescu, ngraham,

D22256: [Image Wallpaper Slideshow] Allow setting of different sorting orders POC

msdobrescu added a comment. In D22256#491942 , @davidedmundson wrote: > The fundamental problem with this approach is you only sort through the dirs correctly, but not the total across multiple paths. > > i.e > If I have two paths dirA

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. Is it more efficient from the performance point of view to have a QSortFilterProxyModel class per sorting method? I have a 6000 images case, would that be consuming compared to setting the sort class once according to the sorting method? REPOSITORY R120 Plasma

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. Could it be some threading issue? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22121 To: davidre, #plasma, davidedmundson Cc: davidedmundson, msdobrescu, ngraham, filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2,

D22256: [Image Wallpaper Slideshow] Allow setting of different sorting orders POC

msdobrescu created this revision. msdobrescu added reviewers: ngraham, davidre. msdobrescu added a project: Plasma. Herald added a subscriber: plasma-devel. msdobrescu requested review of this revision. REVISION SUMMARY Another take of bug 186181, as POC. The idea is to use filesystem sorting

D22256: [Image Wallpaper Slideshow] Allow setting of different sorting orders POC

msdobrescu added a comment. In D22256#490558 , @davidedmundson wrote: > This is awkward timing. > > See D22121 : [Image Wallpaper Slideshow] Allow setting of different sorting orders > > Sorry

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. Thanks, selecting from any image is useful, to avoid starting always from the top one. But should not enable the button until a check is done. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22121 To: davidre, #plasma,

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. I'd say it is some half-implemented feature, probably. Could it be useful to navigate by keyboard and check/uncheck images, by using space, for example? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22121 To: davidre, #plasma,

D22256: [Image Wallpaper Slideshow] Allow setting of different sorting orders POC

msdobrescu added a comment. In D22256#492127 , @davidre wrote: > I guess one drawback is that you have to reindex all the files when the sort mode is changed. I expect to simply reload the list, from the filesystem. That should be done

D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

msdobrescu added a comment. In D22121#491369 , @ngraham wrote: > Huh, in a VM everything works for me too. I wonder if there's some subtle issue with the fact that I'm running a custom-compiled Plasma at a different path on bare metal.