support for standalone ksplash themes
ClosedPublic

Authored by mart on Apr 4 2018, 10:53 AM.

Details

Summary

support the new standalone splashscreens of D11918
also add a GHNS button for the new splashscreen store category

FEATURE: 358839

Test Plan

splashscreens are correctly listed, installed and removed
both ones in lnf packages and standalone are listed.

Diff Detail

Repository
R119 Plasma Desktop
Branch
mart/ksplashGhns
Lint
No Linters Available
Unit
No Unit Test Coverage
mart created this revision.Apr 4 2018, 10:53 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 4 2018, 10:53 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart requested review of this revision.Apr 4 2018, 10:53 AM

I'm a bit confused by a few things.

  1. We seem to have our own generic ksplash package format already. See KCMSplashScreen::load ~128

Which isn't deleted. How do your changes tie in with that - and with the existing themes?

  1. What's the advantage of using a separate package over re-using LnF packages with just ksplash in it.
kcms/ksplash/kcm.cpp
152 ↗(On Diff #31282)

This is loading separate splash packages

ngraham edited the summary of this revision. (Show Details)Apr 4 2018, 2:00 PM
mart added a comment.EditedApr 4 2018, 2:41 PM

I'm a bit confused by a few things.

  1. We seem to have our own generic ksplash package format already. See KCMSplashScreen::load ~128 Which isn't deleted. How do your changes tie in with that - and with the existing themes?

right now it just checks in lnf packages that provide a splashscreen, the changes keep that compatible, so all lnf packages with a splash will be listed, there isn't a splash only format supported atm

  1. What's the advantage of using a separate package over re-using LnF packages with just ksplash in it.

the idea is the store wants to move all packages that have a splash and nothing else as a separate splashscreen category and not having them listed in look and feel if everything it does is to just change the splash screen.
the separate package format is just a lnf package format with everything that is not a splashscren removed, so they would go in the splashscreen category in store.kde.org

kcms/ksplash/kcm.cpp
152 ↗(On Diff #31282)

availablepackages goes trough all the installed lnf packages and takes all those which actually do provide a splashscreen in it.
i added also the packages installed in their own prefix, so it will list both

the idea is the store wants to move all packages that have a splash and nothing else as a separate splashscreen category and not having them listed in look and feel if everything it does is to just change the splash screen.

Store change makes sense, as does the GHNS.
But the store doesn't know anything about contents. You can upload a pdf and tag it as a splash screen.

So what do we gain by making a new package structure for splash screens over using the current one?

mart added a comment.Apr 4 2018, 4:26 PM

the idea is the store wants to move all packages that have a splash and nothing else as a separate splashscreen category and not having them listed in look and feel if everything it

So what do we gain by making a new package structure for splash screens over using the current one?

actually i did it only for having a different packageroot.. which i can achieve it by setting by hand a different packageroot to the lnf structure on every use both in the kcm and ksplash, which would be the same, just a bit uglier

What do we need the different package root for?
Just for making sure it's not selected in the lnf KCM?

mart added a comment.Apr 5 2018, 10:22 AM

What do we need the different package root for?
Just for making sure it's not selected in the lnf KCM?

yes, to not list in lnf any of those that have splashscreen only.
another thing that could be done is to exclude from the list those that don't have neither a defaults not a layout.js, but that becomes quite a fragile heuristic (with even more disk access while listing), i would find cleaner to just have them installed somewhere else, to map 1:1 the store...

An argument to install everyhting in the same folder is if the store will ever have the possibility for items to have 2 categories, so the same thing would appear both in splashscreens and lnf.. i don't think tha6t's in the plan tough

We're going to see this again for lockscreen, logout, osd, previews, runcommand and the 3 kwin bits.

Is it still the long term plan for lnfs to have these?
It's fine if that's purely backwards compatibility, but it's a whole different story to have a plan where things come from two sources in two different ways.

There seems to be two perfectly viable long term strategies we're looking at:

  • We use LnFs everywhere and filter everywhere (what we were doing for splash)
  • We have separate packages for all the different parts (what this patch is going towards) and the goal is to make lnfs a true package of packages.

I'm happy with either.

but that becomes quite a fragile heuristic

The current state is that splash screens on the store appear on the look and feel screen. Developers need to explicitly make a change to work with the new system to avoid that. If it's a "breaking change" anyway, we still have all our options open right now.

mart updated this revision to Diff 31470.Apr 6 2018, 11:11 AM
  • use lnf packages for splashscreen
mart added a comment.Apr 6 2018, 11:13 AM

There seems to be two perfectly viable long term strategies we're looking at:

  • We use LnFs everywhere and filter everywhere (what we were doing for splash)

ok, so i modified the behavior such that the lnf kcm only shows packages which provide a "defaults" file or a "layouts" folder
splashscreens use the same structure again. with this, D11918 can be dropped

davidedmundson accepted this revision.Apr 6 2018, 1:37 PM
This revision is now accepted and ready to land.Apr 6 2018, 1:37 PM
This revision was automatically updated to reflect the committed changes.