Jenkins build is back to normal : plasma-desktop_master_qt5 #488

[KDE CI System]

See 

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/
---

(Updated July 24, 2014, 6:17 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

this makes Button inherit from the QtControl and annd an accompaining 
plasma-looking theme

now, the ugly part of the patch:
iconSource is an url, so it screws up passing freedesktop compatible names (it 
expects names of pngs local to the qml file directory, testimony of the main 
platform target for controls actually being android/ios). one solution may be 
overriding iconSource as a normal string, but i would like the theme working 
also on a plain Button, so it extract only the filename from the url.


Diffs (updated)
-

  examples/applets/widgetgallery/contents/ui/Buttons.qml 9134df9 
  src/declarativeimports/core/iconitem.cpp 38012cc 
  src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
  src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119455/diff/


Testing
---

in both a normal plasma session or the widget gallery buttons work fine, 
painting is 100% identical


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Marco Martin]

> On July 24, 2014, 4:56 p.m., David Edmundson wrote:
> > src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml, line 83
> > 
> >
> > style.labelImplicitWidth doesn't exist?

gah, leftover


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63091
---


On July 24, 2014, 4:31 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 4:31 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   examples/applets/widgetgallery/contents/ui/Buttons.qml 9134df9 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Marco Martin]

> On July 24, 2014, 4:56 p.m., David Edmundson wrote:
> > src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml, line 41
> > 
> >
> > This isn't very declarative, is there a reason we can't do:
> > 
> > 
> > Button
> > {
> >   style: ButtonStyle{}
> >   property alias minimumWidth: style.minimumWidth 
> > }
> > 
> > and in this file
> > 
> > property alias minimumWidth: buttonContent.minimumWidth
> > 
> > same for height

as far i know, the only way to access the style is with like minimumWidth: 
__style.minimumWidth

was to avoid the private property since the __
if we are sure __style is not going to be removed, sine, but i would be not so 
sure of that


> On July 24, 2014, 4:56 p.m., David Edmundson wrote:
> > src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml, line 58
> > 
> >
> > if you want fd-o icons, generally you use iconName instead of 
> > iconSource.
> > 
> > We may need some changes for source compatibility, but that should be 
> > in the Button rather than the style I think.

I'll have to override iconsource to be an alias of iconname then, since all the 
users use only iconSource


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63091
---


On July 24, 2014, 4:31 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 4:31 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   examples/applets/widgetgallery/contents/ui/Buttons.qml 9134df9 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[David Edmundson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63091
---



src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml


If you change Row to RowLayout this could be worked out for you.
Layouts also have attached LayoutProperties on them. IMHO this makes the 
width calculation in the Button super easier too (just Layout.fillWidth: true)



src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml


This isn't very declarative, is there a reason we can't do:

Button
{
  style: ButtonStyle{}
  property alias minimumWidth: style.minimumWidth 
}

and in this file

property alias minimumWidth: buttonContent.minimumWidth

same for height



src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml


if you want fd-o icons, generally you use iconName instead of iconSource.

We may need some changes for source compatibility, but that should be in 
the Button rather than the style I think.



src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml


For the style I think we just want to do:

source: control.iconName || control.iconSource

as I think IconItem internally can handle both and choose the right thing



src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml


style.labelImplicitWidth doesn't exist?


- David Edmundson


On July 24, 2014, 4:31 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 4:31 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   examples/applets/widgetgallery/contents/ui/Buttons.qml 9134df9 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/
---

(Updated July 24, 2014, 4:31 p.m.)


Review request for KDE Frameworks and Plasma.


Changes
---

better rendering for the menu arrow, support checked buttons


Repository: plasma-framework


Description
---

this makes Button inherit from the QtControl and annd an accompaining 
plasma-looking theme

now, the ugly part of the patch:
iconSource is an url, so it screws up passing freedesktop compatible names (it 
expects names of pngs local to the qml file directory, testimony of the main 
platform target for controls actually being android/ios). one solution may be 
overriding iconSource as a normal string, but i would like the theme working 
also on a plain Button, so it extract only the filename from the url.


Diffs (updated)
-

  examples/applets/widgetgallery/contents/ui/Buttons.qml 9134df9 
  src/declarativeimports/core/iconitem.cpp 38012cc 
  src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
  src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119455/diff/


Testing
---

in both a normal plasma session or the widget gallery buttons work fine, 
painting is 100% identical


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Marco Martin]

> On July 24, 2014, 4:20 p.m., Martin Gräßlin wrote:
> > Concerning the icon names: for the Desktops Effects KCM the names are 
> > working without me doing anything. So there must be a solution hidden in 
> > the widgets emulating style.

yes, the native style figures out that internally, creates an internal qaction, 
and then uses the icon of the qaction.
it has a couple of drawbacks tough, it's internal api, so may break anytime, 
and returns a qicon, while i actually need the name, to use the icon from the 
plasma theme when available


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63087
---


On July 24, 2014, 3:55 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Marco Martin]

> On July 24, 2014, 4:18 p.m., Andrew Lake wrote:
> > Exciting! 
> > 
> > Might this eventually support a ButtonStyle.qml in the plasma theme that 
> > overrides the svg-based ButtonStyle if it's present? That way we could take 
> > the already developed Breeze ButtonStyle.qml and just ship it in the plasma 
> > theme.

perhaps.
that would be a step after when the set is more or less complete tough
one thing that makes me a bit hesitant tough is how "too much" logic still 
needs to be in the styles, very easy to break stuff


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63086
---


On July 24, 2014, 3:55 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63087
---


Concerning the icon names: for the Desktops Effects KCM the names are working 
without me doing anything. So there must be a solution hidden in the widgets 
emulating style.

- Martin Gräßlin


On July 24, 2014, 5:55 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 5:55 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119455: make Button a QtControl

[Andrew Lake]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/#review63086
---


Exciting! 

Might this eventually support a ButtonStyle.qml in the plasma theme that 
overrides the svg-based ButtonStyle if it's present? That way we could take the 
already developed Breeze ButtonStyle.qml and just ship it in the plasma theme.

- Andrew Lake


On July 24, 2014, 3:55 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119455/
> ---
> 
> (Updated July 24, 2014, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> this makes Button inherit from the QtControl and annd an accompaining 
> plasma-looking theme
> 
> now, the ugly part of the patch:
> iconSource is an url, so it screws up passing freedesktop compatible names 
> (it expects names of pngs local to the qml file directory, testimony of the 
> main platform target for controls actually being android/ios). one solution 
> may be overriding iconSource as a normal string, but i would like the theme 
> working also on a plain Button, so it extract only the filename from the url.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
>   src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
> PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119455/diff/
> 
> 
> Testing
> ---
> 
> in both a normal plasma session or the widget gallery buttons work fine, 
> painting is 100% identical
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 119455: make Button a QtControl

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119455/
---

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

this makes Button inherit from the QtControl and annd an accompaining 
plasma-looking theme

now, the ugly part of the patch:
iconSource is an url, so it screws up passing freedesktop compatible names (it 
expects names of pngs local to the qml file directory, testimony of the main 
platform target for controls actually being android/ios). one solution may be 
overriding iconSource as a normal string, but i would like the theme working 
also on a plain Button, so it extract only the filename from the url.


Diffs
-

  src/declarativeimports/core/iconitem.cpp 38012cc 
  src/declarativeimports/plasmacomponents/qml/Button.qml 262e20f 
  src/declarativeimports/plasmacomponents/qml/styles/ButtonStyle.qml 
PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119455/diff/


Testing
---

in both a normal plasma session or the widget gallery buttons work fine, 
painting is 100% identical


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Build failed in Jenkins: plasma-desktop_master_qt5 #487

[KDE CI System]

See 

Changes:

[kensington] Fix header include to match plasma-workspace change.

--
[...truncated 2488 lines...]
:1031:69:
 warning: ‘KUrl’ is deprecated [-Wdeprecated-declarations]
 QPair KonqOperations::pasteInfo(const KUrl& targetUrl)
 ^
:
 In static member function ‘static QPair 
KonqOperations::pasteInfo(const KUrl&)’:
:1038:16:
 warning: ‘List’ is deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kurl.h:143)
 [-Wdeprecated-declarations]
 KUrl::List urls = KUrl::List::fromMimeData(mimeData);
^
:1038:56:
 warning: ‘static KUrl::List KUrl::List::fromMimeData(const QMimeData*, 
KUrl::List::DecodeOptions, KUrl::MetaDataMap*)’ is deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kurl.h:317)
 [-Wdeprecated-declarations]
 KUrl::List urls = KUrl::List::fromMimeData(mimeData);
^
:1041:73:
 warning: ‘KFileItem::KFileItem(mode_t, mode_t, const QUrl&, bool)’ is 
deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kio/inst/include/KF5/KIOCore/kfileitem.h:104)
 [-Wdeprecated-declarations]
 KFileItem item(KFileItem::Unknown, KFileItem::Unknown, targetUrl);
 ^
:1045:92:
 warning: ‘KFileItem::KFileItem(mode_t, mode_t, const QUrl&, bool)’ is 
deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kio/inst/include/KF5/KIOCore/kfileitem.h:104)
 [-Wdeprecated-declarations]
 const KFileItem item(KFileItem::Unknown, KFileItem::Unknown, 
urls.first(), true);

^
In file included from 
:25:0,
 from 
:23,
 from 
:9,
 from 
:2:
:36:18:
 warning: ‘KPushButton’ is deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kpushbutton.h:47)
 [-Wdeprecated-declarations]
 KPushButton *installKnsButton;
  ^
:37:18:
 warning: ‘KPushButton’ is deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kpushbutton.h:47)
 [-Wdeprecated-declarations]
 KPushButton *installButton;
  ^
:38:18:
 warning: ‘KPushButton’ is deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kpushbutton.h:47)
 [-Wdeprecated-declarations]
 KPushButton *removeButton;
  ^
:
 In member function ‘void Ui_ThemePage::setupUi(QWidget*)’:
:77:32:
 warning: ‘KPushButton’ is deprecated (declared at 
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kdelibs4support/inst/include/KF5/KDELibs4Support/kpushbutton.h:47)
 [-Wdeprecated-declarations]
 installKnsButton = new KPushButton(ThemePage);
^
:82:29:
 warning: ‘KPushButton’ is deprecated (declared at 
/srv/jenkins/install

Re: Review Request 119452: Rename libkworkspace for coinstallability with kde-workspace4.

[Michael Palimaka]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119452/
---

(Updated July 24, 2014, 3:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

A number of KDE 4 applications depend on libkworkspace from kde-workspace, 
which currently collides with libkworkspace provided by plasma-workspace.

Renaming makes it easier downstream to provide the ability run these 
applications in a Plasma 5 session.


Diffs
-

  libkworkspace/CMakeLists.txt e417c76ad4a032e6dc2fde9aae74352303821983 

Diff: https://git.reviewboard.kde.org/r/119452/diff/


Testing
---

plasma-workspace, khotkeys, and powerdevil builds. plasma-desktop requires a 
matching header include rename. Not aware of any other frameworks-based 
libkworkspace consumers.


Thanks,

Michael Palimaka

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Aleix Pol Gonzalez]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.
> 
> Marco Martin wrote:
> uhm, why a filter on palette change?
> this would work for themes that use system colors but not the other ones.
> theme::themechanged should cover both cases (and if it doesn't, it should)
> 
> Aleix Pol Gonzalez wrote:
> See SvgPrivate::checkColorHints().
> 
> I agree it could make sense having it in plasma theme, but this should be 
> part of another patch.
> 
> Marco Martin wrote:
> it does a repaintneeded, that is correct.
> but yeah, having it in theme doesn't make much sense, because technically 
> the theme didn't change and you can't know from there if one of the svgs 
> actually needs a repaint.
> 
> so, what all of this suggests me is that the thing that would make most 
> sense is actually use repaintneeded, and either:
> a) just remove from the hash the id of this texture (multiple removes 
> still possible tough)
> b) event compress the clear()
> c) both of the above
> 
> Aleix Pol Gonzalez wrote:
> I'm getting a bit lost, sorry. Why is listening to all svg's better? 
> RepaintNeeded will emit upon signals we don't need. Note that themeChanged 
> doesn't clean the cache and it will rather be the textures just stopping to 
> use the old theme, because the hash is refcounted. That's why I introduced 
> more elements to the hash key.
> 
> David Edmundson wrote:
> Calling clear on an empty hash is an atomic operation. Maybe it should 
> just go back to the version in revision 1.
> It's much simpler and probably much faster than all this extra string 
> appending and monitoring.
> 
> Marco Martin wrote:
> yes. If we can assure that the following scenario will never happen, 
> better version number one
> scenario as in
> * clear
> * put something back in
> * clear again
> * repeat for every single instance

I think we can ensure that, because the painting is carried out in a different 
thread, so it will end up in the next frame painting.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 11:14 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Marco Martin]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.
> 
> Marco Martin wrote:
> uhm, why a filter on palette change?
> this would work for themes that use system colors but not the other ones.
> theme::themechanged should cover both cases (and if it doesn't, it should)
> 
> Aleix Pol Gonzalez wrote:
> See SvgPrivate::checkColorHints().
> 
> I agree it could make sense having it in plasma theme, but this should be 
> part of another patch.
> 
> Marco Martin wrote:
> it does a repaintneeded, that is correct.
> but yeah, having it in theme doesn't make much sense, because technically 
> the theme didn't change and you can't know from there if one of the svgs 
> actually needs a repaint.
> 
> so, what all of this suggests me is that the thing that would make most 
> sense is actually use repaintneeded, and either:
> a) just remove from the hash the id of this texture (multiple removes 
> still possible tough)
> b) event compress the clear()
> c) both of the above
> 
> Aleix Pol Gonzalez wrote:
> I'm getting a bit lost, sorry. Why is listening to all svg's better? 
> RepaintNeeded will emit upon signals we don't need. Note that themeChanged 
> doesn't clean the cache and it will rather be the textures just stopping to 
> use the old theme, because the hash is refcounted. That's why I introduced 
> more elements to the hash key.
> 
> David Edmundson wrote:
> Calling clear on an empty hash is an atomic operation. Maybe it should 
> just go back to the version in revision 1.
> It's much simpler and probably much faster than all this extra string 
> appending and monitoring.

yes. If we can assure that the following scenario will never happen, better 
version number one
scenario as in
* clear
* put something back in
* clear again
* repeat for every single instance


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 11:14 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[David Edmundson]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.
> 
> Marco Martin wrote:
> uhm, why a filter on palette change?
> this would work for themes that use system colors but not the other ones.
> theme::themechanged should cover both cases (and if it doesn't, it should)
> 
> Aleix Pol Gonzalez wrote:
> See SvgPrivate::checkColorHints().
> 
> I agree it could make sense having it in plasma theme, but this should be 
> part of another patch.
> 
> Marco Martin wrote:
> it does a repaintneeded, that is correct.
> but yeah, having it in theme doesn't make much sense, because technically 
> the theme didn't change and you can't know from there if one of the svgs 
> actually needs a repaint.
> 
> so, what all of this suggests me is that the thing that would make most 
> sense is actually use repaintneeded, and either:
> a) just remove from the hash the id of this texture (multiple removes 
> still possible tough)
> b) event compress the clear()
> c) both of the above
> 
> Aleix Pol Gonzalez wrote:
> I'm getting a bit lost, sorry. Why is listening to all svg's better? 
> RepaintNeeded will emit upon signals we don't need. Note that themeChanged 
> doesn't clean the cache and it will rather be the textures just stopping to 
> use the old theme, because the hash is refcounted. That's why I introduced 
> more elements to the hash key.

Calling clear on an empty hash is an atomic operation. Maybe it should just go 
back to the version in revision 1.
It's much simpler and probably much faster than all this extra string appending 
and monitoring.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 11:14 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Aleix Pol Gonzalez]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.
> 
> Marco Martin wrote:
> uhm, why a filter on palette change?
> this would work for themes that use system colors but not the other ones.
> theme::themechanged should cover both cases (and if it doesn't, it should)
> 
> Aleix Pol Gonzalez wrote:
> See SvgPrivate::checkColorHints().
> 
> I agree it could make sense having it in plasma theme, but this should be 
> part of another patch.
> 
> Marco Martin wrote:
> it does a repaintneeded, that is correct.
> but yeah, having it in theme doesn't make much sense, because technically 
> the theme didn't change and you can't know from there if one of the svgs 
> actually needs a repaint.
> 
> so, what all of this suggests me is that the thing that would make most 
> sense is actually use repaintneeded, and either:
> a) just remove from the hash the id of this texture (multiple removes 
> still possible tough)
> b) event compress the clear()
> c) both of the above

I'm getting a bit lost, sorry. Why is listening to all svg's better? 
RepaintNeeded will emit upon signals we don't need. Note that themeChanged 
doesn't clean the cache and it will rather be the textures just stopping to use 
the old theme, because the hash is refcounted. That's why I introduced more 
elements to the hash key.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 11:14 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Marco Martin]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.
> 
> Marco Martin wrote:
> uhm, why a filter on palette change?
> this would work for themes that use system colors but not the other ones.
> theme::themechanged should cover both cases (and if it doesn't, it should)
> 
> Aleix Pol Gonzalez wrote:
> See SvgPrivate::checkColorHints().
> 
> I agree it could make sense having it in plasma theme, but this should be 
> part of another patch.

it does a repaintneeded, that is correct.
but yeah, having it in theme doesn't make much sense, because technically the 
theme didn't change and you can't know from there if one of the svgs actually 
needs a repaint.

so, what all of this suggests me is that the thing that would make most sense 
is actually use repaintneeded, and either:
a) just remove from the hash the id of this texture (multiple removes still 
possible tough)
b) event compress the clear()
c) both of the above


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 11:14 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[Martin Gräßlin]

> On July 24, 2014, 12:34 p.m., David Edmundson wrote:
> > lookandfeelaccess/lookandfeelaccess.cpp, line 89
> > 
> >
> > My concern is this will create more problems than it's worth.
> > 
> > Scenario:
> >  - I add a new feature in the lock screen in 5.1 with a new rootContext 
> > variable
> >  - I update the QML to use this new feature in 5.1
> >  - a user upgrades his system
> >  - We then reload the package (but not the binary) we get a QML error 
> > and the user can't log in again.
> 
> Marco Martin wrote:
> so do you think some more complicated things like the lockscreen souldn't 
> be themeable? may make sense on a maintainance standpoint, but yeah, needs 
> definition of what should be in there, what shouldn't
> 
> David Edmundson wrote:
> It probably should be themeable. My concern was changing files for an 
> application whilst it's running.
> 
> But after I think about it more, that would happen anyway.
> All it would take is to have a Loader in a release that opens a file that 
> gets renamed/deleted in an upgraded version and you'll have the same 
> problems. It probably is best to notify and update the package so the app at 
> least has a chance to handle it.
> 
> I withdraw my comment.
> 
> Marco Martin wrote:
> anyways, the class is just sending a signal, if the application has 
> particular problems in reloading at runtime, it would just ignore the signal.
> btw at the moment it's signaling when the theme gets changed, but not 
> when the files of the package gets upgraded (that looks like a separate can 
> of worms...)

David, I think your fear is without need in this case as the greeter is not a 
long running process, but one that gets freshly started each time the screen 
gets locked.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---


On July 24, 2014, 11:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 11:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/
---

(Updated July 24, 2014, 11:14 a.m.)


Review request for Plasma.


Changes
---

Pleased David.


Repository: plasma-framework


Description
---

Create a cache that has pointers to all the textures that we've generated, so 
in case we have one already created, we can re-use it.


Diffs (updated)
-

  src/declarativeimports/core/framesvgitem.cpp 323b06b 
  src/declarativeimports/core/iconitem.cpp 38012cc 
  src/declarativeimports/core/svgitem.cpp eccff55 
  src/declarativeimports/core/svgtexturenode.h 21b9b2f 

Diff: https://git.reviewboard.kde.org/r/119425/diff/


Testing
---

see the qDebug (to be removed before commit). 

plasmashell 2> out
$ grep s_cache out | grep ": miss" | wc -l
342
$ grep s_cache out | grep ": hit" | wc -l
126

So still having 3 times more hits than miss, so there's big room for 
improvement. Good news is that with this, we get a ~25% of memory and bandwidth 
save, in a per-item basis.


Thanks,

Aleix Pol Gonzalez

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Aleix Pol Gonzalez]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.
> 
> Marco Martin wrote:
> uhm, why a filter on palette change?
> this would work for themes that use system colors but not the other ones.
> theme::themechanged should cover both cases (and if it doesn't, it should)

See SvgPrivate::checkColorHints().

I agree it could make sense having it in plasma theme, but this should be part 
of another patch.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 12:44 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[Marco Martin]

> On July 24, 2014, 10:55 a.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 45
> > 
> >
> > Having spoken to you offline this does make sense, but it needs 
> > documentation as to why it's doing this.

uhm, why a filter on palette change?
this would work for themes that use system colors but not the other ones.
theme::themechanged should cover both cases (and if it doesn't, it should)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---


On July 24, 2014, 12:44 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[Marco Martin]

> On July 24, 2014, 10:34 a.m., David Edmundson wrote:
> > lookandfeelaccess/lookandfeelaccess.cpp, line 89
> > 
> >
> > My concern is this will create more problems than it's worth.
> > 
> > Scenario:
> >  - I add a new feature in the lock screen in 5.1 with a new rootContext 
> > variable
> >  - I update the QML to use this new feature in 5.1
> >  - a user upgrades his system
> >  - We then reload the package (but not the binary) we get a QML error 
> > and the user can't log in again.
> 
> Marco Martin wrote:
> so do you think some more complicated things like the lockscreen souldn't 
> be themeable? may make sense on a maintainance standpoint, but yeah, needs 
> definition of what should be in there, what shouldn't
> 
> David Edmundson wrote:
> It probably should be themeable. My concern was changing files for an 
> application whilst it's running.
> 
> But after I think about it more, that would happen anyway.
> All it would take is to have a Loader in a release that opens a file that 
> gets renamed/deleted in an upgraded version and you'll have the same 
> problems. It probably is best to notify and update the package so the app at 
> least has a chance to handle it.
> 
> I withdraw my comment.

anyways, the class is just sending a signal, if the application has particular 
problems in reloading at runtime, it would just ignore the signal.
btw at the moment it's signaling when the theme gets changed, but not when the 
files of the package gets upgraded (that looks like a separate can of worms...)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119425: Cache the textures created for the fast path

[David Edmundson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review63049
---



src/declarativeimports/core/framesvgitem.cpp


Having spoken to you offline this does make sense, but it needs 
documentation as to why it's doing this.


- David Edmundson


On July 24, 2014, 12:44 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> ---
> 
> (Updated July 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Create a cache that has pointers to all the textures that we've generated, so 
> in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> ---
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for 
> improvement. Good news is that with this, we get a ~25% of memory and 
> bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[David Edmundson]

> On July 24, 2014, 10:34 a.m., David Edmundson wrote:
> > lookandfeelaccess/lookandfeelaccess.cpp, line 89
> > 
> >
> > My concern is this will create more problems than it's worth.
> > 
> > Scenario:
> >  - I add a new feature in the lock screen in 5.1 with a new rootContext 
> > variable
> >  - I update the QML to use this new feature in 5.1
> >  - a user upgrades his system
> >  - We then reload the package (but not the binary) we get a QML error 
> > and the user can't log in again.
> 
> Marco Martin wrote:
> so do you think some more complicated things like the lockscreen souldn't 
> be themeable? may make sense on a maintainance standpoint, but yeah, needs 
> definition of what should be in there, what shouldn't

It probably should be themeable. My concern was changing files for an 
application whilst it's running.

But after I think about it more, that would happen anyway.
All it would take is to have a Loader in a release that opens a file that gets 
renamed/deleted in an upgraded version and you'll have the same problems. It 
probably is best to notify and update the package so the app at least has a 
chance to handle it.

I withdraw my comment.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[Marco Martin]

On July 24, 2014, 10:34 a.m., Marco Martin wrote:
> > I'd quite like to have a chat about the goals of look and feel (next Monday 
> > meeting?). It was something where you guys clearly had a plan (or at least 
> > a name) before I got more heavily involved in Plasma, and I'm really not on 
> > the same page.

yes, we can do that monday


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[Marco Martin]

> On July 24, 2014, 10:34 a.m., David Edmundson wrote:
> > lookandfeelaccess/shellpluginloader.h, line 31
> > 
> >
> > not used?

that is just the header 1:1 coming from libplasmaquick, usual old problem


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[Marco Martin]

> On July 24, 2014, 10:34 a.m., David Edmundson wrote:
> > lookandfeelaccess/lookandfeelaccess.cpp, line 89
> > 
> >
> > My concern is this will create more problems than it's worth.
> > 
> > Scenario:
> >  - I add a new feature in the lock screen in 5.1 with a new rootContext 
> > variable
> >  - I update the QML to use this new feature in 5.1
> >  - a user upgrades his system
> >  - We then reload the package (but not the binary) we get a QML error 
> > and the user can't log in again.

so do you think some more complicated things like the lockscreen souldn't be 
themeable? may make sense on a maintainance standpoint, but yeah, needs 
definition of what should be in there, what shouldn't


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119452: Rename libkworkspace for coinstallability with kde-workspace4.

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119452/#review63043
---

Ship it!


fine for me

- Marco Martin


On Lug. 24, 2014, 10:19 a.m., Michael Palimaka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119452/
> ---
> 
> (Updated Lug. 24, 2014, 10:19 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> A number of KDE 4 applications depend on libkworkspace from kde-workspace, 
> which currently collides with libkworkspace provided by plasma-workspace.
> 
> Renaming makes it easier downstream to provide the ability run these 
> applications in a Plasma 5 session.
> 
> 
> Diffs
> -
> 
>   libkworkspace/CMakeLists.txt e417c76ad4a032e6dc2fde9aae74352303821983 
> 
> Diff: https://git.reviewboard.kde.org/r/119452/diff/
> 
> 
> Testing
> ---
> 
> plasma-workspace, khotkeys, and powerdevil builds. plasma-desktop requires a 
> matching header include rename. Not aware of any other frameworks-based 
> libkworkspace consumers.
> 
> 
> Thanks,
> 
> Michael Palimaka
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[David Edmundson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63037
---



lookandfeelaccess/lookandfeelaccess.cpp


My concern is this will create more problems than it's worth.

Scenario:
 - I add a new feature in the lock screen in 5.1 with a new rootContext 
variable
 - I update the QML to use this new feature in 5.1
 - a user upgrades his system
 - We then reload the package (but not the binary) we get a QML error and 
the user can't log in again.



lookandfeelaccess/shellpluginloader.h


not used?


I'd quite like to have a chat about the goals of look and feel (next Monday 
meeting?). It was something where you guys clearly had a plan (or at least a 
name) before I got more heavily involved in Plasma, and I'm really not on the 
same page.

- David Edmundson


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 119452: Rename libkworkspace for coinstallability with kde-workspace4.

[Michael Palimaka]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119452/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

A number of KDE 4 applications depend on libkworkspace from kde-workspace, 
which currently collides with libkworkspace provided by plasma-workspace.

Renaming makes it easier downstream to provide the ability run these 
applications in a Plasma 5 session.


Diffs
-

  libkworkspace/CMakeLists.txt e417c76ad4a032e6dc2fde9aae74352303821983 

Diff: https://git.reviewboard.kde.org/r/119452/diff/


Testing
---

plasma-workspace, khotkeys, and powerdevil builds. plasma-desktop requires a 
matching header include rename. Not aware of any other frameworks-based 
libkworkspace consumers.


Thanks,

Michael Palimaka

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119451: some machinery for look and feel package

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/#review63040
---


Also another problem connected to this:

how to take the default icons, default fonts, colors etc from values specified 
inside this?
those defaults should be put in a framework that can't really depend from this 
in any shape or form..

- Marco Martin


On July 24, 2014, 9:43 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119451/
> ---
> 
> (Updated July 24, 2014, 9:43 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This is still nowhere near final mergeability, but rather a request for 
> comments for early stages ;)
> 
> So, what does an application that uses stuff from l&f needs?
> * open the proper l&f package, as configured
> * fetch files from it
> * use files of the default one if the provided one is incomplete
> * monitor for theme changes and if necessary reload the qml
> * some applications may even want to have an optional local config that 
> overrides it, like the splash, but is out of scope of a central management
>  
> the branch uses a little class that does all of the above (minus the last 
> point) and uses it for now in the splash screen
> For now it would just be statically linked into users, since they should be 
> all in plasma-framework for now (of course not optimal)
> 
> *maybe* is stuff for libplasmaquick, but that library since locks a release 
> of p-f with the same release of users of it, should probably me made public 
> or else, so I'm a bit hesitant to add further stuff into it.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/CMakeLists.txt 16c58a0 
>   ksplash/ksplashqml/SplashWindow.cpp 23603f5 
>   ksplash/ksplashqml/shellpluginloader.h 9c0f624 
>   lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
>   lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
>   lookandfeelaccess/shellpluginloader.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119451/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 119451: some machinery for look and feel package

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119451/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

This is still nowhere near final mergeability, but rather a request for 
comments for early stages ;)

So, what does an application that uses stuff from l&f needs?
* open the proper l&f package, as configured
* fetch files from it
* use files of the default one if the provided one is incomplete
* monitor for theme changes and if necessary reload the qml
* some applications may even want to have an optional local config that 
overrides it, like the splash, but is out of scope of a central management
 
the branch uses a little class that does all of the above (minus the last 
point) and uses it for now in the splash screen
For now it would just be statically linked into users, since they should be all 
in plasma-framework for now (of course not optimal)

*maybe* is stuff for libplasmaquick, but that library since locks a release of 
p-f with the same release of users of it, should probably me made public or 
else, so I'm a bit hesitant to add further stuff into it.


Diffs
-

  ksplash/ksplashqml/CMakeLists.txt 16c58a0 
  ksplash/ksplashqml/SplashWindow.cpp 23603f5 
  ksplash/ksplashqml/shellpluginloader.h 9c0f624 
  lookandfeelaccess/lookandfeelaccess.h PRE-CREATION 
  lookandfeelaccess/lookandfeelaccess.cpp PRE-CREATION 
  lookandfeelaccess/shellpluginloader.h PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119451/diff/


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel