Re: Planning merging the single qqml engine

2015-05-21 Thread David Edmundson
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

2015-05-19 Thread Marco Martin
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

2015-05-19 Thread David Edmundson
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

2015-05-19 Thread Marco Martin
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

2015-05-19 Thread Marco Martin
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

2015-05-19 Thread Marco Martin
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

2015-05-19 Thread Marco Martin
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

2015-05-18 Thread Ivan Čukić
> 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

2015-05-18 Thread Mark Gaiser
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

2015-05-18 Thread Marco Martin
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

2015-05-18 Thread Kai Uwe Broulik
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

2015-05-18 Thread Ivan Čukić
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

2015-05-18 Thread David Edmundson
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

2015-05-18 Thread Marco Martin
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

2015-05-18 Thread Mark Gaiser
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

2015-05-18 Thread Marco Martin
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