Re: Planning merging the single qqml engine
On Mon, May 18, 2015 at 9:18 PM, David Edmundson wrote: > > > On Mon, May 18, 2015 at 8:28 PM, Marco Martin wrote: > >> Hi all, >> >> I think the branches for the single shared qqmlengine are pretty much >> ready >> (few cleanups upcoming days), seems pretty stable here.. did someone ran >> the >> branch for a while as well? > > > Thanks for doing all that. > > Now to make you hate me. > I have a crash, https://paste.kde.org/ppeqjl1c1 > > merging master into your branch fixed it. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Tuesday 19 May 2015, David Edmundson wrote: > > I would like to have all set before frameworks 5.11 > > > > One thing you and I talked about once was having the shell have a hook in > > p-f to call a static method setting what version of plasma-framework it was > expecting. > Would that be a suitable solution here? who would cann that on what? -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Mon, May 18, 2015 at 8:28 PM, Marco Martin wrote: > Hi all, > > I think the branches for the single shared qqmlengine are pretty much ready > (few cleanups upcoming days), seems pretty stable here.. did someone ran > the > branch for a while as well? > > In the end i managed to get a single engine for the whole session, views > included (had to duplicate the View class in plasmaquick and kept the old > one > as deprecated for retrocompatibility unfortunately) > > the memory save seems pretty good, i *hope* stability improved as well :p > > what still uses separed engines are the applet configuration dialogs: this > is > necessary because they are supposed to use a different style for the > qquickcontrols, and being the settings object that decides the style a qml > singleton (qml singletons are unique by engine), the engine has to be > different from the desktop/panel. The good thing is that config dialogs are > instantiated relatively rarely, in most sessions not at all, so shouldn't > touch too much a "normal run" > > For retrocompatibility the applets unfortunately have to specify explicitly > they can share the engine in their metadata file (or eventual > plasmapackage:/ > urls break) > > at the moment it's using > X-Plasma-RequiredExtensions=SharedEngine > > but I'm leaning more on the direction of something like > X-Plasma-MinimumAPIVersion=5.11 > > I would like to have all set before frameworks 5.11 > > One thing you and I talked about once was having the shell have a hook in p-f to call a static method setting what version of plasma-framework it was expecting. Would that be a suitable solution here? ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Monday 18 May 2015, Ivan Čukić wrote: > - make it opt-in (as Marco says) > - deprecate it > - report notifications to the user 'this applet might make your > desktop unstable' for all used applets that haven't opted-in - this > would serve as a notification both to the users and the developers of > said applets > - schedule marking the support for these applets for the release > after next, or something. this is with the explicitly white list is a thing that kills kittens and is over the top weird behavior, but at least doesn't force to litter metadatas and makes possible to definitely drop the legacy at some point -- Marco Martin diff --git a/src/plasmaquick/appletquickitem.cpp b/src/plasmaquick/appletquickitem.cpp index 3ffe269..b6d23d3 100644 --- a/src/plasmaquick/appletquickitem.cpp +++ b/src/plasmaquick/appletquickitem.cpp @@ -42,6 +42,9 @@ namespace PlasmaQuick QHash AppletQuickItemPrivate::s_rootObjects = QHash(); +QSet AppletQuickItemPrivate::s_legacyApplets = QSet({"org.kde.plasma.bluetooth", "org.kde.plasma.pager", "org.kde.desktopcontainment", "org.kde.plasma.folder", "org.kde.panel", "org.kde.plasma.analogclock", "org.kde.plasma.battery", "org.kde.plasma.systemtray"}); + + AppletQuickItemPrivate::AppletQuickItemPrivate(Plasma::Applet *a, AppletQuickItem *item) : q(item), switchWidth(-1), @@ -49,7 +52,13 @@ AppletQuickItemPrivate::AppletQuickItemPrivate(Plasma::Applet *a, AppletQuickIte applet(a), expanded(false) { -if (a->pluginInfo().property("X-Plasma-RequiredExtensions").toStringList().contains("SharedEngine")) { +//TODO: remove the legacy support at some point +//use the shared engine only for applets that are nt in the legacy list +//if they are, use the shared engine if their mayor version is at least 3 +const QStringList version = a->pluginInfo().version().split("."); +if (!AppletQuickItemPrivate::s_legacyApplets.contains(a->pluginInfo().pluginName()) || + (!version.isEmpty() && version.first().toInt() >= 3)) { + qmlObject = new KDeclarative::QmlObjectSharedEngine(q); if (!qmlObject->engine()->urlInterceptor()) { PackageUrlInterceptor *interceptor = new PackageUrlInterceptor(qmlObject->engine(), Plasma::Package()); diff --git a/src/plasmaquick/private/appletquickitem_p.h b/src/plasmaquick/private/appletquickitem_p.h index c754a80..79c1a2e 100644 --- a/src/plasmaquick/private/appletquickitem_p.h +++ b/src/plasmaquick/private/appletquickitem_p.h @@ -104,6 +104,7 @@ public: bool expanded : 1; static QHash s_rootObjects; +static QSet s_legacyApplets; }; } ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Monday 18 May 2015, David Edmundson wrote: > Now to make you hate me. > I have a crash, https://paste.kde.org/ppeqjl1c1 > > That's running: > kdeclarative - your branch > plasma-framework - your branch > plsama-workspace - master > (which is pretty close to running latest frameworks with Plasma 5.3) > > It's possibly unrelated, but I switched the top two back to master and it > went away. tried to run with p-w master, not getting that crash the bt says that Plasma::PluginLoader::self()->loadApplet() is somehow failing and returning a null ptr, that suggests it's not finding a valid package for it (wonder if i should return a new Applet * in any case there to be at least sure it's a valid pointer) can you see in console output at the last time that says "Loading applet: " what applet name says? -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Tuesday 19 May 2015, Marco Martin wrote: > On Monday 18 May 2015, Ivan Čukić wrote: > > While I'm usually more conservative, I do think that something more > > decisive should be done in this case (and cases like these). While we > > do not want to break everything with each new release (wink wink ghm > > ghm nudge nudge), I don't think we want to support every > > not-still-preferred-approach-of-implementing-something we made before > > indefinitely. > > a very crude approach may be maintaining a white list of name+version of > the applets that needs this, just to make old workspace work for a while > and eventually removing it the applets are: http://lxr.kde.org/search?_filestring=&_string=plasmapackage%3A%2F - bluetooth - pager - desktop cont - panel cont - analog clock - battery - notifications - systray the check can be done on those plugin names on appletquickitem line 52 so if the plugin name is one of them and the plugin version is less than some value bigger than all of them, go multiple engines and yeah, that check in ~6 months or so would be removed -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Monday 18 May 2015, Ivan Čukić wrote: > While I'm usually more conservative, I do think that something more > decisive should be done in this case (and cases like these). While we > do not want to break everything with each new release (wink wink ghm > ghm nudge nudge), I don't think we want to support every > not-still-preferred-approach-of-implementing-something we made before > indefinitely. a very crude approach may be maintaining a white list of name+version of the applets that needs this, just to make old workspace work for a while and eventually removing it > If there are important 3rd-party plasma 5 applets (are there?) that we > really need to keep back-compatibility for, I'd propose this: we should goover kdelook i gues, there should be 5-6 plasmoids there -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
> The ironic thing here is that something would have to be introduced > that will be deprecated Not really. It would deprecate the old qml engine per plasmoid. It is not about deprecating the opt-in thing. The ideal is to achieve 100% opt-in, that is, for all plugins to eventually get the SingleEngine requirement. Cheers, Ivan On 18 May 2015 at 22:51, Mark Gaiser wrote: > On Mon, May 18, 2015 at 9:53 PM, Marco Martin wrote: >> >> On Monday 18 May 2015, Mark Gaiser wrote: >> > Anyway, with that attribute you let the applet developer OPT-IN in for >> > shared engine. Setting that attribute can be easily forgotten. If >> > anything, >> > they should OPT-OUT thus by default use the shared engine. >> > Perhaps a attribute like this is more appropriate?: >> > X-Plasma-RequiresOwnQmlEngine=true >> > >> > or something alike. >> > >> > I'd definitely go for OPT-OUT (defaults = use shared engine). >> >> no, because the key here is retrocompatibility, old applets have to work >> as >> is. >> it's true that this makes it error prone, but that's the ugly need :/ >> otherwise there wouldn't be any reason for supporting both modes > > > While that - on it's own - is a very nice goal, sometimes you just have new > developments that are clearly better and the way forward. This is one such > case. Sure, you want to keep compatibility, but you should also strongly > motivate those that develop applets to use the shared engine. > > Ivan's idea of deprecating it and clearly letting the user know might be a > good compromise here. The ironic thing here is that something would have to > be introduced that will be deprecated from the beginning. Something sounds > wrong about that :) > > ___ > Plasma-devel mailing list > Plasma-devel@kde.org > https://mail.kde.org/mailman/listinfo/plasma-devel > -- Cheerio, Ivan -- While you were hanging yourself on someone else's words Dying to believe in what you heard I was staring straight into the shining sun ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Mon, May 18, 2015 at 9:53 PM, Marco Martin wrote: > On Monday 18 May 2015, Mark Gaiser wrote: > > Anyway, with that attribute you let the applet developer OPT-IN in for > > shared engine. Setting that attribute can be easily forgotten. If > anything, > > they should OPT-OUT thus by default use the shared engine. > > Perhaps a attribute like this is more appropriate?: > > X-Plasma-RequiresOwnQmlEngine=true > > > > or something alike. > > > > I'd definitely go for OPT-OUT (defaults = use shared engine). > > no, because the key here is retrocompatibility, old applets have to work as > is. > it's true that this makes it error prone, but that's the ugly need :/ > otherwise there wouldn't be any reason for supporting both modes While that - on it's own - is a very nice goal, sometimes you just have new developments that are clearly better and the way forward. This is one such case. Sure, you want to keep compatibility, but you should also strongly motivate those that develop applets to use the shared engine. Ivan's idea of deprecating it and clearly letting the user know might be a good compromise here. The ironic thing here is that something would have to be introduced that will be deprecated from the beginning. Something sounds wrong about that :) ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Monday 18 May 2015, Kai Uwe Broulik wrote: > What I found interesting that you changed import "plasmapkg:/code/logic.js" > to import "logic.js" although the qml file is in a different folder, why > does that work? since it can still figure out the package, the url interceptor takes js files from code/ and images from images/ on that the behavior didn't change, it was already rewriting paths like that -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
Hi, > That's running: > kdeclarative - your branch > plasma-framework - your branch > plsama-workspace - master > (which is pretty close to running latest frameworks with Plasma 5.3) Are there any significant changes in plasma-workspace and plasma-desktop, other than applet migration, since there's also a branch for them? Looking pretty nice though! Memory consumption for a freshly started Plasma session (no applets expanded so far) went from 215MiB to 145MiB on my laptop, startup also feels a little bit faster. What I found interesting that you changed import "plasmapkg:/code/logic.js" to import "logic.js" although the qml file is in a different folder, why does that work? > Being a massively miserable grumpy git, I'd rather merge just after 5.11, > giving us 4 weeks of testing. +1 for that Cheers, Kai Uwe ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
While I'm usually more conservative, I do think that something more decisive should be done in this case (and cases like these). While we do not want to break everything with each new release (wink wink ghm ghm nudge nudge), I don't think we want to support every not-still-preferred-approach-of-implementing-something we made before indefinitely. If there are important 3rd-party plasma 5 applets (are there?) that we really need to keep back-compatibility for, I'd propose this: - make it opt-in (as Marco says) - deprecate it - report notifications to the user 'this applet might make your desktop unstable' for all used applets that haven't opted-in - this would serve as a notification both to the users and the developers of said applets - schedule marking the support for these applets for the release after next, or something. Cheers, Ivan On 18 May 2015 at 21:53, Marco Martin wrote: > On Monday 18 May 2015, Mark Gaiser wrote: >> Anyway, with that attribute you let the applet developer OPT-IN in for >> shared engine. Setting that attribute can be easily forgotten. If anything, >> they should OPT-OUT thus by default use the shared engine. >> Perhaps a attribute like this is more appropriate?: >> X-Plasma-RequiresOwnQmlEngine=true >> >> or something alike. >> >> I'd definitely go for OPT-OUT (defaults = use shared engine). > > no, because the key here is retrocompatibility, old applets have to work as > is. > it's true that this makes it error prone, but that's the ugly need :/ > otherwise there wouldn't be any reason for supporting both modes > > -- > Marco Martin > ___ > Plasma-devel mailing list > Plasma-devel@kde.org > https://mail.kde.org/mailman/listinfo/plasma-devel -- Cheerio, Ivan -- While you were hanging yourself on someone else's words Dying to believe in what you heard I was staring straight into the shining sun ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Mon, May 18, 2015 at 8:28 PM, Marco Martin wrote: > Hi all, > > I think the branches for the single shared qqmlengine are pretty much ready > (few cleanups upcoming days), seems pretty stable here.. did someone ran > the > branch for a while as well? Thanks for doing all that. Now to make you hate me. I have a crash, https://paste.kde.org/ppeqjl1c1 That's running: kdeclarative - your branch plasma-framework - your branch plsama-workspace - master (which is pretty close to running latest frameworks with Plasma 5.3) It's possibly unrelated, but I switched the top two back to master and it went away. Other than that, I think code-wise from me it's nearly good to go. Can I check it's these https://git.reviewboard.kde.org/r/123736/ and https://git.reviewboard.kde.org/r/123735/ and the branches in p-w, p-d, are just changing the metadata? in plasma-workspace TaskDelegate.qml has an import line removed, is that intended? In the end i managed to get a single engine for the whole session, views > included (had to duplicate the View class in plasmaquick and kept the old > one > as deprecated for retrocompatibility unfortunately) > > the memory save seems pretty good, i *hope* stability improved as well :p > > what still uses separed engines are the applet configuration dialogs: this > is > necessary because they are supposed to use a different style for the > qquickcontrols, and being the settings object that decides the style a qml > singleton (qml singletons are unique by engine), the engine has to be > different from the desktop/panel. The good thing is that config dialogs are > instantiated relatively rarely, in most sessions not at all, so shouldn't > touch too much a "normal run" > > Lets not worry about that for now, we've got from super many -> just a few. Better to get this stuff merged, than worry about getting it down to 1. For retrocompatibility the applets unfortunately have to specify explicitly > they can share the engine in their metadata file (or eventual > plasmapackage:/ > urls break) > > at the moment it's using > X-Plasma-RequiredExtensions=SharedEngine > > but I'm leaning more on the direction of something like > X-Plasma-MinimumAPIVersion=5.11 > Out of curiosity, which plasmoids actually need their own engine? Were they all changed as a bulk find & replace? > I would like to have all set before frameworks 5.11 > So that gives us roughly 2 weeks of testing. Without the plasma-workspace changes coming in 5.4 it doesn't really have any benefit to the user. Being a massively miserable grumpy git, I'd rather merge just after 5.11, giving us 4 weeks of testing. > thoughts? > > -- > Marco Martin > ___ > Plasma-devel mailing list > Plasma-devel@kde.org > https://mail.kde.org/mailman/listinfo/plasma-devel > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Monday 18 May 2015, Mark Gaiser wrote: > Anyway, with that attribute you let the applet developer OPT-IN in for > shared engine. Setting that attribute can be easily forgotten. If anything, > they should OPT-OUT thus by default use the shared engine. > Perhaps a attribute like this is more appropriate?: > X-Plasma-RequiresOwnQmlEngine=true > > or something alike. > > I'd definitely go for OPT-OUT (defaults = use shared engine). no, because the key here is retrocompatibility, old applets have to work as is. it's true that this makes it error prone, but that's the ugly need :/ otherwise there wouldn't be any reason for supporting both modes -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Planning merging the single qqml engine
On Mon, May 18, 2015 at 9:28 PM, Marco Martin wrote: > Hi all, > > I think the branches for the single shared qqmlengine are pretty much ready > (few cleanups upcoming days), seems pretty stable here.. did someone ran > the > branch for a while as well? > > In the end i managed to get a single engine for the whole session, views > included (had to duplicate the View class in plasmaquick and kept the old > one > as deprecated for retrocompatibility unfortunately) > > the memory save seems pretty good, i *hope* stability improved as well :p > > what still uses separed engines are the applet configuration dialogs: this > is > necessary because they are supposed to use a different style for the > qquickcontrols, and being the settings object that decides the style a qml > singleton (qml singletons are unique by engine), the engine has to be > different from the desktop/panel. The good thing is that config dialogs are > instantiated relatively rarely, in most sessions not at all, so shouldn't > touch too much a "normal run" > > For retrocompatibility the applets unfortunately have to specify explicitly > they can share the engine in their metadata file (or eventual > plasmapackage:/ > urls break) > > at the moment it's using > X-Plasma-RequiredExtensions=SharedEngine > What does "RequiredExtensions" mean anyway? Is that a new attribute where you intent to add more "extensions" or does it already exist? It's name sounds slightly confusing to me. How can a SharedEngine be a "extension"? Anyway, with that attribute you let the applet developer OPT-IN in for shared engine. Setting that attribute can be easily forgotten. If anything, they should OPT-OUT thus by default use the shared engine. Perhaps a attribute like this is more appropriate?: X-Plasma-RequiresOwnQmlEngine=true or something alike. I'd definitely go for OPT-OUT (defaults = use shared engine). > but I'm leaning more on the direction of something like > X-Plasma-MinimumAPIVersion=5.11 > > I would like to have all set before frameworks 5.11 > > thoughts? > > -- > Marco Martin > ___ > Plasma-devel mailing list > Plasma-devel@kde.org > https://mail.kde.org/mailman/listinfo/plasma-devel > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Planning merging the single qqml engine
Hi all, I think the branches for the single shared qqmlengine are pretty much ready (few cleanups upcoming days), seems pretty stable here.. did someone ran the branch for a while as well? In the end i managed to get a single engine for the whole session, views included (had to duplicate the View class in plasmaquick and kept the old one as deprecated for retrocompatibility unfortunately) the memory save seems pretty good, i *hope* stability improved as well :p what still uses separed engines are the applet configuration dialogs: this is necessary because they are supposed to use a different style for the qquickcontrols, and being the settings object that decides the style a qml singleton (qml singletons are unique by engine), the engine has to be different from the desktop/panel. The good thing is that config dialogs are instantiated relatively rarely, in most sessions not at all, so shouldn't touch too much a "normal run" For retrocompatibility the applets unfortunately have to specify explicitly they can share the engine in their metadata file (or eventual plasmapackage:/ urls break) at the moment it's using X-Plasma-RequiredExtensions=SharedEngine but I'm leaning more on the direction of something like X-Plasma-MinimumAPIVersion=5.11 I would like to have all set before frameworks 5.11 thoughts? -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel