[AppletQuickItem] Only set QtQuick Controls 1 style once per engine
ClosedPublic

Authored by broulik on Jan 31 2017, 10:37 AM.

Details

Summary

The style is global per engine, there's no need to set it for every applet created as the engines are shared.
Since this entire thing is just a hack, just setting a dynamic property to identify that we've set a style is valid imho.
Also turn it into a plain QtObject since we don't need a fully-fledged Item.

Test Plan

Verified that a QtQuick Controls 1 Button still uses the Plasma theme.

Saves like 16 ms for me.

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik updated this revision to Diff 10755.Jan 31 2017, 10:37 AM
broulik retitled this revision from to RFC: [AppletQuickItem] Cache QQmlComponent used for settings QtQuick Controls 1 style.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added a reviewer: Plasma.
broulik set the repository for this revision to R242 Plasma Framework (Library).
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptJan 31 2017, 10:37 AM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript

Personally I would just kill the whole thing.

  • All Plasma code all uses Plasma Components not QQC so this has zero effect. It was for an idea that didn't really materialise
  • It gives an obscure ASAN warning on freeing "o", that I don't know how to fix
  • This isn't going to work with QQC2
src/plasmaquick/appletquickitem.cpp
496

if you were to do this:
just do c = new QQmlComponent(qApp);

and you have the cleanup done for you.

mart added a subscriber: mart.Feb 1 2017, 4:58 PM

Personally I would just kill the whole thing.

  • All Plasma code all uses Plasma Components not QQC so this has zero effect. It was for an idea that didn't really materialise

actually it did materialise, and has nothing to do with plasmacomponents:

  • if you use normal QQC1 in a plasmoid, it will have the plasma theme, so it is supported (and you can't be sure it is actually not used, especially in stuff from the store)
  • if you use the same component in a config dialog, it will use the QStyle theme

then.. this thing cannot work anymore for qqc2 as their theming approach is completely different, but is a future problem.

broulik abandoned this revision.Feb 21 2017, 4:31 PM
broulik updated this revision to Diff 23315.Dec 3 2017, 10:13 AM
broulik retitled this revision from RFC: [AppletQuickItem] Cache QQmlComponent used for settings QtQuick Controls 1 style to [AppletQuickItem] Only set QtQuick Controls 1 style once per engine.
broulik edited the summary of this revision. (Show Details)
broulik edited the test plan for this revision. (Show Details)
broulik edited the summary of this revision. (Show Details)

Ping. (Stupid Phab doesn't move this thing to the top in search result because it'd ooooold)

davidedmundson accepted this revision.Dec 5 2017, 10:32 PM
This revision is now accepted and ready to land.Dec 5 2017, 10:32 PM
This revision was automatically updated to reflect the committed changes.