Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Xuetian Weng

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

(Updated Dec. 11, 2014, 3:55 a.m.)


Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Kai Uwe 
Broulik.


Changes
---

Let's just make this simpler first. Stop animation if the pixmap change is 
caused by the geometry change.


Repository: plasma-framework


Description
---

Making transition between two different size doesn't make much sense, since 
repainting is usually happens at that time and it could take some time to 
finish. And animation need to be stopped if m_animValue is set manually.


Diffs (updated)
-

  src/declarativeimports/core/iconitem.cpp 145a7cd 

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


Testing (updated)
---

Looks fine on tray icon and lock screen, less blurry transition.


Thanks,

Xuetian Weng

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


Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Xuetian Weng


> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote:
> > Wasn't that part of the idea? Having it scale up the pixmap first when 
> > resizing and then re-rendering it later?
> 
> Xuetian Weng wrote:
> 1. icon size (the widget size) doesn't change frequently. Usually it only 
> happens when user resize the UI or changes the settings.
> 2. the animation IMHO is for smooth transition between different 
> icons/different icon effect (check volume icon, media player play/pause, 
> hovering with opacity changes), I don't see we need to have animated 
> transition from a scaled icon to the actual correct size icon.
> 
> David Edmundson wrote:
> Icon size changes may be infrequent but they do happen. It's important 
> that when we resize the panel we don't re-render a tonne of SVGs constantly, 
> we need resizing applets to be smooth.
> 
> I don't understand where this blurriness is coming from; that implies 
> we're loading it at a wrong size then resizing; if that's the case, lets fix 
> that rather than hide it.
> 
> Sebastian Kügler wrote:
> During resize, we're rescaling a pixmap instead of re-rendering the SVG 
> for every frame. That can induce blur.
> 
> Also, rendering an SVG at random sizes van induce blur, since not 
> everything can be aligned to pixels all the time, our SVGs are optimized for 
> "standard sizes".
> 
> Xuetian Weng wrote:
> The problem is not loading it at wrong size. It is the widget is not 
> resized to correct size yet.
> The most significant one on plasma here is Device Notifier (When it's 
> popped up at the first time), I print out some debug message at 
> ::geometryChanged
> ICON QSizeF(1, 86) QSizeF(1, 1) QVariant(QString, "device-notifier")
> ICON QSizeF(129, 86) QSizeF(1, 86) QVariant(QString, "device-notifier")
> 
> As you can see, the size changed from 1x1 to 1x86 then to 129x86. So they 
> are valid size, and the blurriness is exposed by the transition animation.
> 
> David Edmundson wrote:
> Thanks. I can reproduce with the Device Notifier + debug
> 
> What I'm trying to argue is that it should go directly to 129x86. (or at 
> least be an invalid size till then)
> 
> Otherwise we're loading an SVG 3 times and not using two of them this 
> patch will hide the visual artifacts of that, but we'll still be wasting a 
> lot of CPU cycles doing something silly.

emm, seems the 1x1 size comes from lines 67 and 68: 
http://quickgit.kde.org/?p=plasma-workspace.git&a=blob&h=071705a85aee9ee00a3c88657c93d20544eb5c0f&hb=8b670015582e6b501a2a81a23f38eb5772a36e85&f=applets%2Fsystemtray%2Fplugin%2Fprotocols%2Fplasmoid%2Fplasmoidtask.cpp

from current code I don't see an easy way to avoid that... I think there's one 
resize event can be avoided by call setSize(), but anyway there need to one 
extra resize.


- Xuetian


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


On Dec. 9, 2014, 4:43 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121411/
> ---
> 
> (Updated Dec. 9, 2014, 4:43 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Kai Uwe 
> Broulik.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Making transition between two different size doesn't make much sense, since 
> repainting is usually happens at that time and it could take some time to 
> finish. And animation need to be stopped if m_animValue is set manually.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 
> ed3bb978f01cca98315fd425778139e4b9eeb64f 
> 
> Diff: https://git.reviewboard.kde.org/r/121411/diff/
> 
> 
> Testing
> ---
> 
> Looks fine on tray icon and lock screen, no blurry transition anymore.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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


Change in plasma-framework[master]: Use a shared config in tooltip

2014-12-10 Thread David Edmundson (Code Review)
David Edmundson has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/216

Change subject: Use a shared config in tooltip
..

Use a shared config in tooltip

Change-Id: I68521f9066114a265c3f3444332e5e67016207f7
---
M src/declarativeimports/core/tooltip.cpp
1 file changed, 2 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/plasma-framework 
refs/changes/16/216/1

diff --git a/src/declarativeimports/core/tooltip.cpp 
b/src/declarativeimports/core/tooltip.cpp
index 839b8b7..dc0b457 100644
--- a/src/declarativeimports/core/tooltip.cpp
+++ b/src/declarativeimports/core/tooltip.cpp
@@ -71,10 +71,8 @@
 
 void ToolTip::settingsChanged()
 {
-KConfig config("plasmarc");
-KConfigGroup cg(&config, "PlasmaToolTips");
-
-m_interval = cg.readEntry("Delay", 700);
+KConfigGroup cfg = KConfigGroup(KSharedConfig::openConfig("plasmarc"), 
"PlasmaToolTips");
+m_interval = cfg.readEntry("Delay", 700);
 m_tooltipsEnabledGlobally = (m_interval > 0);
 }
 

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/216
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I68521f9066114a265c3f3444332e5e67016207f7
Gerrit-PatchSet: 1
Gerrit-Project: plasma-framework
Gerrit-Branch: master
Gerrit-Owner: David Edmundson 
Gerrit-Reviewer: Marco Martin 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Change in plasma-framework[master]: Reparse settings when they change

2014-12-10 Thread David Edmundson (Code Review)
David Edmundson has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/217

Change subject: Reparse settings when they change
..

Reparse settings when they change

Change-Id: Ibb13077943330417fa08d23ce1ea60f2aff5defc
---
M src/declarativeimports/core/tooltip.cpp
M src/declarativeimports/core/tooltip.h
2 files changed, 7 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/plasma-framework 
refs/changes/17/217/1

diff --git a/src/declarativeimports/core/tooltip.cpp 
b/src/declarativeimports/core/tooltip.cpp
index dc0b457..2e19f0d 100644
--- a/src/declarativeimports/core/tooltip.cpp
+++ b/src/declarativeimports/core/tooltip.cpp
@@ -49,7 +49,7 @@
 m_showTimer->setSingleShot(true);
 connect(m_showTimer, &QTimer::timeout, this, &ToolTip::showToolTip);
 
-settingsChanged();
+loadSettings();
 
 const QString configFile = 
QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation) + 
QLatin1Char('/') + "plasmarc";
 KDirWatch::self()->addFile(configFile);
@@ -71,6 +71,11 @@
 
 void ToolTip::settingsChanged()
 {
+KSharedConfig::openConfig("plasmarc")->reparseConfiguration();
+}
+
+void ToolTip::loadSettings()
+{
 KConfigGroup cfg = KConfigGroup(KSharedConfig::openConfig("plasmarc"), 
"PlasmaToolTips");
 m_interval = cfg.readEntry("Delay", 700);
 m_tooltipsEnabledGlobally = (m_interval > 0);
diff --git a/src/declarativeimports/core/tooltip.h 
b/src/declarativeimports/core/tooltip.h
index 105bf38..f71bdd1 100644
--- a/src/declarativeimports/core/tooltip.h
+++ b/src/declarativeimports/core/tooltip.h
@@ -180,6 +180,7 @@
 void settingsChanged();
 
 private:
+void loadSettings();
 bool m_tooltipsEnabledGlobally;
 bool m_containsMouse;
 Plasma::Types::Location m_location;

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/217
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb13077943330417fa08d23ce1ea60f2aff5defc
Gerrit-PatchSet: 1
Gerrit-Project: plasma-framework
Gerrit-Branch: master
Gerrit-Owner: David Edmundson 
Gerrit-Reviewer: Marco Martin 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Change in plasma-framework[master]: Save SVGs only when a theme is unloaded, not on each theme p...

2014-12-10 Thread David Edmundson (Code Review)
David Edmundson has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/215

Change subject: Save SVGs only when a theme is unloaded, not on each theme proxy
..

Save SVGs only when a theme is unloaded, not on each theme proxy

A single Plasma theme will be represented by one ThemePrivate object,
which is exported in multiple Theme objects.

We want to save the cache when that theme stops being used, not each
time an item stops using the theme.

saveSvgElementsCache calls sync() which involves a lot of parsing and
IO. This makes everything a lot faster at no cost.

Change-Id: Ica6ba0273bc99fb8ad8733a1c90db8f1e87c49ea
---
M src/plasma/private/theme_p.cpp
M src/plasma/theme.cpp
2 files changed, 3 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/plasma-framework 
refs/changes/15/215/1

diff --git a/src/plasma/private/theme_p.cpp b/src/plasma/private/theme_p.cpp
index b9c2a55..1963f74 100644
--- a/src/plasma/private/theme_p.cpp
+++ b/src/plasma/private/theme_p.cpp
@@ -115,7 +115,8 @@
 
 ThemePrivate::~ThemePrivate()
 {
-if (FrameSvgPrivate::s_sharedFrames.contains(this)) { 
+saveSvgElementsCache();
+if (FrameSvgPrivate::s_sharedFrames.contains(this)) {
 foreach (FrameData *data, 
FrameSvgPrivate::s_sharedFrames[this].values()) {
 delete data;
 }
@@ -633,7 +634,7 @@
 break;
 }
 }
-
+
 
 
 switch (role) {
diff --git a/src/plasma/theme.cpp b/src/plasma/theme.cpp
index ba4e9e8..18d4ed6 100644
--- a/src/plasma/theme.cpp
+++ b/src/plasma/theme.cpp
@@ -91,8 +91,6 @@
 
 Theme::~Theme()
 {
-d->saveSvgElementsCache();
-
 if (d == ThemePrivate::globalTheme) {
 if (!ThemePrivate::globalThemeRefCount.deref()) {
 disconnect(ThemePrivate::globalTheme, 0, this, 0);

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/215
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ica6ba0273bc99fb8ad8733a1c90db8f1e87c49ea
Gerrit-PatchSet: 1
Gerrit-Project: plasma-framework
Gerrit-Branch: master
Gerrit-Owner: David Edmundson 
Gerrit-Reviewer: Aaron J. Seigo 
Gerrit-Reviewer: Marco Martin 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Change in plasma-framework[master]: Do not keep reparsing plasmarc configuration

2014-12-10 Thread David Edmundson (Code Review)
David Edmundson has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/214

Change subject: Do not keep reparsing plasmarc configuration
..

Do not keep reparsing plasmarc configuration

KConfig->reparseConfiguration is expensive. It throws away our cached
values.

The Units constructor was calling this every single time. Units is
created a _lot_; once per applet and once per FrameSVGItem.

This meant we were reloading the same config ~100 times on startup for
no reason.

Perf showed this as being ~5% of the total startup time.

-7.47% 0.00%  plasmashell  libKF5ConfigCore.so.5.5.0
   [.] KConfig::reparseConfiguration()

   - KConfig::reparseConfiguration()

  + 66.51% Units::settingsFileChanged(QString const&)

  + 25.95% KConfig::KConfig(QString const&,
QFlags, QStandardPaths::StandardLocation)

  + 3.93% KDesktopFile::KDesktopFile(QString const&)

  + 3.61% Units::settingsFileChanged(QString const&)

Change-Id: Ia70b7001ba473c8063e6c999b8e4233ea5b206f5
---
M src/declarativeimports/core/units.cpp
M src/declarativeimports/core/units.h
2 files changed, 20 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/plasma-framework 
refs/changes/14/214/1

diff --git a/src/declarativeimports/core/units.cpp 
b/src/declarativeimports/core/units.cpp
index ba06553..43533df 100644
--- a/src/declarativeimports/core/units.cpp
+++ b/src/declarativeimports/core/units.cpp
@@ -1,6 +1,7 @@
 /***
  *   Copyright 2013 Marco Martin *
  *   Copyright 2014 Sebastian Kügler*
+ *   Copyright 2014 David Edmundson*
  * *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
@@ -59,8 +60,8 @@
 connect(KDirWatch::self(), &KDirWatch::dirty, this, 
&Units::settingsFileChanged);
 // ... but also remove/recreate cycles, like KConfig does it
 connect(KDirWatch::self(), &KDirWatch::created, this, 
&Units::settingsFileChanged);
-// Trigger configuration read
-settingsFileChanged(plasmarc);
+// read configuration
+updatePlasmaRCSettings();
 }
 
 Units::~Units()
@@ -70,18 +71,24 @@
 void Units::settingsFileChanged(const QString &file)
 {
 if (file.endsWith(plasmarc)) {
-
-KConfigGroup cfg = KConfigGroup(KSharedConfig::openConfig(plasmarc), 
groupName);
-cfg.config()->reparseConfiguration();
-const int longDuration = cfg.readEntry("longDuration", 
defaultLongDuration);
-
-if (longDuration != m_longDuration) {
-m_longDuration = longDuration;
-emit durationChanged();
-}
+KSharedConfigPtr cfg = KSharedConfig::openConfig(plasmarc);
+cfg->reparseConfiguration();
+updatePlasmaRCSettings();
 }
 }
 
+void Units::updatePlasmaRCSettings()
+{
+KConfigGroup cfg = KConfigGroup(KSharedConfig::openConfig(plasmarc), 
groupName);
+const int longDuration = cfg.readEntry("longDuration", 
defaultLongDuration);
+
+if (longDuration != m_longDuration) {
+m_longDuration = longDuration;
+emit durationChanged();
+}
+}
+
+
 void Units::iconLoaderSettingsChanged()
 {
 // These are not scaled, we respect the user's setting over dpi scaling
diff --git a/src/declarativeimports/core/units.h 
b/src/declarativeimports/core/units.h
index e1d9bde..a469423 100644
--- a/src/declarativeimports/core/units.h
+++ b/src/declarativeimports/core/units.h
@@ -164,11 +164,12 @@
 
 private Q_SLOTS:
 void iconLoaderSettingsChanged();
-void settingsFileChanged(const QString &settings);
+void settingsFileChanged(const QString &file);
 
 private:
 void updateDevicePixelRatio();
 void updateSpacing();
+void updatePlasmaRCSettings();
 /**
  * @return The dpi-adjusted size for a given icon size
  */

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/214
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia70b7001ba473c8063e6c999b8e4233ea5b206f5
Gerrit-PatchSet: 1
Gerrit-Project: plasma-framework
Gerrit-Branch: master
Gerrit-Owner: David Edmundson 
Gerrit-Reviewer: Sebastian Kügler 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 121424: [GCI2014]Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Toby Chen

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

(Updated Dec. 10, 2014, 11:22 p.m.)


Review request for Plasma and David Edmundson.


Repository: plasma-desktop


Description
---

[GCI2014]Remove the depencence on KDELibs4Support form KCMAccess


Diffs (updated)
-

  kcms/access/main.cpp 09864fc 
  kcms/access/CMakeLists.txt 64b1cc9 
  kcms/access/kaccess.h 716bfcf 
  kcms/access/kaccess.cpp aea6f86 
  kcms/access/kcmaccess.h d3cb40f 
  kcms/access/kcmaccess.cpp 6c8bc65 

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


Testing
---

I have ran kcmshell5 kcmaccess, did see no problem.


Thanks,

Toby Chen

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


[Breeze] [Bug 341762] When utilizing the Breeze theme for SDDM, it takes ages for SDDM to startup.

2014-12-10 Thread David Edmundson
https://bugs.kde.org/show_bug.cgi?id=341762

David Edmundson  changed:

   What|Removed |Added

 Resolution|--- |WAITINGFORINFO
 Status|UNCONFIRMED |NEEDSINFO
 CC||k...@davidedmundson.co.uk

--- Comment #1 from David Edmundson  ---
wow. That's pretty extreme

A delay that big must be a DBus timeout (or rather 3 sequential DBus timeouts).

We end up querying solid for battery status which does go via an activated
kded; so potentially that's where we block.

Can you:
 - confirm what version you are using.
 - tell me where you get your RPMs from
 - check the log files from SDDM (journalctl -u sddm)

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 121424: [GCI2014]Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Jeremy Whiting

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



kcms/access/kaccess.h


Why is QAbstractButton needed here? I don't see it used in the header file 
anywhere.



kcms/access/kcmaccess.h


I don't see any need for a new configChanged slot that takes a double, all 
it does is the same thing as the original configChanged.



kcms/access/kcmaccess.cpp


This should just be removed, you created a QLabel with this text, no need 
for a commented out line from the old KNumInput api.


- Jeremy Whiting


On Dec. 10, 2014, 2:31 p.m., Toby Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121424/
> ---
> 
> (Updated Dec. 10, 2014, 2:31 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> [GCI2014]Remove the depencence on KDELibs4Support form KCMAccess
> 
> 
> Diffs
> -
> 
>   kcms/access/main.cpp 09864fc 
>   kcms/access/kcmaccess.h d3cb40f 
>   kcms/access/kcmaccess.cpp 6c8bc65 
>   kcms/access/kaccess.h 716bfcf 
>   kcms/access/kaccess.cpp aea6f86 
>   kcms/access/CMakeLists.txt 64b1cc9 
> 
> Diff: https://git.reviewboard.kde.org/r/121424/diff/
> 
> 
> Testing
> ---
> 
> I have ran kcmshell5 kcmaccess, did see no problem.
> 
> 
> Thanks,
> 
> Toby Chen
> 
>

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


Re: Review Request 121424: [GCI2014]Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Toby Chen

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

(Updated Dec. 10, 2014, 9:31 p.m.)


Review request for Plasma and David Edmundson.


Repository: plasma-desktop


Description
---

[GCI2014]Remove the depencence on KDELibs4Support form KCMAccess


Diffs (updated)
-

  kcms/access/main.cpp 09864fc 
  kcms/access/kcmaccess.h d3cb40f 
  kcms/access/kcmaccess.cpp 6c8bc65 
  kcms/access/kaccess.h 716bfcf 
  kcms/access/kaccess.cpp aea6f86 
  kcms/access/CMakeLists.txt 64b1cc9 

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


Testing
---

I have ran kcmshell5 kcmaccess, did see no problem.


Thanks,

Toby Chen

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


[Breeze] [Bug 341762] New: When utilizing the Breeze theme for SDDM, it takes ages for SDDM to startup.

2014-12-10 Thread Raymond Wooninck
https://bugs.kde.org/show_bug.cgi?id=341762

Bug ID: 341762
   Summary: When utilizing the Breeze theme for SDDM, it takes
ages for SDDM to startup.
   Product: Breeze
   Version: unspecified
  Platform: openSUSE RPMs
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: NOR
 Component: general
  Assignee: plasma-devel@kde.org
  Reporter: tittiatc...@gmail.com

Due to the very nice integration with the rest of the desktop, I am using the
Breeze theme for SDDM (created by David Edmundson). It looks way better than
the default theme of SDDM, but it has one very big disadvantage. When the
Breeze theme is active, it takes about 1,5 minute for SDDM to show its login
screen. (Measured with systemd-analyze blame). Using the default theme for SDDM
(Maui), then it only takes a fraction of a second and systemd-analyze blame
doesn't even show a line for it. 

The only difference that I notice is that is feels that it takes longer for
plasma to startup when using the Maui theme, then with the Breeze theme.
However these few seconds still doesn't justify the 1,5 minutes startup for
SDDM. 

Reproducible: Always

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 121424: [GCI2014]Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Aleix Pol Gonzalez

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



kcms/access/kaccess.cpp


Don't leave commented code.



kcms/access/main.cpp


This used to be a unique application, you want to set:
KDBusService service(KDBusService::Unique);


- Aleix Pol Gonzalez


On Dec. 10, 2014, 6:11 p.m., Toby Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121424/
> ---
> 
> (Updated Dec. 10, 2014, 6:11 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> [GCI2014]Remove the depencence on KDELibs4Support form KCMAccess
> 
> 
> Diffs
> -
> 
>   kcms/access/kaccess.h 716bfcf 
>   kcms/access/kaccess.cpp aea6f86 
>   kcms/access/kcmaccess.h d3cb40f 
>   kcms/access/kcmaccess.cpp 6c8bc65 
>   kcms/access/main.cpp 09864fc 
>   kcms/access/CMakeLists.txt 64b1cc9 
> 
> Diff: https://git.reviewboard.kde.org/r/121424/diff/
> 
> 
> Testing
> ---
> 
> I have ran kcmshell5 kcmaccess, did see no problem.
> 
> 
> Thanks,
> 
> Toby Chen
> 
>

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


Re: Review Request 121424: [GCI2014]Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread David Edmundson

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



kcms/access/kaccess.cpp


this isn't a complete port of KUniqueApplication

There should be something with KDBusService in here.



kcms/access/kaccess.cpp


delete this bit of code.



kcms/access/kaccess.cpp


all the way to here.



kcms/access/kaccess.cpp


it should be safe to just delete this line. 
It will default to this.



kcms/access/kaccess.cpp


and this.


- David Edmundson


On Dec. 10, 2014, 6:11 p.m., Toby Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121424/
> ---
> 
> (Updated Dec. 10, 2014, 6:11 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> [GCI2014]Remove the depencence on KDELibs4Support form KCMAccess
> 
> 
> Diffs
> -
> 
>   kcms/access/kaccess.h 716bfcf 
>   kcms/access/kaccess.cpp aea6f86 
>   kcms/access/kcmaccess.h d3cb40f 
>   kcms/access/kcmaccess.cpp 6c8bc65 
>   kcms/access/main.cpp 09864fc 
>   kcms/access/CMakeLists.txt 64b1cc9 
> 
> Diff: https://git.reviewboard.kde.org/r/121424/diff/
> 
> 
> Testing
> ---
> 
> I have ran kcmshell5 kcmaccess, did see no problem.
> 
> 
> Thanks,
> 
> Toby Chen
> 
>

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


Re: Review Request 121219: Load IconItem immediately upon componentComplete()

2014-12-10 Thread David Edmundson


> On Dec. 10, 2014, 6:12 p.m., David Edmundson wrote:
> > src/declarativeimports/core/iconitem.cpp, line 307
> > 
> >
> > Found one of the reasons for the blurring.
> > 
> > To some extent, your first patch was right before I told you to change 
> > it (sorry!)
> > 
> > when the input size is 0,0
> > 
> > Units::roundToIconSize(0,0) returns 8.
> > size is now not <= 0 so we load something.
> > 
> > This means we'll animate when it gets a valid size.
> > 
> > IMHO roundToIconSize should return 0 in this special case, as right now 
> > we have an entire code path that will never be used. 
> > 
> > I don't know if that will cause other problems.
> > 
> > 
> > We never hit this before, because setSource() has a size check, but we 
> > now call loadPixmap directly in componentComplete()

opened here. https://gerrit.vesnicky.cesnet.cz/r/#/c/213/

Kai, can you see if it fixes anything/everything?


- David


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


On Dec. 5, 2014, 9:20 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121219/
> ---
> 
> (Updated Dec. 5, 2014, 9:20 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This makes the IconItem load the icon immediately after component creation 
> and not wait 150ms there for no reason which prevents eg. flickering in the 
> OSD when it shows up.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.h 8aecd17 
>   src/declarativeimports/core/iconitem.cpp ed3bb97 
> 
> Diff: https://git.reviewboard.kde.org/r/121219/diff/
> 
> 
> Testing
> ---
> 
> Totally forgot about that, just took it for granted that the OSD wouldn't 
> flicker anymore.
> Resizing applets still has it scale the texture before re-rendering it.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Change in plasma-framework[master]: roundToIconSize(0) should return 0

2014-12-10 Thread David Edmundson (Code Review)
David Edmundson has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/213

Change subject: roundToIconSize(0) should return 0
..

roundToIconSize(0) should return 0

If an invalid icon size is passed to roundToIconSize we should return an
invalid icon size.

This can cause IconItem to load a small pixmap which will never be
shown.

Change-Id: Ia678f2e879b83317e2971069acf8f00d9ce2e052
---
M src/declarativeimports/core/units.cpp
1 file changed, 3 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/plasma-framework 
refs/changes/13/213/1

diff --git a/src/declarativeimports/core/units.cpp 
b/src/declarativeimports/core/units.cpp
index dfc817a..ba06553 100644
--- a/src/declarativeimports/core/units.cpp
+++ b/src/declarativeimports/core/units.cpp
@@ -107,9 +107,10 @@
 int Units::roundToIconSize(int size)
 {
 /*Do *not* use devicePixelIconSize here, we want to use the sizes of the 
pixmaps of the smallest icons on the disk. And those are unaffected by dpi*/
-if (size < KIconLoader::SizeSmall) {
+if (size <= 0) {
+return 0;
+} else if (size < KIconLoader::SizeSmall) {
 return KIconLoader::SizeSmall/2;
-
 } else if (size < KIconLoader::SizeSmallMedium) {
 return KIconLoader::SizeSmall;
 

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/213
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia678f2e879b83317e2971069acf8f00d9ce2e052
Gerrit-PatchSet: 1
Gerrit-Project: plasma-framework
Gerrit-Branch: master
Gerrit-Owner: David Edmundson 
Gerrit-Reviewer: Marco Martin 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 121219: Load IconItem immediately upon componentComplete()

2014-12-10 Thread David Edmundson

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



src/declarativeimports/core/iconitem.cpp


Found one of the reasons for the blurring.

To some extent, your first patch was right before I told you to change it 
(sorry!)

when the input size is 0,0

Units::roundToIconSize(0,0) returns 8.
size is now not <= 0 so we load something.

This means we'll animate when it gets a valid size.

IMHO roundToIconSize should return 0 in this special case, as right now we 
have an entire code path that will never be used. 

I don't know if that will cause other problems.

We never hit this before, because setSource() has a size check, but we now 
call loadPixmap directly in componentComplete()


- David Edmundson


On Dec. 5, 2014, 9:20 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121219/
> ---
> 
> (Updated Dec. 5, 2014, 9:20 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This makes the IconItem load the icon immediately after component creation 
> and not wait 150ms there for no reason which prevents eg. flickering in the 
> OSD when it shows up.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.h 8aecd17 
>   src/declarativeimports/core/iconitem.cpp ed3bb97 
> 
> Diff: https://git.reviewboard.kde.org/r/121219/diff/
> 
> 
> Testing
> ---
> 
> Totally forgot about that, just took it for granted that the OSD wouldn't 
> flicker anymore.
> Resizing applets still has it scale the texture before re-rendering it.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 121424: [GCI2014]Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Toby Chen

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

(Updated Dec. 10, 2014, 6:11 p.m.)


Review request for Plasma and David Edmundson.


Summary (updated)
-

[GCI2014]Porting Accessibility Config Module away from KDELibs4Support


Repository: plasma-desktop


Description
---

[GCI2014]Remove the depencence on KDELibs4Support form KCMAccess


Diffs
-

  kcms/access/kaccess.h 716bfcf 
  kcms/access/kaccess.cpp aea6f86 
  kcms/access/kcmaccess.h d3cb40f 
  kcms/access/kcmaccess.cpp 6c8bc65 
  kcms/access/main.cpp 09864fc 
  kcms/access/CMakeLists.txt 64b1cc9 

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


Testing
---

I have ran kcmshell5 kcmaccess, did see no problem.


Thanks,

Toby Chen

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


Re: Review Request 121424: Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Toby Chen

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

(Updated Dec. 10, 2014, 6:10 p.m.)


Review request for Plasma and David Edmundson.


Repository: plasma-desktop


Description (updated)
---

[GCI2014]Remove the depencence on KDELibs4Support form KCMAccess


Diffs
-

  kcms/access/kaccess.h 716bfcf 
  kcms/access/kaccess.cpp aea6f86 
  kcms/access/kcmaccess.h d3cb40f 
  kcms/access/kcmaccess.cpp 6c8bc65 
  kcms/access/main.cpp 09864fc 
  kcms/access/CMakeLists.txt 64b1cc9 

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


Testing
---

I have ran kcmshell5 kcmaccess, did see no problem.


Thanks,

Toby Chen

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


Re: Review Request 121424: Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Toby Chen


> On Dec. 10, 2014, 1:50 p.m., Lukáš Tinkl wrote:
> > Did you test the accessibility features?

Yes, they work fine.


- Toby


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


On Dec. 10, 2014, 1:44 p.m., Toby Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121424/
> ---
> 
> (Updated Dec. 10, 2014, 1:44 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Remove the depencence on KDELibs4Support form KCMAccess
> 
> 
> Diffs
> -
> 
>   kcms/access/kaccess.h 716bfcf 
>   kcms/access/kaccess.cpp aea6f86 
>   kcms/access/kcmaccess.h d3cb40f 
>   kcms/access/kcmaccess.cpp 6c8bc65 
>   kcms/access/main.cpp 09864fc 
>   kcms/access/CMakeLists.txt 64b1cc9 
> 
> Diff: https://git.reviewboard.kde.org/r/121424/diff/
> 
> 
> Testing
> ---
> 
> I have ran kcmshell5 kcmaccess, did see no problem.
> 
> 
> Thanks,
> 
> Toby Chen
> 
>

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


Re: Review Request 120970: Remove combo "Graphical Effects"

2014-12-10 Thread David Edmundson


> On Dec. 10, 2014, 3:19 p.m., Lukáš Tinkl wrote:
> > I think this change has to be reverted, the setting in question is being 
> > used in frameworkintegration to set the level of graphical details to Qt, 
> > thru the QPA plugin:
> > 
> > m_hints[QPlatformTheme::UiEffects] = cg.readEntry("GraphicEffectsLevel", 0) 
> > != 0 ? QPlatformTheme::GeneralUiEffect : 0;

That's using it purely as a boolean. I'm not having a 5 entry combo box for 
that.

Personally I say it can be a hidden config option; we don't /need/ to expose 
every option just because it can be set.

If you really want it, I could combine it with Breeze's internal "Enable 
Animation" config option. From a usability POV it seems better than having two 
very similar sounding options which are scattered.


- David


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


On Dec. 10, 2014, 2:49 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120970/
> ---
> 
> (Updated Dec. 10, 2014, 2:49 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Remove combo "Graphical Effects"
> 
> It set a config value which was exposed in
> KGlobalSettings::graphicEffectsLevel that is now deprecated.
> 
> 
> Diffs
> -
> 
>   kcms/style/finetuning.ui 3abb692 
>   kcms/style/kcmstyle.cpp 6585ee6 
> 
> Diff: https://git.reviewboard.kde.org/r/120970/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread David Edmundson


> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote:
> > Wasn't that part of the idea? Having it scale up the pixmap first when 
> > resizing and then re-rendering it later?
> 
> Xuetian Weng wrote:
> 1. icon size (the widget size) doesn't change frequently. Usually it only 
> happens when user resize the UI or changes the settings.
> 2. the animation IMHO is for smooth transition between different 
> icons/different icon effect (check volume icon, media player play/pause, 
> hovering with opacity changes), I don't see we need to have animated 
> transition from a scaled icon to the actual correct size icon.
> 
> David Edmundson wrote:
> Icon size changes may be infrequent but they do happen. It's important 
> that when we resize the panel we don't re-render a tonne of SVGs constantly, 
> we need resizing applets to be smooth.
> 
> I don't understand where this blurriness is coming from; that implies 
> we're loading it at a wrong size then resizing; if that's the case, lets fix 
> that rather than hide it.
> 
> Sebastian Kügler wrote:
> During resize, we're rescaling a pixmap instead of re-rendering the SVG 
> for every frame. That can induce blur.
> 
> Also, rendering an SVG at random sizes van induce blur, since not 
> everything can be aligned to pixels all the time, our SVGs are optimized for 
> "standard sizes".
> 
> Xuetian Weng wrote:
> The problem is not loading it at wrong size. It is the widget is not 
> resized to correct size yet.
> The most significant one on plasma here is Device Notifier (When it's 
> popped up at the first time), I print out some debug message at 
> ::geometryChanged
> ICON QSizeF(1, 86) QSizeF(1, 1) QVariant(QString, "device-notifier")
> ICON QSizeF(129, 86) QSizeF(1, 86) QVariant(QString, "device-notifier")
> 
> As you can see, the size changed from 1x1 to 1x86 then to 129x86. So they 
> are valid size, and the blurriness is exposed by the transition animation.

Thanks. I can reproduce with the Device Notifier + debug

What I'm trying to argue is that it should go directly to 129x86. (or at least 
be an invalid size till then)

Otherwise we're loading an SVG 3 times and not using two of them this patch 
will hide the visual artifacts of that, but we'll still be wasting a lot of CPU 
cycles doing something silly.


- David


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


On Dec. 9, 2014, 4:43 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121411/
> ---
> 
> (Updated Dec. 9, 2014, 4:43 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Kai Uwe 
> Broulik.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Making transition between two different size doesn't make much sense, since 
> repainting is usually happens at that time and it could take some time to 
> finish. And animation need to be stopped if m_animValue is set manually.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 
> ed3bb978f01cca98315fd425778139e4b9eeb64f 
> 
> Diff: https://git.reviewboard.kde.org/r/121411/diff/
> 
> 
> Testing
> ---
> 
> Looks fine on tray icon and lock screen, no blurry transition anymore.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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


Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Xuetian Weng


> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote:
> > Wasn't that part of the idea? Having it scale up the pixmap first when 
> > resizing and then re-rendering it later?
> 
> Xuetian Weng wrote:
> 1. icon size (the widget size) doesn't change frequently. Usually it only 
> happens when user resize the UI or changes the settings.
> 2. the animation IMHO is for smooth transition between different 
> icons/different icon effect (check volume icon, media player play/pause, 
> hovering with opacity changes), I don't see we need to have animated 
> transition from a scaled icon to the actual correct size icon.
> 
> David Edmundson wrote:
> Icon size changes may be infrequent but they do happen. It's important 
> that when we resize the panel we don't re-render a tonne of SVGs constantly, 
> we need resizing applets to be smooth.
> 
> I don't understand where this blurriness is coming from; that implies 
> we're loading it at a wrong size then resizing; if that's the case, lets fix 
> that rather than hide it.
> 
> Sebastian Kügler wrote:
> During resize, we're rescaling a pixmap instead of re-rendering the SVG 
> for every frame. That can induce blur.
> 
> Also, rendering an SVG at random sizes van induce blur, since not 
> everything can be aligned to pixels all the time, our SVGs are optimized for 
> "standard sizes".

The problem is not loading it at wrong size. It is the widget is not resized to 
correct size yet.
The most significant one on plasma here is Device Notifier (When it's popped up 
at the first time), I print out some debug message at ::geometryChanged
ICON QSizeF(1, 86) QSizeF(1, 1) QVariant(QString, "device-notifier")
ICON QSizeF(129, 86) QSizeF(1, 86) QVariant(QString, "device-notifier")

As you can see, the size changed from 1x1 to 1x86 then to 129x86. So they are 
valid size, and the blurriness is exposed by the transition animation.


- Xuetian


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


On Dec. 9, 2014, 4:43 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121411/
> ---
> 
> (Updated Dec. 9, 2014, 4:43 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Kai Uwe 
> Broulik.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Making transition between two different size doesn't make much sense, since 
> repainting is usually happens at that time and it could take some time to 
> finish. And animation need to be stopped if m_animValue is set manually.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 
> ed3bb978f01cca98315fd425778139e4b9eeb64f 
> 
> Diff: https://git.reviewboard.kde.org/r/121411/diff/
> 
> 
> Testing
> ---
> 
> Looks fine on tray icon and lock screen, no blurry transition anymore.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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


Re: Qt 5.4

2014-12-10 Thread Martin Gräßlin
On Wednesday 10 December 2014 15:58:25 Jonathan Riddell wrote:
> Qt 5.4 is out and it's time to make a decision on whether Plasma 5.2 will
> require it.
> 
> Speaking to packagers from Ubuntu, opensuse and Fedora they all have RC
> packages and don't expect it to be difficult to have final packages.
> 
> I know there's a patch Martin says is needed but that shouldn't be hard to
> communicate.

Not an issue, the problem never made it into 5.4.0, it's in the 5.4 branch, 
though

> 
> And there's some features that we want.
> 
> Most Plasma devs seem to be testing with Qt 5.4 anyway.
> 
> Distros like Kubuntu and Fedora want to ship with Plasma 5.2 (as I
> understand it) but in some months when Qt 5.3 will just be old school.
> 
> So it seems to me like we should require Qt 5.4 and switch that soon.
> 
> Jonathan


signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Qt 5.4

2014-12-10 Thread Eike Hein



On 12/10/2014 04:14 PM, Aleix Pol wrote:

On Wed, Dec 10, 2014 at 4:10 PM, Eike Hein mailto:h...@kde.org>> wrote:


There's a bunch of unsolved new regressions (e.g. DND of files is
broken on 5.4) but nothing out of the ordinary breakage level I
guess.


Cheers,
Eike


I wasn't aware, I've worked on that code, if you can provide a test case
I'll give it a go.


Basically, KDE file managers use KDirModel::mimeData(QModelIndexList)
to generate MIME payload for QDrags they initiate, which means a
text/uri-list of file URLs. These are intact before QDrag::exec() but
wind up not making it to the other side - formats() has text/uri-list
and hasUrls() is true, but urls() is an empty QList. It's most likely
an encode/decode issue, i.e. the raw data is in the drop but urls()
fails to decode them. I'm guessing it's related to QTBUG-42285.



Aleix


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


Re: Review Request 120970: Remove combo "Graphical Effects"

2014-12-10 Thread Lukáš Tinkl

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


I think this change has to be reverted, the setting in question is being used 
in frameworkintegration to set the level of graphical details to Qt, thru the 
QPA plugin:

m_hints[QPlatformTheme::UiEffects] = cg.readEntry("GraphicEffectsLevel", 0) != 
0 ? QPlatformTheme::GeneralUiEffect : 0;

- Lukáš Tinkl


On Pro. 10, 2014, 3:49 odp., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120970/
> ---
> 
> (Updated Pro. 10, 2014, 3:49 odp.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Remove combo "Graphical Effects"
> 
> It set a config value which was exposed in
> KGlobalSettings::graphicEffectsLevel that is now deprecated.
> 
> 
> Diffs
> -
> 
>   kcms/style/finetuning.ui 3abb692 
>   kcms/style/kcmstyle.cpp 6585ee6 
> 
> Diff: https://git.reviewboard.kde.org/r/120970/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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


Re: Qt 5.4

2014-12-10 Thread Aleix Pol
On Wed, Dec 10, 2014 at 4:10 PM, Eike Hein  wrote:

>
> There's a bunch of unsolved new regressions (e.g. DND of files is
> broken on 5.4) but nothing out of the ordinary breakage level I
> guess.
>
>
> Cheers,
> Eike


I wasn't aware, I've worked on that code, if you can provide a test case
I'll give it a go.

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


Re: Qt 5.4

2014-12-10 Thread Eike Hein


There's a bunch of unsolved new regressions (e.g. DND of files is
broken on 5.4) but nothing out of the ordinary breakage level I
guess.


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


Re: Qt 5.4

2014-12-10 Thread Aleix Pol
On Wed, Dec 10, 2014 at 3:58 PM, Jonathan Riddell  wrote:

> Qt 5.4 is out and it's time to make a decision on whether Plasma 5.2 will
> require it.
>
> Speaking to packagers from Ubuntu, opensuse and Fedora they all have RC
> packages and don't expect it to be difficult to have final packages.
>
> I know there's a patch Martin says is needed but that shouldn't be hard to
> communicate.
>
> And there's some features that we want.
>
> Most Plasma devs seem to be testing with Qt 5.4 anyway.
>
> Distros like Kubuntu and Fedora want to ship with Plasma 5.2 (as I
> understand it) but in some months when Qt 5.3 will just be old school.
>
> So it seems to me like we should require Qt 5.4 and switch that soon.
>
> Jonathan
>

+1.

I've been using 5.4 since August and works properly, we even got some fixes
in which I'm sure they end up being handy.

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


Re: Qt 5.4

2014-12-10 Thread Sebastian Kügler
On Wednesday, December 10, 2014 15:58:25 Jonathan Riddell wrote:
> Qt 5.4 is out and it's time to make a decision on whether Plasma 5.2 will
> require it.
> 
> Speaking to packagers from Ubuntu, opensuse and Fedora they all have RC
> packages and don't expect it to be difficult to have final packages.
> 
> I know there's a patch Martin says is needed but that shouldn't be hard to
> communicate.
> 
> And there's some features that we want.
> 
> Most Plasma devs seem to be testing with Qt 5.4 anyway.
> 
> Distros like Kubuntu and Fedora want to ship with Plasma 5.2 (as I
> understand it) but in some months when Qt 5.3 will just be old school.
> 
> So it seems to me like we should require Qt 5.4 and switch that soon.

+1.
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Qt 5.4

2014-12-10 Thread Jonathan Riddell
Qt 5.4 is out and it's time to make a decision on whether Plasma 5.2 will
require it.

Speaking to packagers from Ubuntu, opensuse and Fedora they all have RC
packages and don't expect it to be difficult to have final packages.

I know there's a patch Martin says is needed but that shouldn't be hard to
communicate.

And there's some features that we want.

Most Plasma devs seem to be testing with Qt 5.4 anyway.

Distros like Kubuntu and Fedora want to ship with Plasma 5.2 (as I
understand it) but in some months when Qt 5.3 will just be old school.

So it seems to me like we should require Qt 5.4 and switch that soon.

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


Re: Review Request 121421: Make the OSD timeout a bit shorter

2014-12-10 Thread Martin Klapetek

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

(Updated Dec. 10, 2014, 2:55 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

We concluded with Kai that 2.5s seems a bit too long for the OSD timeout, so we 
experimented a bit and decided to shorten it to 1.8s. 


Diffs
-

  lookandfeel/contents/osd/Osd.qml 606eff4 

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


Testing
---

Feels better and gets out of the way sooner, but not too soon.


Thanks,

Martin Klapetek

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


Re: Review Request 120970: Remove combo "Graphical Effects"

2014-12-10 Thread David Edmundson

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

(Updated Dec. 10, 2014, 2:49 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-desktop


Description
---

Remove combo "Graphical Effects"

It set a config value which was exposed in
KGlobalSettings::graphicEffectsLevel that is now deprecated.


Diffs
-

  kcms/style/finetuning.ui 3abb692 
  kcms/style/kcmstyle.cpp 6585ee6 

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


Testing
---


Thanks,

David Edmundson

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


Re: Review Request 121402: Allow user to customise wallpaper used in the lock screen.

2014-12-10 Thread David Edmundson

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

(Updated Dec. 10, 2014, 2:47 p.m.)


Review request for Plasma.


Repository: plasma-workspace


Description
---

Allow user to customise wallpaper used in the lock screen.

Nothing very fancy it's the same plain image approach we use in SDDM. 
Default behaviour is unchanged. We can always expand it later.


Diffs (updated)
-

  lookandfeel/contents/lockscreen/LockScreen.qml ca9beb2 
  ksmserver/screenlocker/kcm/kcm.cpp 0f74c45 
  ksmserver/screenlocker/kcm/kcm.ui c891cc9 
  ksmserver/screenlocker/kcm/selectimagebutton.h PRE-CREATION 
  ksmserver/screenlocker/kcm/selectimagebutton.cpp PRE-CREATION 
  ksmserver/screenlocker/greeter/greeterapp.cpp 60bd347 
  ksmserver/screenlocker/kcfg/kscreenlockersettings.kcfg 5207615 
  ksmserver/screenlocker/kcm/CMakeLists.txt a23a0fc 

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


Testing
---

Opened lockscreen.
Most code is tried and tested in the SDDM KCM.


Thanks,

David Edmundson

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


Re: Review Request 121219: Load IconItem immediately upon componentComplete()

2014-12-10 Thread David Edmundson


> On Dec. 8, 2014, 11:20 p.m., Aleix Pol Gonzalez wrote:
> > +1
> > 
> > Looks good to me
> 
> Kai Uwe Broulik wrote:
> I now experience a lot of situtations where icons start blurry and fade 
> to their full size again, like when opening systray for the first time or 
> locking the session where it's quite severe. :/
> 
> Xuetian Weng wrote:
> The blurry transition is there before this change. I tried to create a 
> patch for fixing this (depends on your change): 
> https://git.reviewboard.kde.org/r/121411/

Can you give a reproducible case of this blurry -> full. 
Then we can fill this with qDebugs.


- David


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


On Dec. 5, 2014, 9:20 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121219/
> ---
> 
> (Updated Dec. 5, 2014, 9:20 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This makes the IconItem load the icon immediately after component creation 
> and not wait 150ms there for no reason which prevents eg. flickering in the 
> OSD when it shows up.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.h 8aecd17 
>   src/declarativeimports/core/iconitem.cpp ed3bb97 
> 
> Diff: https://git.reviewboard.kde.org/r/121219/diff/
> 
> 
> Testing
> ---
> 
> Totally forgot about that, just took it for granted that the OSD wouldn't 
> flicker anymore.
> Resizing applets still has it scale the texture before re-rendering it.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread Sebastian Kügler


> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote:
> > Wasn't that part of the idea? Having it scale up the pixmap first when 
> > resizing and then re-rendering it later?
> 
> Xuetian Weng wrote:
> 1. icon size (the widget size) doesn't change frequently. Usually it only 
> happens when user resize the UI or changes the settings.
> 2. the animation IMHO is for smooth transition between different 
> icons/different icon effect (check volume icon, media player play/pause, 
> hovering with opacity changes), I don't see we need to have animated 
> transition from a scaled icon to the actual correct size icon.
> 
> David Edmundson wrote:
> Icon size changes may be infrequent but they do happen. It's important 
> that when we resize the panel we don't re-render a tonne of SVGs constantly, 
> we need resizing applets to be smooth.
> 
> I don't understand where this blurriness is coming from; that implies 
> we're loading it at a wrong size then resizing; if that's the case, lets fix 
> that rather than hide it.

During resize, we're rescaling a pixmap instead of re-rendering the SVG for 
every frame. That can induce blur.

Also, rendering an SVG at random sizes van induce blur, since not everything 
can be aligned to pixels all the time, our SVGs are optimized for "standard 
sizes".


- Sebastian


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


On Dec. 9, 2014, 4:43 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121411/
> ---
> 
> (Updated Dec. 9, 2014, 4:43 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Kai Uwe 
> Broulik.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Making transition between two different size doesn't make much sense, since 
> repainting is usually happens at that time and it could take some time to 
> finish. And animation need to be stopped if m_animValue is set manually.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 
> ed3bb978f01cca98315fd425778139e4b9eeb64f 
> 
> Diff: https://git.reviewboard.kde.org/r/121411/diff/
> 
> 
> Testing
> ---
> 
> Looks fine on tray icon and lock screen, no blurry transition anymore.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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


Re: Review Request 121421: Make the OSD timeout a bit shorter

2014-12-10 Thread David Edmundson

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

Ship it!


Given the old value was chosen rather randomly, this can't be any worse. +1 
from me.

- David Edmundson


On Dec. 9, 2014, 10:52 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121421/
> ---
> 
> (Updated Dec. 9, 2014, 10:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We concluded with Kai that 2.5s seems a bit too long for the OSD timeout, so 
> we experimented a bit and decided to shorten it to 1.8s. 
> 
> 
> Diffs
> -
> 
>   lookandfeel/contents/osd/Osd.qml 606eff4 
> 
> Diff: https://git.reviewboard.kde.org/r/121421/diff/
> 
> 
> Testing
> ---
> 
> Feels better and gets out of the way sooner, but not too soon.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 121411: Don't trigger animation if size changed.

2014-12-10 Thread David Edmundson


> On Dec. 9, 2014, 6:11 p.m., Kai Uwe Broulik wrote:
> > Wasn't that part of the idea? Having it scale up the pixmap first when 
> > resizing and then re-rendering it later?
> 
> Xuetian Weng wrote:
> 1. icon size (the widget size) doesn't change frequently. Usually it only 
> happens when user resize the UI or changes the settings.
> 2. the animation IMHO is for smooth transition between different 
> icons/different icon effect (check volume icon, media player play/pause, 
> hovering with opacity changes), I don't see we need to have animated 
> transition from a scaled icon to the actual correct size icon.

Icon size changes may be infrequent but they do happen. It's important that 
when we resize the panel we don't re-render a tonne of SVGs constantly, we need 
resizing applets to be smooth.

I don't understand where this blurriness is coming from; that implies we're 
loading it at a wrong size then resizing; if that's the case, lets fix that 
rather than hide it.


- David


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


On Dec. 9, 2014, 4:43 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121411/
> ---
> 
> (Updated Dec. 9, 2014, 4:43 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Kai Uwe 
> Broulik.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Making transition between two different size doesn't make much sense, since 
> repainting is usually happens at that time and it could take some time to 
> finish. And animation need to be stopped if m_animValue is set manually.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/iconitem.cpp 
> ed3bb978f01cca98315fd425778139e4b9eeb64f 
> 
> Diff: https://git.reviewboard.kde.org/r/121411/diff/
> 
> 
> Testing
> ---
> 
> Looks fine on tray icon and lock screen, no blurry transition anymore.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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


Re: Review Request 121424: Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Lukáš Tinkl

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


Did you test the accessibility features?

- Lukáš Tinkl


On Pro. 10, 2014, 2:44 odp., Toby Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121424/
> ---
> 
> (Updated Pro. 10, 2014, 2:44 odp.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Remove the depencence on KDELibs4Support form KCMAccess
> 
> 
> Diffs
> -
> 
>   kcms/access/kaccess.h 716bfcf 
>   kcms/access/kaccess.cpp aea6f86 
>   kcms/access/kcmaccess.h d3cb40f 
>   kcms/access/kcmaccess.cpp 6c8bc65 
>   kcms/access/main.cpp 09864fc 
>   kcms/access/CMakeLists.txt 64b1cc9 
> 
> Diff: https://git.reviewboard.kde.org/r/121424/diff/
> 
> 
> Testing
> ---
> 
> I have ran kcmshell5 kcmaccess, did see no problem.
> 
> 
> Thanks,
> 
> Toby Chen
> 
>

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


Re: Review Request 121424: Porting Accessibility Config Module away from KDELibs4Support

2014-12-10 Thread Toby Chen

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

(Updated Dec. 10, 2014, 1:44 p.m.)


Review request for Plasma and David Edmundson.


Repository: plasma-desktop


Description
---

Remove the depencence on KDELibs4Support form KCMAccess


Diffs
-

  kcms/access/kaccess.h 716bfcf 
  kcms/access/kaccess.cpp aea6f86 
  kcms/access/kcmaccess.h d3cb40f 
  kcms/access/kcmaccess.cpp 6c8bc65 
  kcms/access/main.cpp 09864fc 
  kcms/access/CMakeLists.txt 64b1cc9 

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


Testing
---

I have ran kcmshell5 kcmaccess, did see no problem.


Thanks,

Toby Chen

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


Re: Minutes Monday Plasma Hangout

2014-12-10 Thread Martin Gräßlin
On Tuesday 09 December 2014 12:58:19 Sebastian Kügler wrote:
> Martin:
> - Will check if the fix for context menues is in 5.4.0

just checked: the problematic commit is in 5.4 branch but *not* in 5.4.0 tag.

Cheers
Martin

signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 121421: Make the OSD timeout a bit shorter

2014-12-10 Thread Martin Klapetek


> On Dec. 9, 2014, 11:56 p.m., Kai Uwe Broulik wrote:
> > +1
> > 
> > 1000ms is too short
> > 1500ms feels weird, it's *tick*, wait, …… ad OCD.
> > 2500ms is just too long, especially with this huge (almost) opaque box of 
> > an OSD.
> 
> Aleix Pol Gonzalez wrote:
> I don't really have an opinion, still wanted to remind that maybe what we 
> want is to find different solutions in case it really gets annoying, like 
> making it disappear on click or key press, for example.
> 
> Martin Klapetek wrote:
> OSD on timeout is correct. It's just the default time that's annoying. 
> Hiding it on user input is also something we've discussed, but at this point 
> we're unsure about the "how".
> 
> Aleix Pol Gonzalez wrote:
> That's what I meant, let's discuss the how. You may want to involve the 
> usability team maybe?

I meant "how" as in how to detect user input from the applet ;)


- Martin


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


On Dec. 9, 2014, 11:52 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121421/
> ---
> 
> (Updated Dec. 9, 2014, 11:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We concluded with Kai that 2.5s seems a bit too long for the OSD timeout, so 
> we experimented a bit and decided to shorten it to 1.8s. 
> 
> 
> Diffs
> -
> 
>   lookandfeel/contents/osd/Osd.qml 606eff4 
> 
> Diff: https://git.reviewboard.kde.org/r/121421/diff/
> 
> 
> Testing
> ---
> 
> Feels better and gets out of the way sooner, but not too soon.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 121421: Make the OSD timeout a bit shorter

2014-12-10 Thread Aleix Pol Gonzalez


> On Dec. 9, 2014, 10:56 p.m., Kai Uwe Broulik wrote:
> > +1
> > 
> > 1000ms is too short
> > 1500ms feels weird, it's *tick*, wait, …… ad OCD.
> > 2500ms is just too long, especially with this huge (almost) opaque box of 
> > an OSD.
> 
> Aleix Pol Gonzalez wrote:
> I don't really have an opinion, still wanted to remind that maybe what we 
> want is to find different solutions in case it really gets annoying, like 
> making it disappear on click or key press, for example.
> 
> Martin Klapetek wrote:
> OSD on timeout is correct. It's just the default time that's annoying. 
> Hiding it on user input is also something we've discussed, but at this point 
> we're unsure about the "how".

That's what I meant, let's discuss the how. You may want to involve the 
usability team maybe?


- Aleix


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


On Dec. 9, 2014, 10:52 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121421/
> ---
> 
> (Updated Dec. 9, 2014, 10:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We concluded with Kai that 2.5s seems a bit too long for the OSD timeout, so 
> we experimented a bit and decided to shorten it to 1.8s. 
> 
> 
> Diffs
> -
> 
>   lookandfeel/contents/osd/Osd.qml 606eff4 
> 
> Diff: https://git.reviewboard.kde.org/r/121421/diff/
> 
> 
> Testing
> ---
> 
> Feels better and gets out of the way sooner, but not too soon.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: New framework to review: KPackage

2014-12-10 Thread Marco Martin
On Wednesday 10 December 2014, Albert Astals Cid wrote:
> > I would like to submit it (kpackage repo) for the usual 2 weeks period of
> > review.
> 
> Add const &
> void setDefaultMimeTypes(QStringList mimeTypes);
> void setMimeTypes(const char *key, QStringList mimeTypes);
> 
> You probably want a Q_DISABLE_COPY or similar in PackageLoader

done

> Add const &
> foreach (QString prefix, d->contentsPrefixPaths) {

done

> 
> All those "char *" params make me a bit unhappy, any reason those are not
> QString or QByteArray?

made them QByteArrays


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


Re: Review Request 121421: Make the OSD timeout a bit shorter

2014-12-10 Thread Martin Klapetek


> On Dec. 9, 2014, 11:56 p.m., Kai Uwe Broulik wrote:
> > +1
> > 
> > 1000ms is too short
> > 1500ms feels weird, it's *tick*, wait, …… ad OCD.
> > 2500ms is just too long, especially with this huge (almost) opaque box of 
> > an OSD.
> 
> Aleix Pol Gonzalez wrote:
> I don't really have an opinion, still wanted to remind that maybe what we 
> want is to find different solutions in case it really gets annoying, like 
> making it disappear on click or key press, for example.

OSD on timeout is correct. It's just the default time that's annoying. Hiding 
it on user input is also something we've discussed, but at this point we're 
unsure about the "how".


- Martin


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


On Dec. 9, 2014, 11:52 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121421/
> ---
> 
> (Updated Dec. 9, 2014, 11:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> We concluded with Kai that 2.5s seems a bit too long for the OSD timeout, so 
> we experimented a bit and decided to shorten it to 1.8s. 
> 
> 
> Diffs
> -
> 
>   lookandfeel/contents/osd/Osd.qml 606eff4 
> 
> Diff: https://git.reviewboard.kde.org/r/121421/diff/
> 
> 
> Testing
> ---
> 
> Feels better and gets out of the way sooner, but not too soon.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 121365: Update brightness availability at runtime

2014-12-10 Thread Kai Uwe Broulik

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

(Updated Dec. 10, 2014, 9:40 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

This listens for maximum brightness changes to update the availability 
accordingly. It does not, however, monitor for the brightnesscontrols interface 
to become available on dbus and I have no idea how that would be done.


Diffs
-

  dataengines/powermanagement/powermanagementengine.h 8088209 
  dataengines/powermanagement/powermanagementengine.cpp ae10a0c 

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


Testing
---

Works as before, cannot really test that since I cannot rip out my keyboard. 
Please see the other review.


Thanks,

Kai Uwe Broulik

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


Re: Review Request 121419: Improve Capslock Enabled warning

2014-12-10 Thread Kai Uwe Broulik

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

(Updated Dec. 10, 2014, 9:40 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Daniel Vrátil.


Repository: plasma-workspace


Description
---

Don't use a separate label which causes the layout to shift but just use the 
notification label and prepend the warning to any actual message.


Diffs
-

  lookandfeel/contents/lockscreen/LockScreen.qml ca9beb2 

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


Testing
---

Yup. Looks better imho.


File Attachments


Just the warning
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/12/09/ff5b6404-f55a-4e72-afd3-1348f2e12258__capslockwarning1.png
Warning with failed unlock
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/12/09/f29104c3-3b76-4508-b9be-3fad2a3d7baa__capslockwarning2.png


Thanks,

Kai Uwe Broulik

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